Closed Bug 1341414 Opened 3 years ago Closed 3 years ago

Functions like GetBoolVarCache shouldn't assert when used early in startup

Categories

(Core :: DOM: Content Processes, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: billm, Assigned: blassey)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Right now if you use functions like GetBoolVarCache early in startup, you can trigger this assertion:
http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/modules/libpref/prefapi.cpp#778

We should change GetBoolVarCache so that it doesn't look at the pref in the content process until prefs have been received from the parent. Hopefully that won't break any code that reads the cached values early in startup.
Attachment #8839786 - Flags: review?(wmccloskey)
Comment on attachment 8839786 [details] [diff] [review]
watched_prefs.patch

Review of attachment 8839786 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentPrefs.cpp
@@ +3,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "ContentPrefs.h"

Can you add a comment about needing DOM review to add prefs?
Attachment #8839786 - Flags: review?(wmccloskey) → review+
Pushed by blassey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac8a51344b66
Functions like GetBoolVarCache shouldn't assert when used early in startup r=billm
Pushed by blassey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25ce82ad3b4b
reverting change to ContentPrefs.cpp to fix orange a11y tests r=orange
It looks like this merged to m-c even though for some reason the bug wasn't updated.
https://hg.mozilla.org/mozilla-central/rev/ac8a51344b66
https://hg.mozilla.org/mozilla-central/rev/25ce82ad3b4b
Landed 12 hours ago - it probably didn't get marked because the second patch begins with "reverting".
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
What is ContentPrefs.cpp about and when is one supposed to add prefs there?
Flags: needinfo?(wmccloskey)
I added a comment to this file just now in bug 1356414:

 * This is the list of preferences that are sent to the content process on
 * startup. Only prefs that are required immediately upon startup should be
 * listed here. The first IPC message received in the content process will
 * contain all the other prefs. Prefs should only be listed here if they must be
 * read before the first IPC message is received.
Flags: needinfo?(wmccloskey)
You need to log in before you can comment on or make changes to this bug.