576540 - gfxPlatform::Init called before prefs are read

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Since the latest hourlies gfxPlatform::Init is called before the preferences are read. For a long time we've had code in there which depends on the preferences being available, currently this makes it impossible for us to load D2D/DWrite in there based on the pref. The following stack calls init:

>	xul.dll!gfxWindowsPlatform::gfxWindowsPlatform()  Line 184	C++
 	xul.dll!gfxPlatform::Init()  Line 224 + 0x1b bytes	C++
 	xul.dll!nsThebesGfxModuleCtor()  Line 131	C++
 	xul.dll!nsComponentManagerImpl::KnownModule::Load()  Line 812 + 0xa bytes	C++
 	xul.dll!nsFactoryEntry::GetFactory()  Line 1782 + 0xb bytes	C++
 	xul.dll!nsComponentManagerImpl::CreateInstanceByContractID(const char * aContractID=0x53c53194, nsISupports * aDelegate=0x00000000, const nsID & aIID={...}, void * * aResult=0x0033ec1c)  Line 1144 + 0xc bytes	C++
 	xul.dll!CallCreateInstance(const char * aContractID=0x53c53194, nsISupports * aDelegate=0x00000000, const nsID & aIID={...}, void * * aResult=0x0033ec1c)  Line 171	C++
 	xul.dll!nsCreateInstanceByContractID::operator()(const nsID & aIID={...}, void * * aInstancePtr=0x0033ec1c)  Line 210 + 0x1b bytes	C++
 	xul.dll!nsCOMPtr<nsIDeviceContext>::assign_from_helper(const nsCOMPtr_helper & helper={...}, const nsID & aIID={...})  Line 1272 + 0x13 bytes	C++
 	xul.dll!nsCOMPtr<nsIDeviceContext>::nsCOMPtr<nsIDeviceContext>(const nsCOMPtr_helper & helper={...})  Line 645	C++
 	xul.dll!imglib_Initialize()  Line 245	C++
 	xul.dll!nsComponentManagerImpl::KnownModule::Load()  Line 812 + 0xa bytes	C++
 	xul.dll!nsFactoryEntry::GetFactory()  Line 1782 + 0xb bytes	C++
 	xul.dll!nsComponentManagerImpl::CreateInstanceByContractID(const char * aContractID=0x53d644fc, nsISupports * aDelegate=0x00000000, const nsID & aIID={...}, void * * aResult=0x0033ed24)  Line 1144 + 0xc bytes	C++
 	xul.dll!nsComponentManagerImpl::GetServiceByContractID(const char * aContractID=0x53d644fc, const nsID & aIID={...}, void * * result=0x5493ebb8)  Line 1510 + 0x34 bytes	C++
 	xul.dll!CallGetService(const char * aContractID=0x53d644fc, const nsID & aIID={...}, void * * aResult=0x5493ebb8)  Line 95	C++
 	xul.dll!CallGetService<imgILoader>(const char * aContractID=0x53d644fc, imgILoader * * aDestination=0x5493ebb8)  Line 130 + 0x12 bytes	C++
 	xul.dll!nsContentUtils::Init()  Line 413 + 0xf bytes	C++
 	xul.dll!nsLayoutStatics::Initialize()  Line 169 + 0x5 bytes	C++
 	xul.dll!Initialize()  Line 383 + 0x5 bytes	C++
 	xul.dll!nsComponentManagerImpl::KnownModule::Load()  Line 812 + 0xa bytes	C++
 	xul.dll!nsFactoryEntry::GetFactory()  Line 1782 + 0xb bytes	C++
 	xul.dll!nsComponentManagerImpl::CreateInstanceByContractID(const char * aContractID=0x54591c4c, nsISupports * aDelegate=0x00000000, const nsID & aIID={...}, void * * aResult=0x0033eed0)  Line 1144 + 0xc bytes	C++
 	xul.dll!nsComponentManagerImpl::GetServiceByContractID(const char * aContractID=0x54591c4c, const nsID & aIID={...}, void * * result=0x0033ef38)  Line 1510 + 0x34 bytes	C++
 	xul.dll!CallGetService(const char * aContractID=0x54591c4c, const nsID & aIID={...}, void * * aResult=0x0033ef38)  Line 95	C++
 	xul.dll!nsGetServiceByContractID::operator()(const nsID & aIID={...}, void * * aInstancePtr=0x0033ef38)  Line 278 + 0x13 bytes	C++
 	xul.dll!nsCOMPtr<nsIXPConnect>::assign_from_gs_contractid(nsGetServiceByContractID gs={...}, const nsID & aIID={...})  Line 1252 + 0xf bytes	C++
 	xul.dll!nsCOMPtr<nsIXPConnect>::operator=(nsGetServiceByContractID rhs={...})  Line 714	C++
 	xul.dll!nsChromeRegistry::ManifestProcessingContext::GetXPConnect()  Line 844	C++
 	xul.dll!nsChromeRegistryChrome::ManifestContent(nsChromeRegistry::ManifestProcessingContext & cx={...}, int lineno=0x00000002, char * const * argv=0x0033f100, bool platform=false, bool contentaccessible=true)  Line 909 + 0x8 bytes	C++
 	xul.dll!ParseManifest(NSLocationType aType=NS_COMPONENT_LOCATION, nsILocalFile * aFile=0x0322c808, char * buf=0x01037d68, bool aChromeOnly=false)  Line 589 + 0x3c bytes	C++
 	xul.dll!nsComponentManagerImpl::RegisterManifestFile(NSLocationType aType=NS_COMPONENT_LOCATION, nsILocalFile * aFile=0x0322c808, bool aChromeOnly=false)  Line 613 + 0x1b bytes	C++
 	xul.dll!nsComponentManagerImpl::RegisterLocation(NSLocationType aType=NS_COMPONENT_LOCATION, nsILocalFile * aLocation=0x0100a478, bool aChromeOnly=false)  Line 535 + 0x1e bytes	C++
 	xul.dll!nsComponentManagerImpl::Init()  Line 407	C++
 	xul.dll!NS_InitXPCOM2_P(nsIServiceManager * * result=0x0033fab8, nsIFile * binDirectory=0x0088b898, nsIDirectoryServiceProvider * appFileLocationProvider=0x0033fd38)  Line 497 + 0xb bytes	C++
 	xul.dll!ScopedXPCOMStartup::Initialize()  Line 1166 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=0x00000004, char * * argv=0x0088a008, const nsXREAppData * aAppData=0x0088a7b8)  Line 3337 + 0xb bytes	C++
 	firefox.exe!NS_internal_main(int argc=0x00000004, char * * argv=0x0088a008)  Line 158 + 0x12 bytes	C++
 	firefox.exe!wmain(int argc=0x00000004, wchar_t * * argv=0x00889f00)  Line 120 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 583 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 403	C

