129642 - [XBLFC] XBL forms horked, related to order of pref loading

Status: RESOLVED WONTFIX

Description

[Peter Trudelle]

Using NS Build 2002030703 on Win2K:

Enable XBL form controls in Debug prefs.
Exit App.
Exit QuickLaunch (if active).
Relaunch.
go to a page with forms, such as http://bugzilla.mozilla.org/query.cgi
Expected results: See the all-too-familiar form.
Actual results: the form is unrecognizable and unusable, for instance the text
of all items in input controls is listed where the button to pop the list up
should be.

Workaround:
Switch to a new theme.  
Switch theme back
Exit & relaunch again.

Comment 1

[John Morrison]

I can do this if I enable XBL form controls, and then visiting b.m.o/query.cgi
if I have _not_ exited the application. Otherwise, if I do exit (and the exe has 
fully quit) then XBL form controls are fine when I go to that form page. 

Is there any chance that your (trudelle's) application did not fully exit (i.e., 
left a partially hung app in the background instead of fully quitting).

Comment 2

[Peter Trudelle]

I don't think so. I didn't check, but there was nothing unusual on shutting down
or starting up. Bryner says other people have seen this on *the first* restart
after switching to XBL Form Controls. After going through the workaround, they
look great.  I'll certainly add comments if I see it on subsequent sessions.

Comment 3

[Peter Trudelle]

It's baaack!  My machine hung in standby, so I had to force a restart, and when
I did the XBL controls exploded again.  Workaround still effective, though it
ends in a crash, and will be much more of a pain when we delay theme switching
until restart.

Comment 4

[Peter Trudelle]

This is happening every time my OS crashes, which considering I'm running
Windows, will probably be at least 3 times a week.  

Comment 5

[Brian Ryner (not reading)]

please attach the contents of the chrome directory in your profile directory
when you get into this state.  thanks.

Comment 6

[Peter Trudelle]

Created attachment 74907 [details]
Contents of my chrome dir while horked

This is a zipped copy of my the chrome directory in my profile, while horked.

Comment 7

[Brian Ryner (not reading)]

Hm, this all looks fine, actually (skin and locale are both selected).  Do
either of the following steps fix the problem?

- Remove XUL.mfl in your profile directory
- Rename localstore.rdf in your profile directory (this contains a lot of
persisted state, so you'll want to put it back once you've established whether
this fixes it)

Also, if you create a new profile, are the form controls ok for that profile?

Thanks.

Comment 8

[Peter Trudelle]

nsbeta1+ per Nav triage team, nav1

Comment 9

[Peter Trudelle]

After removing xul.mfl, the forms look fine on restart.

Comment 10

[John Morrison]

Removing XUL.mfl ... that's odd. We don't serialize anything related to the 
forms.jar into the fastload file, do we?

Comment 11

[Brian Ryner (not reading)]

cc'ing brendan and ben for fastload weirdness.  Can you guys offer any insight here?

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

Currently, only individual script files are serialized and deserialized. I don't
personally know how that could cause a problem. Brendan? 

Comment 13

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 14

[John Morrison]

changing to nsbeta1-, per conversation with bryner

Comment 15

[James Cox]

still seeing this, however I don't seem to be able to switch themes. I recently
upgraded my profile (was still on a users50/ style profile). maybe there is
something there causing problems?

Comment 16

[Aaron Leventhal]

Created attachment 82940 [details]
Aaronl's XBL horkage (could be different from reporter's)

Reporter: does your horkage look anything like mine?

Comment 17

[Aaron Leventhal]

Created attachment 82947 [details]
Aaronl's XBL horkage (could be different from reporter's)

Comment 18

[Aaron Leventhal]

Created attachment 82985 [details]
Five line prefs file that recreates the bug on my machine. Will create output as in attachment 82947 [details]

Comment 19

[John Morrison]

Um, please tell me that those "capability.principal.codebase..." prefs are not
part of this bug.

Comment 20

[Aaron Leventhal]

My horkage doesn't occur without the capability.principal prefs in the prefs.js.
I don't know about anyone else's

Comment 21

[Peter Trudelle]

That's the same horkage I saw.

Comment 22

[Jonas Jørgensen]

