After updating my mingw build, I get this build error: /cygdrive/c/mozilla/mozilla/build/cygwin-wrapper g++ -mno-cygwin -o nsDownloadMa nager.o -c -DMOZILLA_INTERNAL_API -DOSTYPE=\"WINNT5.1\" -DOSARCH=WINNT -I../.. /../../dist/include/xpcom -I../../../../dist/include/string -I../../../../dist/i nclude/rdf -I../../../../dist/include/uriloader -I../../../../dist/include/mimet ype -I../../../../dist/include/necko -I../../../../dist/include/pref -I../../../ ../dist/include/progressDlg -I../../../../dist/include/intl -I../../../../dist/i nclude/windowwatcher -I../../../../dist/include/webbrowserpersist -I../../../../ dist/include/appshell -I../../../../dist/include/dom -I../../../../dist/include/ embed_base -I../../../../dist/include/alerts -I../../../../dist/include/storage -I../../../../dist/include/xulapp -I../../../../dist/include -I../../../../dis t/include/downloads -I../../../../dist/include/nspr -DMOZ_PNG_READ -DPNG_NO_MMX _CODE -DMOZ_PNG_WRITE -I../../../../dist/sdk/include -fno-rtti -fno-exce ptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsy nth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pedantic -mms-b itfields -pipe -DDEBUG -D_DEBUG -DDEBUG_mw -DTRACING -g -DMOZILLA_CLIENT -inc lude ../../../../mozilla-config.h /cygdrive/c/mozilla/mozilla/toolkit/components /downloads/src/nsDownloadManager.cpp In file included from c:/mozilla/mozilla/toolkit/components/downloads/src/nsDown loadManager.cpp:76: c:/mozilla/mozilla/toolkit/components/downloads/src/nsDownloadScanner.h:12:19: m soav.h: No such file or directory In file included from c:/mozilla/mozilla/toolkit/components/downloads/src/nsDown loadManager.cpp:76: c:/mozilla/mozilla/toolkit/components/downloads/src/nsDownloadScanner.h:49: erro r: `IOfficeAntiVirus' was not declared in this scope c:/mozilla/mozilla/toolkit/components/downloads/src/nsDownloadScanner.h:49: erro r: template argument 1 is invalid c:/mozilla/mozilla/toolkit/components/downloads/src/nsDownloadScanner.h:49: erro r: ISO C++ forbids declaration of `mAVScanner' with no type make[6]: *** [nsDownloadManager.o] Error 1 make[6]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/toolkit/compone nts/downloads/src' make[5]: *** [libs] Error 2 make[5]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/toolkit/compone nts/downloads' make[4]: *** [libs] Error 2 make[4]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/toolkit/compone nts' make[3]: *** [libs_tier_toolkit] Error 2 make[3]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox' make[2]: *** [tier_toolkit] Error 2 make[2]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox' make[1]: *** [alldep] Error 2 make[1]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox' make: *** [alldep] Error 2 C:\mozilla\mozilla>

It looks like it cannot find the header msoav.h which defines the IOfficeAntiVirus interface. This header should be included on any reasonably recent Windows SDK. Can you check which sdk you're using and if the header is in its include directory?