Comment 1

[Benjamin Smedberg]

So, there are several things wrong here, and you can probably fix just one of them, but they are all really bugs. Module constructors should not use the component manager. In this case it appears that three different module ctors are on the stack, and they are all doing the bad thing:

A. Layout module ctor: nsContentUtils::Init -> CallGetService<imgILoader>
B. imglib module ctor gets nsIDeviceContext
C. nsThebesGfxModuleCtor > nsGfxPlatform::Init > gfxWindowsPlatform::gfxWindowsPlatform

All of these are bad, but the gfx ctor getting prefs is probably the worst of the bunch. You don't even know that the profile has been loaded yet! It seems like you were relying on nobody happening to instnatiate any gfx component until after the profile was started. The normal/correct way to use prefs is either to wait until profile-after-change, or to add a pref observer and dynamically respond to pref changes.

The immediate cause of this is probably that xpconnect is now a part of the layout module, so we're initializing the layout module a bit earlier than we otherwise might.

Comment 3

[Joe Drew (not getting mail)]

This should be a hard blocker for beta2, because it'll break D2D for beta2 users.

Comment 5

[Benjamin Smedberg]

Attachment: Lazy-init image loader, rev. 1

Comment 6

[Joe Drew (not getting mail)]

Comment on attachment 455762 [details] [diff] [review]
Lazy-init image loader, rev. 1

Looks fine to me, but should be looked at by a content person too.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 455762 [details] [diff] [review]
Lazy-init image loader, rev. 1

sr=me, but won't this break if we have to load content (profile manager, etc) before the profile is selected?  I guess we have existing bugs on that...

Comment 8

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/f9e9f951c0ab

Yeah, apparently libpr0n and gfx constructors both do this kind of thing. Should fix, but not right now.

Comment 9

[Daniel C]

Mozilla/5.0 (X11; Linux i686; en-US; rv:2.0b2pre) Gecko/20100703 Minefield/4.0b2pre

(In reply to comment #7)
> (From update of attachment 455762 [details] [diff] [review])
> sr=me, but won't this break if we have to load content (profile manager, etc)
> before the profile is selected?  I guess we have existing bugs on that...

I can confirm that there are no profile images in the profilemanager starting with today's nightly.

Comment 10

[Alfred Kayser]

This should actually be backed out, as it caused a serious regression: see bug
576813, until the patch is improved to not cause bug 576813.
Note that comment #7 already raises some concern that this may break things?
Bug 576813 conforms that this patch does break things!
Comment #9 then even confirms that images are now missing.

Comment 11

[Benjamin Smedberg]

I can't reproduce the profile manager report in comment #9, and don't see a bug report about it. Comment 7 is incorrect (because the profile manager runs in a separate process). I'm happy to look at fixing bug 576813, but we should not back this out.

Comment 12

[Daniel C]

(In reply to comment #11)
> I can't reproduce the profile manager report in comment #9, and don't see a bug
> report about it.

Filed bug 576872 based on comment 9.

Comment 13

[Matt Brubeck (:mbrubeck)]

Bug 577076 (some chrome images not loading in Fennec) appears to be caused by this change too, based on testing with hg bisect.