I see the same horkage, but I do NOT have any capability.principal prefs in my
prefs.js.

Comment 23

[Aaron Leventhal]

Jonas, if you move your |user_pref("nglayout.debug.enable_xbl_forms", true);|
line to the top of prefs.js and then run does fix the problem temporarily?

It does on mine. The actual position of the XBL pref affects this. Once you exit
and relaunch, it's horked again though, since the prefs.js is alphabetized as
you exit.

Comment 24

[Aaron Leventhal]

For some reason it's not using the anonymous content. So, for example, <input
type="checkbox" /> still looks like an <input> instead of <xul:image>.

However, CSSFrameConstructor knows that it *should* be parsing an HTML checkbox
as XUL in ConstructXULFrame(), so it goes there and doesn't know what to do with
the HTML tag. Kind of caught in a no man's land where neither XBL or old form
controls are being used.

Comment 25

[Aaron Leventhal]

So far, I've narrowed this down to nsCSSFrameConstuctor::ConstructFrameInternal()
When things are working, display->mBinding =
"chrome://.../checkbox.xml#checkbox" so that it can load the bindings for checkbox.
When things aren't working, display->mBinding = ""

I'm trying to find where this URL gets resolved and stored, so I can see what's
going wrong.

Comment 26

[Aaron Leventhal]

I'm working on a bug where XBL form controls are horked because of the order of
prefs in prefs.js. I have a .gif of the horked controls and the prefs.js which
causes the bug on my system.

http://bugzilla.mozilla.org/show_bug.cgi?id=129642

The specific case I'm working on invovles capability.principal prefs, but some
users experiencing the bug don't have that pref set. In my test case, the
capability.principal prefs are set in my prefs.js file. This is important
because the pref is still supported. nsScriptSecurityManager::InitPrincipals
gets called from the pref callback as prefs.js loads.

The reason for the bug is the the the chrome registry is initialized as soon as
a string bundle is required. This can happen before the entire prefs has
finished loading, because of pref callbacks. I think there must be a number of
pref call backs that use string bundles.

The string bundle initialization initializes "chrome://" URL handling, which
ends up calling the nsChromeRegistry::nsChromeRegistry() constructor. This
constructor uses the "nglayout.debug.enable_xbl_forms" pref, which hasn't been
initialized yet. Therefor and xbl-forms.css is not loaded, so there is no
anonymous content for the XBL controls.

Later, as the HTML statup page loads, nsCSSFrameConstructor checks the same XBL
form control pref, which is now initialized. It skips past the normal frame
construction in favor of using the anonymous content, of which there is
currently none.

This problem seems deeper to me than a simple fix. We could fix it in many
different places, but aren't there a lot of potenital problems of the same
variety? If pref listeners are used for statup initialization they shouldn't
need any other prefs in their calling chain, because those prefs may not be set yet.

Comment 27

[Aaron Leventhal]

Created attachment 83148 [details]
This stack shows how xbl-forms.css is prevented from loading, even with the XBL forms pref turned on

Comment 28

[Aaron Leventhal]

Aything that listens to a pref to initialize itself, can cause a cascade of
other things to initialize, each of which may require another pref for proper
operation. Unless all of the prefs are loaded, the initialization of these
services will often see the incorrect setting for the pref. In our case, the
horkage is caused because one part of the code sees one setting, and another
part of the code sees a different setting.

This is precarious. If we fix this specific incident, I'm not convinced that
similar things won't happen in other places. Perhaps we should delay pref
callbacks until all of the prefs are loaded?

Comment 29

[Jonas Jørgensen]

Re: Comment 23 --
> Jonas, if you move your |user_pref("nglayout.debug.enable_xbl_forms", true);|
> line to the top of prefs.js and then run does fix the problem temporarily?

It fixed it permanently. I am now unable to reproduce the horkage no matter
where the line is in my pref files.

Comment 30

[Jonas Jørgensen]

Sorry, it did only fix it temporarily after all. My horkage is back now.

Comment 31

[Brian Nesse (gone)]

>Perhaps we should delay pref callbacks until all of the prefs are loaded?

At one time prefs used to work this way. No callbacks were fired during loading.
This was changed at some point, I'm not sure why... maybe alecf remembers why...
cc'ing him to see.