Well, moving the msoav.h header file from the Microsoft Platform SDK into the mingw include dir fixes that build problem, but then I get this build error: cmps.dll nsToolkitCompsModule.o ./module.res -Wl,--whole-archive ../ startup/src/libappstartup_s.a ../downloads/src/libdownload_s.a ../alerts/src/li balerts_s.a ../url-classifier/src/liburlclassifier_s.a ../feeds/src/libfeed_s.a ../typeaheadfind/src/libfastfind_s.a -Wl,--no-whole-archive -L../../../modules/z lib/src -lmozz -L../../../dist/bin -L../../../dist/lib -lgkgfx ../../../dist/lib /libunicharutil_s.a -L../../../dist/lib -lxpcom -lxpcom_core -L../../../dist/bin -L../../../dist/lib -lnspr4 -lplc4 -lplds4 -L../../../dist/lib -ljs3250 -lm -lgdi32 -lwinmm -lwsock32 -lshell32 -lole32 ../downloads/src/libdownload_s.a(nsDownloadScanner.o): In function `ZN17nsDownlo adScanner9FindCLSIDEv':c:/mozilla/mozilla/toolkit/components/downloads/src/nsDow nloadScanner.cpp:113: undefined reference to `IID_ICatInformation' :c:/mozilla/mozilla/toolkit/components/downloads/src/nsDownloadScanner.cpp:113: undefined reference to `CLSID_StdComponentCategoriesMgr' collect2: ld returned 1 exit status make[5]: *** [tkitcmps.dll] Error 1 make[5]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/toolkit/compone nts/build' make[4]: *** [libs] Error 2 make[4]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox/toolkit/compone nts' make[3]: *** [libs_tier_toolkit] Error 2 make[3]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox' make[2]: *** [tier_toolkit] Error 2 make[2]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox' make[1]: *** [alldep] Error 2 make[1]: Leaving directory `/cygdrive/c/mozilla/mozilla/_firefox' make: *** [alldep] Error 2 But Mozilla needs to be built with Mingw without depending on Microsoft Platform SDK header files anyway, so I guess this code has to be ifdef-ed out for mingw users. It's not like one can copy the msoav.h header file into the mingw package by default, right?

I'm very much inclined to not add more ifdefs to the download manager code...

So this should probably be disabled for mingw users. This makes that possible again.

(In reply to comment #4) > So this should probably be disabled for mingw users. why should it probably be disabled for mingw users? It seems like it's a mingw bug for not supplying a header file that is part of the normal MSSDK.

(In reply to comment #5) > why should it probably be disabled for mingw users? It seems like it's a mingw > bug for not supplying a header file that is part of the normal MSSDK. You mean that it is allowed for mingw to include the header which is normally part of the MSSDK?

> You mean that it is allowed for mingw to include the header which is normally > part of the MSSDK? You cannot do this directly -- however, you can use the MSDN documentation to write a header file containing the required declarations. Ideally, this header file is then submitted to the w32api part of mingw. The header file is not enough (see Comment 2); for linking you also need the corresponding library files that are also part of w32api. In your case it seems that you need to extend these a bit to include CLSID_StdComponentCategoriesMgr.

Alright, I'm OK with taking this fix temporarily, but I want a bug filed upstream with mingw to fix this, and once that lands in mingw (as per comment 7), I'd like to back this out. Rob still needs to verify that this patch will work out OK.

Ok, thanks. Well, there is some documentation here: http://msdn2.microsoft.com/en-us/library/ms537369.aspx Looking at msoav.h, it may not seem that difficult to write an IOfficeAntiVirus definition based on the info on msdn, but I'm afraid I'm not capable of that.

Comment on attachment 278966 [details] [diff] [review] patch >+ifndef GNU_CXX > ifeq ($(OS_ARCH),WINNT) > CPPSRCS += nsDownloadScanner.cpp > endif >+endif This looks good. >+#ifndef __MINGW32__ > #ifdef XP_WIN > mScanner = new nsDownloadScanner(); > if (!mScanner) > return NS_ERROR_OUT_OF_MEMORY; > rv = mScanner->Init(); > if (NS_FAILED(rv)) { > delete mScanner; > mScanner = nsnull; > } > #endif >+#endif You still need to initialize mScanner here, otherwise it might be non-null. >+#ifndef __MINGW32__ > #ifdef XP_WIN > case nsIDownloadManager::DOWNLOAD_SCANNING: > { > nsresult rv = mDownloadManager->mScanner ? mDownloadManager->mScanner->ScanDownload(this) : NS_ERROR_NOT_INITIALIZED; > // If we failed, then fall through to 'download finished' > if (NS_SUCCEEDED(rv)) > break; > mDownloadState = aState = nsIDownloadManager::DOWNLOAD_FINISHED; > } > #endif >+#endif What happens if someone sets the state to scanning (as is done in OnStateChange IIRC).

Sorry, I was happy that I got my build to work again, I didn't really think any further than that. I've now filed a mingw bug, at: https://sourceforge.net/tracker/index.php?func=detail&aid=1785282&group_id=2435&atid=352435

Ping?

Comment on attachment 279032 [details] [diff] [review] patch Looks good technically. Just two style nits. 1. You have this pattern very often: >+#ifndef __MINGW32__ > #ifdef XP_WIN ... > #endif >+#endif which I think would be better replaced by >-#ifdef XP_WIN >+#if defined(XP_WIN) and !defined(__MINGW32__) ... > #endif since fewer preprocessor statements are (in general) better for readability/maintainability. >-#ifdef XP_WIN >+#if defined(WIN32) && !defined(__MINGW32__) > (void)SetState(nsIDownloadManager::DOWNLOAD_SCANNING); > #else > (void)SetState(nsIDownloadManager::DOWNLOAD_FINISHED); > #endif > } I'm not sure why you switched to checking for WIN32, but it's prevailing style to check for XP_WIN throughout the Mozilla codebase. I don't think there's any situation where the two would give different behavior.

Sorry for the delay, thanks for the review, I think this addresses your comments.

Comment on attachment 294450 [details] [diff] [review] patchv2 >+ifndef GNU_CXX > ... >+endif I haven't actually tried this out, but I assume that it works. >+#ifndef __MINGW32__ > #ifdef XP_WIN > nsDownloadManager() : mScanner(nsnull) { }; > private: > nsDownloadScanner *mScanner; > #endif >+#endif This one still needs to be consolidated into a single if/endif pair.

Sorry, I had that one actually changed, but I forgot to rediff it. I'll check the >+ifndef GNU_CXX thing on mozilla-build.

Comment on attachment 295935 [details] [diff] [review] patchv3 looks good

Comment on attachment 295935 [details] [diff] [review] patchv3 Thanks, asking approval.

Comment on attachment 295935 [details] [diff] [review] patchv3 a=mconnor on behalf of drivers for 1.9 checkin mingw builds don't get anything cool ;)

Checking in Makefile.in; /cvsroot/mozilla/toolkit/components/downloads/src/Makefile.in,v <-- Makefile.i n new revision: 1.18; previous revision: 1.17 done Checking in nsDownloadManager.cpp; /cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v <-- nsDownloadManager.cpp new revision: 1.153; previous revision: 1.152 done Checking in nsDownloadManager.h; /cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.h,v <-- ns DownloadManager.h new revision: 1.58; previous revision: 1.57 done Checked into trunk.

This change makes Sun CC unhappy. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Ports/1199883000.1199883184.10686.gz#err6 Building deps for nsDownloadManager.cpp using Sun Studio CC NEXT ERROR "nsDownloadManager.h", line 74: Error: Badly formed constant expression. "nsDownloadManager.h", line 91: Error: Badly formed constant expression. "nsDownloadManager.cpp", line 132: Error: Badly formed constant expression. "nsDownloadManager.cpp", line 841: Error: Badly formed constant expression. "nsDownloadManager.cpp", line 1784: Error: Badly formed constant expression. "nsDownloadManager.cpp", line 2088: Error: Badly formed constant expression.

+#if defined(XP_WIN) && !defined(__MINGW32__) is better than +#if defined(XP_WIN) and !defined(__MINGW32__) right?

I just hit bustage on Solaris too. This fixes it.

Solaris fix committed... Checking in toolkit/components/downloads/src/nsDownloadManager.cpp; /cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v <-- nsDownloadManager.cpp new revision: 1.154; previous revision: 1.153 done Checking in toolkit/components/downloads/src/nsDownloadManager.h; /cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.h,v <-- nsDownloadManager.h new revision: 1.59; previous revision: 1.58 done

Sorry for that (I don't have Solaris, as you can tell).