Remove gfxPrefs in favour of StaticPrefs
Categories
(Core :: Graphics, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: jya, Assigned: jya)
References
(Blocks 2 open bugs, Regressed 1 open bug)
Details
Attachments
(28 files, 3 obsolete files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1550422 - P0. Use X11UndefineNone.h to avoid conflicts with the macro None from X11/X.h. r?karlt
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
gfxPrefs was once required as Preferences weren't available outside the main thread and weren't thread-safe.
StaticPrefs has fixed all those issues, it is however not available in the GPU process.
Having a unique set of preferences make the code clearer and is easier to maintain.
So if we make StaticPrefs available in the GPU process, we will no longer need gfxPrefs.
In the past we've had several bugs where gfxPrefs weren't used where they should have been, in particular on the Windows media decoder, as some (but not all) code runs in the GPU process.
Comment 1•5 years ago
|
||
Comment 2•5 years ago
|
||
Note that GPU process access is not the only thing that gfxPrefs does that StaticPrefs doesn't.
gfxPrefs also allows override prefs, for example this one and has setter methods (see these search results for example). There's also the distinction between "Once" prefs (which don't change their value after startup) and "Live" prefs (which do) and from my admittedly quick glance at StaticPrefs I'm not totally sure if that exposes similar functionality.
That being said, I'd be happy to see gfxPrefs merged into StaticPrefs - I just want to point out it may not be quite so simple as it might seem.
Updated•5 years ago
|
Assignee | ||
Comment 3•5 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
gfxPrefs also allows override prefs, for example this one and has setter methods (see these search results for example). There's also the distinction between "Once" prefs (which don't change their value after startup) and "Live" prefs (which do) and from my admittedly quick glance at StaticPrefs I'm not totally sure if that exposes similar functionality.
Yes, I'm aware of those.
StaticPrefs for now can only imitate the "Live" range.
The Skip is interesting, but I actually question if we actually need it.
For the Once, they more appear to be an optimisation in order to not unnecessarily update them. At least that's how I've used them in the past. Those are used during startup and are also read once only. So I believe it could go.
The one you linked to, is actually the only one giving me trouble.
gfxPrefs has been rather fragile in regards to when and where it need to be initialised. Just look at the number of various calls to gfxPrefs::GetSingleton().
The core issue here is that gfxPrefs is initialized after gfxVars, while Preferences is initialised very early on.
Rather than modifying StaticPrefs and Preferences, I believe it's better to deal with that where it's used instead.
Assignee | ||
Comment 4•5 years ago
|
||
Assignee | ||
Comment 5•5 years ago
|
||
Depends on D30586
Assignee | ||
Comment 6•5 years ago
|
||
Depends on D30587
Assignee | ||
Comment 7•5 years ago
|
||
This will allow to remove gfxPrefs later. On Windows in particular, the need to decide gfxPrefs vs StaticPrefs for the WMF decoders has caused several bugs in the past.
We will remove the confusion as a consequence.
Depends on D30588
Assignee | ||
Comment 8•5 years ago
|
||
Depends on D30589
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 9•5 years ago
|
||
No need to keep the object alive.
Assignee | ||
Comment 10•5 years ago
|
||
Will be needed to remove gfxPrefs in favor of StaticPrefs
Depends on D31014
Assignee | ||
Comment 11•5 years ago
|
||
Depends on D31015
Assignee | ||
Comment 12•5 years ago
|
||
Depends on D31016
Updated•5 years ago
|
Assignee | ||
Comment 13•5 years ago
|
||
This is used by some gfx code and is required to convert gfxPrefs into StaticPrefs.
The setter only modify the value of the StaticPref in the current process and doesn't propagate to others.
Assignee | ||
Comment 14•5 years ago
|
||
Depends on D31254
Assignee | ||
Comment 15•5 years ago
|
||
gfxPrefs Live preferences are almost identical to StaticPrefs.
We leave aside for now those that set a custom change callback as this feature isn't yet supported in StaticPrefs.
Depends on D31255
Assignee | ||
Comment 16•5 years ago
|
||
This works identically to what gfxPrefs UpdatePolicy offers.
Depends on D31256
Assignee | ||
Comment 17•5 years ago
|
||
This is required to emulate some gfxPrefs functionalities.
Depends on D31257
Assignee | ||
Comment 18•5 years ago
|
||
Depends on D31258
Hi Jean-Yves, as was pointed out by Jonathan Kew on the dev-platform email thread, we should not land this fix before May 20th (upcoming Merge day). Please confirm.
Comment 20•5 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #3)
gfxPrefs also allows override prefs, for example this one
The one you linked to, is actually the only one giving me trouble.
Now that containerless scrolling is enabled by default on all platforms, we should be able to make layout.scroll.root-frame-containers
a regular pref (as opposed to an override pref).
I also notice that currently it's the only override pref, so perhaps we can follow that up by removing the "override pref" mechanism altogether, and solve this headache for you. (Kats, any thoughts on that? Do you envision other / future uses for the override pref mechanism?)
Comment 21•5 years ago
|
||
One thing that's unfortunate about StaticPrefs as compared to gfxPrefs is that StaticPrefs uses the "header file that's included in a non-global context" pattern with StaticPrefList.h. Some editors (like Eclipse) have a hard time grokking this pattern, leading to a regression in the editing experience.
Comment 22•5 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #20)
I also notice that currently it's the only override pref, so perhaps we can follow that up by removing the "override pref" mechanism altogether, and solve this headache for you. (Kats, any thoughts on that? Do you envision other / future uses for the override pref mechanism?)
I'd prefer to keep it as I do expect it will come in handy again. But if it's too ugly/hacky in StaticPrefs I wouldn't fight too hard to keep it.
Assignee | ||
Comment 23•5 years ago
|
||
(In reply to Ritu Kothari (:ritu) from comment #19)
Hi Jean-Yves, as was pointed out by Jonathan Kew on the dev-platform email thread, we should not land this fix before May 20th (upcoming Merge day). Please confirm.
I can confirm that it will be the case yes.
Assignee | ||
Comment 24•5 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #20)
(In reply to Jean-Yves Avenard [:jya] from comment #3)
gfxPrefs also allows override prefs, for example this one
The one you linked to, is actually the only one giving me trouble.
Now that containerless scrolling is enabled by default on all platforms, we should be able to make
layout.scroll.root-frame-containers
a regular pref (as opposed to an override pref).
Is it?
Damned, I had missed that it was now set to 0 in all.js.
I also notice that currently it's the only override pref, so perhaps we can follow that up by removing the "override pref" mechanism altogether, and solve this headache for you. (Kats, any thoughts on that? Do you envision other / future uses for the override pref mechanism?)
I hope not, I hate to have coded something for nothing :)
Assignee | ||
Comment 25•5 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #21)
One thing that's unfortunate about StaticPrefs as compared to gfxPrefs is that StaticPrefs uses the "header file that's included in a non-global context" pattern with StaticPrefList.h. Some editors (like Eclipse) have a hard time grokking this pattern, leading to a regression in the editing experience.
Indeed, the implementation of StaticPrefs is not the most elegant in that regards, but unfortunately I understand why it was done that way.
Right now the static definition of the StaticPrefs container is done in Preferences.cpp; we could move to what gfxPrefs did.
I'll see if I can improve things a bit here.
Comment 26•5 years ago
|
||
kmag, I'd be interested to hear your thoughts on P13, particularly in relation to where I said "I admit I am having a hard time...".
Assignee | ||
Comment 27•5 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #26)
kmag, I'd be interested to hear your thoughts on P13, particularly in relation to where I said "I admit I am having a hard time...".
I commented on the review but maybe it deserves more details.
There are two cases we need to handle:
- StaticPrefs is being initialised on the parent process
- StaticPrefs is being initialised on other processes.
Before P13:
during 1), the StaticPrefs is set to its default value, as defined in StaticPrefList.h. Then for each StaticPrefs we call Preferences::AddXXVarCache(aCache, aName, aDefaultValue, true); (as aIsStartup is always true really, it's only ever set to false if we reset the pref).
AddXXVarCache will also set the pref if it's overridden in all.js which always trigger a call to the callback. As such, the StaticPrefs will ultimately be set to the custom value if set.
during 2), The preference isn't set, but AddXXVarCache is called, which as described above will set the StaticPrefs to the preference value as it was set during case 1).
Now after P13: For the Live pref, we do exactly as above.
For "Once" pref however, we do things differently:
-
We set the pref to the default value, and that is it. We then read the preference custom value. Once that is done, we call StaticPrefs::InitOncePrefs which we read the value of each preference and update the values of each Once pref.
-
Now when Preferences::AddXXVarCache is called, we need to read the preference value. It has been set to the correct value by the parent process when launching the process. Preferences are up to date.
Now there is a flaw in that logic, but that existed with the gfxPrefs. The Once StaticPrefs may not be updated as the main preference is changed, but it is set to last update of the preference as it was read when the process was started.
So if the user do modify the pref, open a new window which triggers the launch of a new content process, that content process StaticPrefs will not have the original preference as seen by the content process when it first launch.
I don't think it matters. "Once" here means that its value will not change during the lifetime of the current process, not the lifetime of the parent process.
Hope that clarifies things.
Updated•5 years ago
|
Assignee | ||
Comment 28•5 years ago
|
||
Depends on D31259
Assignee | ||
Comment 29•5 years ago
|
||
Fly-by fix, we make LoggingPrefs::sGfxLogLevel as it is written on the main thread but read on different threads.
Depends on D31459
Assignee | ||
Comment 30•5 years ago
|
||
Depends on D31460
Assignee | ||
Comment 31•5 years ago
|
||
StaticPrefs doesn't support nsCString type and the changes required to support this would be rather big. Seeing that there was only a single gfxPrefs using this, and this is a "Once" pref ; we move it to gfxVars instead.
Depends on D31461
Assignee | ||
Comment 32•5 years ago
|
||
Depends on D31462
Assignee | ||
Comment 33•5 years ago
|
||
Depends on D31463
Assignee | ||
Comment 34•5 years ago
|
||
Following the shift in unified build setup following the removal of gfxPrefs.{cpp,c} we hit this error.
Depends on D31464
Assignee | ||
Comment 35•5 years ago
|
||
Depends on D31465
Assignee | ||
Comment 36•5 years ago
|
||
Depends on D31466
Assignee | ||
Comment 37•5 years ago
|
||
And with some tidying some comments and removing stray #include "gfxPrefs.h"
Depends on D31467
Updated•5 years ago
|
Assignee | ||
Comment 39•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 40•5 years ago
|
||
Prefs aren't yet sorted as they should be, this will be done in bug 1552643
Depends on D31468
Assignee | ||
Comment 41•5 years ago
|
||
Skip and Once prefs are only ever written on the main thread once. There's no need to make those prefs atomic.
Depends on D31731
Assignee | ||
Comment 42•5 years ago
|
||
And set the underlying preference. StatiPrefs::Set becomes a convenience access to the original preference which is what gfxPrefs was actually doing.
Depends on D31732
Assignee | ||
Comment 43•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 44•5 years ago
|
||
:kmag, your r- an earlier revision which is now blocking this queue.
Could you have a look again?
Assignee | ||
Comment 45•5 years ago
|
||
Updated•5 years ago
|
Comment 46•5 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/02fc78890032 P0. Use X11UndefineNone.h to avoid conflicts with the macro None from X11/X.h. r=karlt https://hg.mozilla.org/integration/autoland/rev/371b4da5b771 P00. Fix Windows compilation. r=dmajor https://hg.mozilla.org/integration/autoland/rev/f45f471a1a40 P1. Add GPU process selector to prefs module. r=spohl,mattwoodrow https://hg.mozilla.org/integration/autoland/rev/ca47ef6c59f7 P2. add shared pref serializer/deserializer to GPU process. r=kmag https://hg.mozilla.org/integration/autoland/rev/ba46b53643ec P3. Fix typo when filtering preferences to sync. r=njn https://hg.mozilla.org/integration/autoland/rev/952ddac02972 P4. Sync preferences when they changed. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/2db647dcdf1c P5. Sync preferences with RDD process when then changed. r=mattwoodrow,mjf https://hg.mozilla.org/integration/autoland/rev/a38796266b28 P6. Release object early when error. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/60fe635ec2c9 P7. Add VR process selector to prefs module. r=spohl https://hg.mozilla.org/integration/autoland/rev/10c153ddbaea P8. Add shared pref serializer/deserializer to VR process. r=kmag,daoshengmu https://hg.mozilla.org/integration/autoland/rev/9c0c9e80f309 P9. Sync preferences in VR process when they change. r=daoshengmu https://hg.mozilla.org/integration/autoland/rev/86a8ba1b755c P10. Define StaticPrefs setter. r=njn https://hg.mozilla.org/integration/autoland/rev/ea64b4d8d4ff P11. Add atomic float preferences support. r=njn https://hg.mozilla.org/integration/autoland/rev/2fef10a7cce5 P12. Convert Live gfxPrefs into StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/f781d415cef6 P13. Add Skip, Once and Live cached preference policy. r=njn,kmag https://hg.mozilla.org/integration/autoland/rev/e1b7abc99ae9 P14. Add GetXXName and GetXXDefault methods to StaticPrefs. r=njn https://hg.mozilla.org/integration/autoland/rev/898ed02804fe P15. Move Skip and Once gfxPrefs to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/7a930fc51125 P17. Convert gfxPrefs::GfxLoggingLevel to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/e2938a444234 P18. Convert gfxPrefs::LayoutFrameRate to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/285fa46e4f26 P19. Convert gfxPrefs::LayersWindowRecordingPath to gfxVars. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/1c90b9cb3ad4 P20. Add missing namespace. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/a1961a51ae44 P21. Remove gfxPref sync with VR process. r=daoshengmu https://hg.mozilla.org/integration/autoland/rev/ca67e8e45262 P22. Remove gfxPref sync with GPU process. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/6ada1116b241 P23. Remove now unused gfxPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/059cff1a3dde P24. Fix style of StaticPrefs. r=njn https://hg.mozilla.org/integration/autoland/rev/b6861d3badf8 P25. Don't make prefs unnecessarily atomic. r=njn https://hg.mozilla.org/integration/autoland/rev/529f5be01ab9 P26. Make setter only usable on main process. r=njn
Assignee | ||
Comment 47•5 years ago
|
||
Comment 48•5 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5dd10a365ba9 P27. Fix long line. r=gerald
Comment 49•5 years ago
|
||
Backed out 28 changesets (bug 1550422) for marionette AssertionError and failing browser_policy_hardware_acceleration.js on a CLOSED TREE
Backout link: https://hg.mozilla.org/integration/autoland/rev/5a60b9fe09370c269227df87a525d9f098e54c37
**Push with failures:**https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=529f5be01ab9b349f552e09e02a4cea40ba0a4b5&selectedJob=247878962
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247878962&repo=autoland&lineNumber=14575
Log snippet:
01:52:17 INFO - TEST-START | browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js
01:52:17 INFO - GECKO(2171) | Chrome file doesn't exist: /Users/cltbld/tasks/task_1558575477/build/tests/mochitest/browser/browser/components/enterprisepolicies/tests/browser/hardware_acceleration/head.js
01:52:17 INFO - TEST-INFO | started process screencapture
01:52:17 INFO - TEST-INFO | screencapture: exit 0
01:52:17 INFO - Buffered messages logged at 01:52:16
01:52:17 INFO - Entering test bound test_policy_hardware_acceleration
01:52:17 INFO - Buffered messages finished
01:52:17 INFO - TEST-UNEXPECTED-FAIL | browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js | Hardware acceleration disabled - Got OpenGL, expected Basic
01:52:17 INFO - Stack trace:
01:52:17 INFO - chrome://mochikit/content/browser-test.js:test_is:1325
01:52:17 INFO - chrome://mochitests/content/browser/browser/components/enterprisepolicies/tests/browser/hardware_acceleration/browser_policy_hardware_acceleration.js:test_policy_hardware_acceleration:8
01:52:17 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
01:52:17 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
01:52:17 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
01:52:17 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
01:52:17 INFO - Leaving test bound test_policy_hardware_acceleration
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247878405&repo=autoland&lineNumber=2016
Log snippet:
01:31:50 INFO - 1558575110877 Marionette DEBUG 3 -> [0,11,"WebDriver:GetPageSource",{}]
01:31:50 ERROR - TEST-UNEXPECTED-FAIL | gfx\tests\marionette\test_pref_rollout_workaround.py WrPrefRolloutWorkAroundTestCase.test_wr_rollout_workaround_on_qualifying_hw | AssertionError: Should start out as WR opt-in
01:31:50 INFO - Traceback (most recent call last):
01:31:50 INFO - File "Z:\task_1558571672\build\venv\lib\site-packages\marionette_harness\marionette_test\testcases.py", line 159, in run
01:31:50 INFO - testMethod()
01:31:50 INFO - File "Z:\task_1558571672\build\tests\marionette\tests\gfx\tests\marionette\test_pref_rollout_workaround.py", line 82, in test_wr_rollout_workaround_on_qualifying_hw
01:31:50 INFO - self.assertEqual(status, 'opt-in', 'Should start out as WR opt-in')
01:31:50 INFO - TEST-INFO took 5328ms
01:31:50 INFO - 1558575110887 Marionette DEBUG 3 <- [1,11,null,{"value":"<html><head></head><body></body></html>"}]
01:31:50 INFO - 1558575110889 Marionette DEBUG 3 -> [0,12,"Marionette:SetContext",{"value":"chrome"}]
01:31:50 INFO - 1558575110890 Marionette DEBUG 3 <- [1,12,null,{"value":null}]
01:31:50 INFO - 1558575110894 Marionette DEBUG 3 -> [0,13,"WebDriver:DeleteSession",{}]
01:31:50 INFO - SUMMARY
01:31:50 INFO - -------
01:31:50 INFO - passed: 1
01:31:50 INFO - 1558575110897 Marionette DEBUG 3 <- [1,13,null,{"value":null}]
01:31:50 INFO - failed: 1
01:31:50 INFO - todo: 0
01:31:50 INFO - FAILED TESTS
01:31:50 INFO - -------
01:31:50 INFO - test_pref_rollout_workaround.py test_pref_rollout_workaround.WrPrefRolloutWorkAroundTestCase.test_wr_rollout_workaround_on_qualifying_hw
01:31:50 INFO - 1558575110900 Marionette DEBUG Closed connection 3
01:31:50 INFO - SUITE-END | took 22s
01:31:51 INFO - [Parent 9604, Main Thread] WARNING: IPC message discarded: actor cannot send: file z:/build/build/src/ipc/glue/ProtocolUtils.cpp, line 608
01:31:51 INFO - [Parent 9604, Main Thread] WARNING: IPC message discarded: actor cannot send: file z:/build/build/src/ipc/glue/ProtocolUtils.cpp, line 608
01:31:51 INFO - [Parent 9604, Main Thread] WARNING: IPC message discarded: actor cannot send: file z:/build/build/src/ipc/glue/ProtocolUtils.cpp, line 608
01:31:52 ERROR - Return code: 10
Assignee | ||
Comment 50•5 years ago
|
||
Additionally, rename some pref constants to improve code clarity as we can no longer rely on using the StaticPrefs accessor.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 51•5 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d7ba4a18dd54 P0. Use X11UndefineNone.h to avoid conflicts with the macro None from X11/X.h. r=karlt https://hg.mozilla.org/integration/autoland/rev/54c85ac75dd0 P00. Fix Windows compilation. r=dmajor https://hg.mozilla.org/integration/autoland/rev/132e0b8d8468 P1. Add GPU process selector to prefs module. r=spohl,mattwoodrow https://hg.mozilla.org/integration/autoland/rev/7d3c2d486706 P2. add shared pref serializer/deserializer to GPU process. r=kmag https://hg.mozilla.org/integration/autoland/rev/72e5de040dda P3. Fix typo when filtering preferences to sync. r=njn https://hg.mozilla.org/integration/autoland/rev/ab4c4e806977 P4. Sync preferences when they changed. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/3aa72f89e295 P5. Sync preferences with RDD process when then changed. r=mattwoodrow,mjf https://hg.mozilla.org/integration/autoland/rev/2ba22cddeb6f P6. Release object early when error. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/c20f66494d69 P7. Add VR process selector to prefs module. r=spohl https://hg.mozilla.org/integration/autoland/rev/6dc82f88333d P8. Add shared pref serializer/deserializer to VR process. r=kmag,daoshengmu https://hg.mozilla.org/integration/autoland/rev/f92f2cc29cb1 P9. Sync preferences in VR process when they change. r=daoshengmu https://hg.mozilla.org/integration/autoland/rev/2f328853c1ab P10. Define StaticPrefs setter. r=njn https://hg.mozilla.org/integration/autoland/rev/097091082423 P11. Add atomic float preferences support. r=njn https://hg.mozilla.org/integration/autoland/rev/e0cd10d35327 P12. Convert Live gfxPrefs into StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/77d2f80257d1 P13. Add Skip, Once and Live cached preference policy. r=njn,kmag https://hg.mozilla.org/integration/autoland/rev/91c3acdb2454 P14. Add GetXXName and GetXXDefault methods to StaticPrefs. r=njn https://hg.mozilla.org/integration/autoland/rev/75b04de7e99c P15. Move Skip and Once gfxPrefs to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/ade7384c6186 P17. Convert gfxPrefs::GfxLoggingLevel to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/1033546224a7 P18. Convert gfxPrefs::LayoutFrameRate to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/f855f9309c8b P19. Convert gfxPrefs::LayersWindowRecordingPath to gfxVars. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/9ea6da7a74ec P20. Add missing namespace. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/0edd264464c2 P21. Remove gfxPref sync with VR process. r=daoshengmu https://hg.mozilla.org/integration/autoland/rev/6f0997976944 P22. Remove gfxPref sync with GPU process. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/d98dfc565927 P23. Remove now unused gfxPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/69df7818d4a3 P24. Fix style of StaticPrefs. r=njn https://hg.mozilla.org/integration/autoland/rev/3b70307c0db5 P25. Don't make prefs unnecessarily atomic. r=njn https://hg.mozilla.org/integration/autoland/rev/a16295296035 P26. Make setter only usable on main process. r=njn https://hg.mozilla.org/integration/autoland/rev/0b4029671710 P27. Do not set WebRender preferences as code don't expect them to exists. r=cpearce
Comment 52•5 years ago
|
||
Backed out 31 changesets (bug 1552643, bug 1550422) for xpcshell crash on a CLOSED TREE. Bug 1552643 was also backed out because backing out only this bug did not apply cleanly (4 out of 59 hunks FAILED -- saving rejects to file modules/libpref/init/StaticPrefList.h.rej)
Backout link: https://hg.mozilla.org/integration/autoland/rev/af54b2de7028db03f42207598f7a0b4ba81e262f
**Push with failures:**https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=0b40296717100fa26c8f7bdea8e365eeb523cfbd&selectedJob=248315209
Log link for xpcshell crash: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=248315209&repo=autoland&lineNumber=1796
Log snippet for xpcshell crash:
03:09:40 INFO - TEST-START | storage/test/unit/test_connection_asyncClose.js
03:09:40 INFO - rmtree() failed for "('c:\users\task_1558744571\appdata\local\temp\xpc-plugins-sgnbo7',)". Reason: The process cannot access the file because it is being used by another process (13). Retrying...
03:09:40 INFO - mozcrash Saved minidump as Z:\task_1558744571\build\blobber_upload_dir\21b3adfd-f06d-4bd8-ba2d-018490a9fa86.dmp
03:09:40 WARNING - PROCESS-CRASH | services/sync/tests/unit/test_status_checkSetup.js | application crashed [@ mozilla::widget::WinCompositorWindowThread::ShutDown()]
03:09:40 INFO - Crash dump filename: c:\users\task_1558744571\appdata\local\temp\xpc-other-hs8xpd\21b3adfd-f06d-4bd8-ba2d-018490a9fa86.dmp
03:09:40 INFO - Operating system: Windows NT
03:09:40 INFO - 10.0.17134
03:09:40 INFO - CPU: amd64
03:09:40 INFO - family 6 model 85 stepping 3
03:09:40 INFO - 8 CPUs
03:09:40 INFO - GPU: UNKNOWN
03:09:40 INFO - Crash reason: EXCEPTION_BREAKPOINT
03:09:40 INFO - Crash address: 0x7ff838f0374b
03:09:40 INFO - Assertion: Unknown assertion type 0x00000000
03:09:40 INFO - Process uptime: 2 seconds
03:09:40 INFO - Thread 0 (crashed)
03:09:40 INFO - 0 xul.dll!mozilla::widget::WinCompositorWindowThread::ShutDown() [WinCompositorWindowThread.cpp:0b40296717100fa26c8f7bdea8e365eeb523cfbd : 54 + 0x0]
03:09:40 INFO - rax = 0x00007ff83d17b19f rdx = 0x00007ff8748aa640
03:09:40 INFO - rcx = 0x00007ff8713e5b10 rbx = 0x0000000000000003
03:09:40 INFO - rsi = 0x00000218c8e05210 rdi = 0x0000007cc15feab8
03:09:40 INFO - rbp = 0x0000000000000000 rsp = 0x0000007cc15fe880
03:09:40 INFO - r8 = 0x0000007cc15f89d8 r9 = 0x0000007cc15f9ff0
03:09:40 INFO - r10 = 0x0000000000000000 r11 = 0x0000007cc15f9f00
03:09:40 INFO - r12 = 0x00000218c8e2d130 r13 = 0x0000000000000001
03:09:40 INFO - r14 = 0x00007ff83dfa1698 r15 = 0x0000000000000003
03:09:40 INFO - rip = 0x00007ff838f0374b
03:09:40 INFO - Found by: given as instruction pointer in context
03:09:40 INFO - 1 xul.dll!mozilla::gfx::GPUParent::ActorDestroy(mozilla::ipc::IProtocol::ActorDestroyReason) [GPUParent.cpp:0b40296717100fa26c8f7bdea8e365eeb523cfbd : 518 + 0x5]
03:09:40 INFO - rbx = 0x0000000000000003 rbp = 0x0000000000000000
03:09:40 INFO - rsp = 0x0000007cc15fe920 r12 = 0x00000218c8e2d130
03:09:40 INFO - r13 = 0x0000000000000001 r14 = 0x00007ff83dfa1698
03:09:40 INFO - r15 = 0x0000000000000003 rip = 0x00007ff83667d13e
03:09:40 INFO - Found by: call frame info
03:09:40 INFO - 2 xul.dll!mozilla::ipc::IProtocol::DestroySubtree(mozilla::ipc::IProtocol::ActorDestroyReason) [ProtocolUtils.cpp:0b40296717100fa26c8f7bdea8e365eeb523cfbd : 699 + 0xc]
03:09:40 INFO - rbx = 0x0000000000000003 rbp = 0x0000000000000000
03:09:40 INFO - rsp = 0x0000007cc15fea90 r12 = 0x00000218c8e2d130
03:09:40 INFO - r13 = 0x0000000000000001 r14 = 0x00007ff83dfa1698
03:09:40 INFO - r15 = 0x0000000000000003 rip = 0x00007ff8358544fc
03:09:40 INFO - Found by: call frame info
03:09:40 INFO - 3 xul.dll!mozilla::gfx::PGPUParent::OnChannelClose() [PGPUParent.cpp: : 1546 + 0xa]
03:09:40 INFO - rbx = 0x0000000000000003 rbp = 0x0000000000000000
03:09:40 INFO - rsp = 0x0000007cc15feb00 r12 = 0x00000218c8e2d130
03:09:40 INFO - r13 = 0x0000000000000001 r14 = 0x00007ff83dfa1698
03:09:40 INFO - r15 = 0x0000000000000003 rip = 0x00007ff835a49892
03:09:40 INFO - Found by: call frame info
03:09:40 INFO - 4 xul.dll!nsresult mozilla::detail::RunnableMethodImpl<mozilla::ipc::MessageChannel ,void (mozilla::ipc::MessageChannel::)(),0,mozilla::RunnableKind::Cancelable>::Run() [nsThreadUtils.h:0b40296717100fa26c8f7bdea8e365eeb523cfbd : 1174 + 0xa]
03:09:40 INFO - rbx = 0x0000000000000003 rbp = 0x0000000000000000
03:09:40 INFO - rsp = 0x0000007cc15feb30 r12 = 0x00000218c8e2d130
03:09:40 INFO - r13 = 0x0000000000000001 r14 = 0x00007ff83dfa1698
03:09:40 INFO - r15 = 0x0000000000000003 rip = 0x00007ff83585c3da
03:09:40 INFO - Found by: call frame info
03:09:40 INFO - 5 xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:0b40296717100fa26c8f7bdea8e365eeb523cfbd : 1176 + 0x6]
03:09:40 INFO - rbx = 0x0000000000000003 rbp = 0x0000000000000000
03:09:40 INFO - rsp = 0x0000007cc15feb60 r12 = 0x00000218c8e2d130
03:09:40 INFO - r13 = 0x0000000000000001 r14 = 0x00007ff83dfa1698
03:09:40 INFO - r15 = 0x0000000000000003 rip = 0x00007ff835098e40
03:09:40 INFO - Found by: call frame info
Log snippet for Assertion failure:
03:09:40 INFO - PID 2096 | JavaScript error: resource://gre/modules/osfile/osfile_async_front.jsm, line 400: Error: OS.File has been shut down. Rejecting post to DirectoryIterator_prototype_close
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | ###!!! [Child][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
03:09:40 INFO - PID 2096 | Assertion failure: sWinCompositorWindowThread, at z:/build/build/src/widget/windows/WinCompositorWindowThread.cpp:54
03:09:40 INFO - TEST-PASS | services/sync/tests/unit/test_history_engine.js | took 16827ms
Assignee | ||
Comment 53•5 years ago
|
||
backout was due to bug 1554438.
We can easily fix that particular crash, however I'm still puzzled on how either gfxVars::UseWebRender() returned a different value between start and shutdown or why gfxPrefs::Direct3D11UseDoubleBuffering() returned a different value between begin and end.
The case from gfxVars::UseWebRender() going from true to false was handled, but not the other way round, which shouldn't be happening really.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 54•5 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bc404207024e P0. Use X11UndefineNone.h to avoid conflicts with the macro None from X11/X.h. r=karlt https://hg.mozilla.org/integration/autoland/rev/67a82c16923e P00. Fix Windows compilation. r=dmajor https://hg.mozilla.org/integration/autoland/rev/b1c1cf0f0d94 P1. Add GPU process selector to prefs module. r=spohl,mattwoodrow https://hg.mozilla.org/integration/autoland/rev/8ae5ae35d3ce P2. add shared pref serializer/deserializer to GPU process. r=kmag https://hg.mozilla.org/integration/autoland/rev/c9a6c9c61c28 P3. Fix typo when filtering preferences to sync. r=njn https://hg.mozilla.org/integration/autoland/rev/e4bea1a40aa9 P4. Sync preferences when they changed. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/a6e383c39ebf P5. Sync preferences with RDD process when then changed. r=mattwoodrow,mjf https://hg.mozilla.org/integration/autoland/rev/bafb6a31a43b P6. Release object early when error. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/62c133df1b2c P7. Add VR process selector to prefs module. r=spohl https://hg.mozilla.org/integration/autoland/rev/d4cbcf4f1900 P8. Add shared pref serializer/deserializer to VR process. r=kmag,daoshengmu https://hg.mozilla.org/integration/autoland/rev/0a6cc70b3c31 P9. Sync preferences in VR process when they change. r=daoshengmu https://hg.mozilla.org/integration/autoland/rev/f77d8b7d5e57 P10. Define StaticPrefs setter. r=njn https://hg.mozilla.org/integration/autoland/rev/b4fa4b400a35 P11. Add atomic float preferences support. r=njn https://hg.mozilla.org/integration/autoland/rev/37fd585d6c8f P12. Convert Live gfxPrefs into StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/9d69224b5529 P13. Add Skip, Once and Live cached preference policy. r=njn,kmag https://hg.mozilla.org/integration/autoland/rev/f5ebdcca640d P14. Add GetXXName and GetXXDefault methods to StaticPrefs. r=njn https://hg.mozilla.org/integration/autoland/rev/1d9384906287 P15. Move Skip and Once gfxPrefs to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/d5ec03906d35 P17. Convert gfxPrefs::GfxLoggingLevel to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/0750ee399aa5 P18. Convert gfxPrefs::LayoutFrameRate to StaticPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/efb7f5fa4197 P19. Convert gfxPrefs::LayersWindowRecordingPath to gfxVars. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/5f9a6d06a67d P20. Add missing namespace. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/3cab1a8375d6 P21. Remove gfxPref sync with VR process. r=daoshengmu https://hg.mozilla.org/integration/autoland/rev/664443080776 P22. Remove gfxPref sync with GPU process. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/5440467afab5 P23. Remove now unused gfxPrefs. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/3d2af49acdd7 P24. Fix style of StaticPrefs. r=njn https://hg.mozilla.org/integration/autoland/rev/0ad9be661742 P25. Don't make prefs unnecessarily atomic. r=njn https://hg.mozilla.org/integration/autoland/rev/2a1a127f4aef P26. Make setter only usable on main process. r=njn https://hg.mozilla.org/integration/autoland/rev/40dda46eb126 P27. Do not set WebRender preferences as code don't expect them to exists. r=cpearce
Comment 55•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bc404207024e
https://hg.mozilla.org/mozilla-central/rev/67a82c16923e
https://hg.mozilla.org/mozilla-central/rev/b1c1cf0f0d94
https://hg.mozilla.org/mozilla-central/rev/8ae5ae35d3ce
https://hg.mozilla.org/mozilla-central/rev/c9a6c9c61c28
https://hg.mozilla.org/mozilla-central/rev/e4bea1a40aa9
https://hg.mozilla.org/mozilla-central/rev/a6e383c39ebf
https://hg.mozilla.org/mozilla-central/rev/bafb6a31a43b
https://hg.mozilla.org/mozilla-central/rev/62c133df1b2c
https://hg.mozilla.org/mozilla-central/rev/d4cbcf4f1900
https://hg.mozilla.org/mozilla-central/rev/0a6cc70b3c31
https://hg.mozilla.org/mozilla-central/rev/f77d8b7d5e57
https://hg.mozilla.org/mozilla-central/rev/b4fa4b400a35
https://hg.mozilla.org/mozilla-central/rev/37fd585d6c8f
https://hg.mozilla.org/mozilla-central/rev/9d69224b5529
https://hg.mozilla.org/mozilla-central/rev/f5ebdcca640d
https://hg.mozilla.org/mozilla-central/rev/1d9384906287
https://hg.mozilla.org/mozilla-central/rev/d5ec03906d35
https://hg.mozilla.org/mozilla-central/rev/0750ee399aa5
https://hg.mozilla.org/mozilla-central/rev/efb7f5fa4197
https://hg.mozilla.org/mozilla-central/rev/5f9a6d06a67d
https://hg.mozilla.org/mozilla-central/rev/3cab1a8375d6
https://hg.mozilla.org/mozilla-central/rev/664443080776
https://hg.mozilla.org/mozilla-central/rev/5440467afab5
https://hg.mozilla.org/mozilla-central/rev/3d2af49acdd7
https://hg.mozilla.org/mozilla-central/rev/0ad9be661742
https://hg.mozilla.org/mozilla-central/rev/2a1a127f4aef
https://hg.mozilla.org/mozilla-central/rev/40dda46eb126
Description
•