Comment 32

[Brian Nesse (gone)]

<sigh> Really cc'ing alecf this time...

Comment 33

[Alec Flett]

Here was the problem that resulted in that decision: in some situations, the
layout engine was accessing prefs before a profile was selected - for instance,
when the profile window comes up - you need to display UI, but no per-profile
prefs can be loaded yet.

What was happening was Gecko was caching the pref (for performance reasons) but
if the pref value was different once the prefs were started, there was no way
for Gecko to know that it should start caching a new value. we decided that
given the few pref callbacks that are registered before a profile was selected,
it made sense to just tell then when the pref changes.

that said, I think it is still the right thing to do. I think if code is somehow
designed in a way where one pref value depends on another pref value, then that
whole system is screwed, and needs to be repaired. 

I don't find this precarious -  someone could say refcounting is precarious
because "what if someone forgets to Release()?!" - but we use it every day...

Comment 34

[Brian Nesse (gone)]

After talking with Aaron and Conrad about this I finally have a more clear
understanding of the underlying issue. Attempting to summarize...

When the Pref Service starts up, it reads in the default values. Once the
profile is selected, these values may change. Objects which were caching values
before the profile was loaded were not informed of the change because it was a
"load time" change rather than a "run time" change. So the Pref Service was
changed to send change notifications at "load time".

The problem here is that we are aggressively performing notifications while
reading the file, rather than caching the notifications until the file read
completes. This is opening us to a state where preferences not yet read from a
file can be accessed by those that have been read. To take this a step
farther... we could potentially have a cross file dependency as well, say
between all.js and mailnews.js (default loading) or prefs.js and user.js
(profile loading).

Alec and I have talked about this and it would seem that the most thorough fix
would be to attempt to cache callbacks across entire load groups (default and
user/profile)... I expect this just became my problem :(

Comment 35

[Conrad Carlen (not reading bugmail)]

I agree with what Brian says. Though, isn't the problem only with prefs.js whose
contents are alphabetized and rewritten dynamically? In terms of cross-file
dependencies (between all.js and mailnews.js) the contents of those are ordered
up front and this can be avoided by that ordering. So, I think eliminating
per-file ordering problems by caching the callbacks generated by that single
file load would be sufficient.

Comment 36

[Brian Nesse (gone)]

While it is true that an order can be imposed on the loading of default
preferences I believe that depending on this fact to insure that all necessary
preferences are in memory when needed is a bad idea.

I can easily imagine a case where, say, a preference in all.js triggers a
callback that is dependent on a preference defined in mailnews.js... which will
be processed later. I can envision similar scenarios where a preference in
prefs.js is dependent on a value declared in user.js.

Comment 37

[Alec Flett]

yeah, to second what brian is saying - its quite possible that components that
have their own sets of prefs (i.e. mailnews.js) have callbacks to global prefs
stored in all.js

and as it turns out, at least from what brian described, it isn't really all
that much more work (or any more work?) to hold all the callbacks for the
reading of ALL the files than to do them one-by-one...so we might as well do
them for all the files at once.

Comment 38

[Brian Nesse (gone)]

Correct. Once you've gotten to the point of caching the callbacks, the
distinction between caching for a single file vs. caching for a collection of
files is only a matter of where you "throw the switch".

Comment 39

[Michael Lefevre]

*** Bug 153308 has been marked as a duplicate of this bug. ***

Comment 40

[Vedran Miletic]

retargeting

Comment 41

[Benjamin Smedberg]

Shouldn't chromeservice register a prefs callback for
"nglayout.debug.enable_xbl_forms" anyway, when it gets initialized? Shouldn't
anyone who's caching a pref register a callback? I don't understand why we're
fixing this at the prefs level instead of the "client" code.

Wouldn't this same issue affect profile-switching anyway, in a manner that can
only be solved by the client code?

--BDS

Comment 42

[Boris Zbarsky [:bzbarsky, bz on IRC]]

This bug needs an owner....

Comment 43

[Serge Gautherie (:sgautherie)]

(Filter "spam" on 'prefs-nobody-20080612'.)

Comment 44

[Nickolay_Ponomarev]

Since bug 57209 is wontfix and this is a bug in that code per bsmedberg, marking wontfix.