Open Bug 549090 Opened 13 years ago Updated 5 years ago

gfxWindowsPlatform can be called before user prefs are loaded

Categories

(Core :: Widget: Win32, defect, P3)

defect

Tracking

()

People

(Reporter: rain1, Unassigned)

Details

app-startup happens at [1], while user prefs are read at profile-do-change [2], which happens after [1], at [3]. This means that user prefs haven't been read during app-startup.

Now several addons register themselves to do stuff at app-startup, and if one of them causes e.g. gfxPlatform to be initialized (as Adblock Plus does), gfxPlatform doesn't get the user value. This probably wasn't a problem in the past, but is now one because of [4]. The net effect is that enabling Adblock Plus currently causes DirectWrite/Direct2D to be shut off.

[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3322
[2] http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/nsPrefService.cpp#167
[3] http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3356
[4] http://mxr.mozilla.org/comm-central/source/mozilla/gfx/thebes/src/gfxWindowsPlatform.cpp#137
The offending line is <https://hg.adblockplus.org/adblockplus/file/99f42cf05b3c/chrome/content/elemhide.js#l30>, which causes the entire layout module to be initialized -- it's fixed in ABP trunk though.
Stylish apparently has the same behaviour (at least it's triggering the same D2D glitch).
Ugh, it's even worse for debug builds -- Adblock Plus causes gfxPlatform to be initialized during autoreg, at <http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3286>.
Isn't that just a bug in the directwrite code?  It should be observing the pref, not just getting it once...

Note that you can't read prefs until profile-change, because until then you don't know what profile to read prefs from!
Component: Preferences: Backend → Widget: Win32
QA Contact: preferences-backend → win32
Summary: app-startup happens before user preferences have been read → direct2d pref needs to be live
(In reply to comment #4)
> Isn't that just a bug in the directwrite code?  It should be observing the
> pref, not just getting it once...
> 
> Note that you can't read prefs until profile-change, because until then you
> don't know what profile to read prefs from!

We cannot, at the moment change font backend while running. (this will become possible in the relatively near future as fonts are refactored)

We should be able to change render mode while running, but that's simply not how the code in gfxWindowsPlatform is setup. Note this is not a DirectWrite thing, the code reading the render-mode value predates either DWrite or D2D.
Well, as I see it we have several options:

1)  Reset the render mode on profile change (if possible).
2)  Mark this bug invalid and tell extension to not muck with layout before
    profile change.

Frankly, I thought we were telling them #2 already...
gfxPlatform code is dependent on prefs being available, I don't think that's a simple thing to change (or necessarily desirable).

bz, is there a general mechanism for denoting/enforcing initialization order in cases like this?  And why/how are extensions loading before basic initialization occurs?
There is at least one non-extension case where we show UI before loading profile preferences, which is profile migration. In this case we normally won't have a user-set value for this specific pref (because the profile is just being created).

Note that you can read from prefs at pretty much any time after XPCOM registration: it just won't give you user-set values until after we've loaded the profile.
> bz, is there a general mechanism for denoting/enforcing initialization order

Not really, no.  I suppose we could fail layout init before profile-before-change, but then profile migration wouldn't work per comment 8.

> And why/how are extensions loading before basic initialization occurs?

I have no idea.  Benjamin might.
Extensions which do old-style profile roaming show UI before we load the profile, as does our own profile-migration UI and the safe-mode UI. You can't enforce that load order without breaking some important stuff!
This code is in the gfxWindowsPlatform constructor, it could easily live in the Init() method.  But if extensions need to lay out text before the platform object is initialized, lots of things aren't going to work correctly, not just DWrite.  The Init() process is relatively heavy (we initialize the font list there) so we don't want to do this twice if we want decent startup times.
I don't think the startup time impact should be a big concern. This will only happen for users who have the pref set to a non-default value, which is probably going to be a relatively small number, *and* who do something with an extension or profile migration that triggers early gfxPlatform init.
We should defer all deferable work (like the processing of fonts for CMAP construction) until user-prefs have loaded, certainly.
Note that Adblock Plus behavior changed because of bug 542111. While I'm not going to change it back of course, I consider this a platform issue - the code should either be more robust or there should be a big warning somewhere telling you that getting stylesheet service too early is a no-no.
What are the effects of delaying "processing of fonts for CMAP construction" until after user prefs have loaded? Wouldn't that break UI which needs to show before that point?
(In reply to comment #15)
> What are the effects of delaying "processing of fonts for CMAP construction"
> until after user prefs have loaded? Wouldn't that break UI which needs to show
> before that point?

No, it shouldn't. We load all cmaps lazily (after startup) to improve font fallback performance, which is what Roc is referring to, but we also load cmap tables as needed for the individual fonts we're actually using.
No longer blocks: 527707
Summary: direct2d pref needs to be live → gfxWindowsPlatform can be called before user prefs are loaded
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.