Closed Bug 1550422 Opened 5 years ago Closed 5 years ago

Remove gfxPrefs in favour of StaticPrefs

Categories

(Core :: Graphics, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla69
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
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.

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task

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.

Priority: -- → P3

(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.

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: nobody → jyavenard

No need to keep the object alive.

Will be needed to remove gfxPrefs in favor of StaticPrefs

Depends on D31014

Attachment #9063939 - Attachment description: Bug 1550422 - P5. Sync preferences with RDD process when then changed. r?mattwoodrow!,mjf! → Bug 1550422 - P5. Sync preferences with RDD process when then change. r?mattwoodrow!,mjf!

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.

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

This works identically to what gfxPrefs UpdatePolicy offers.

Depends on D31256

This is required to emulate some gfxPrefs functionalities.

Depends on D31257

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.

Flags: needinfo?(jyavenard)

(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?)

Depends on: 1552040

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.

(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.

(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.

Flags: needinfo?(jyavenard)

(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 :)

(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.

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...".

Flags: needinfo?(kmaglione+bmo)
Blocks: 1552126

(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:

  1. StaticPrefs is being initialised on the parent process
  2. 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:

  1. 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.

  2. 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.

Attachment #9063939 - Attachment description: Bug 1550422 - P5. Sync preferences with RDD process when then change. r?mattwoodrow!,mjf! → Bug 1550422 - P5. Sync preferences with RDD process when then changed. r?mattwoodrow!,mjf!

Depends on D31259

Fly-by fix, we make LoggingPrefs::sGfxLogLevel as it is written on the main thread but read on different threads.

Depends on D31459

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

Depends on D31463

Following the shift in unified build setup following the removal of gfxPrefs.{cpp,c} we hit this error.

Depends on D31464

And with some tidying some comments and removing stray #include "gfxPrefs.h"

Depends on D31467

Attachment #9063939 - Attachment description: Bug 1550422 - P5. Sync preferences with RDD process when then changed. r?mattwoodrow!,mjf! → Bug 1550422 - P5. Sync preferences with RDD process when they changed. r?mattwoodrow!,mjf!
Blocks: 1552272

Commented on patch.

Flags: needinfo?(kmaglione+bmo)
Blocks: 1552377
Attachment #9065416 - Attachment is obsolete: true
Attachment #9065421 - Attachment description: Bug 1550422 - P21. Fix Linux compilation. r?mattwoodrow → Bug 1550422 - P21. Fix Linux compilation. r?karlt!
Blocks: 1552643
Attachment #9063939 - Attachment description: Bug 1550422 - P5. Sync preferences with RDD process when they changed. r?mattwoodrow!,mjf! → Bug 1550422 - P5. Sync preferences with RDD process when then changed. r?mattwoodrow!,mjf!

Prefs aren't yet sorted as they should be, this will be done in bug 1552643

Depends on D31468

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

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

Attachment #9065421 - Attachment description: Bug 1550422 - P21. Fix Linux compilation. r?karlt! → Bug 1550422 - P0. Fix Linux compilation. r?karlt!
Attachment #9065422 - Attachment description: Bug 1550422 - P22. Fix Windows compilation. r?dmajor → Bug 1550422 - P21. Fix Windows compilation. r?dmajor
Attachment #9065423 - Attachment description: Bug 1550422 - P23. Remove gfxPref sync with VR process. r?daoshengmu! → Bug 1550422 - P22. Remove gfxPref sync with VR process. r?daoshengmu!
Attachment #9065424 - Attachment description: Bug 1550422 - P24. Remove gfxPref sync with GPU process. r?mattwoodrow → Bug 1550422 - P23. Remove gfxPref sync with GPU process. r?mattwoodrow
Attachment #9065425 - Attachment description: Bug 1550422 - P25. Remove now unused gfxPrefs. r?jrmuizel! → Bug 1550422 - P24. Remove now unused gfxPrefs. r?jrmuizel!
Attachment #9065900 - Attachment description: Bug 1550422 - P26. Fix style of StaticPrefs. r?njn! → Bug 1550422 - P25. Fix style of StaticPrefs. r?njn!
Attachment #9065901 - Attachment description: Bug 1550422 - P27. Don't make prefs unnecessarily atomic. r?njn! → Bug 1550422 - P26. Don't make prefs unnecessarily atomic. r?njn!
Attachment #9065961 - Attachment description: Bug 1550422 - P28. Make setter only usable on main process. r?njn! → Bug 1550422 - P27. Make setter only usable on main process. r?njn!
Attachment #9065422 - Attachment description: Bug 1550422 - P21. Fix Windows compilation. r?dmajor → Bug 1550422 - P00. Fix Windows compilation. r?dmajor
Attachment #9065423 - Attachment description: Bug 1550422 - P22. Remove gfxPref sync with VR process. r?daoshengmu! → Bug 1550422 - P21. Remove gfxPref sync with VR process. r?daoshengmu!
Attachment #9065424 - Attachment description: Bug 1550422 - P23. Remove gfxPref sync with GPU process. r?mattwoodrow → Bug 1550422 - P22. Remove gfxPref sync with GPU process. r?mattwoodrow
Attachment #9065425 - Attachment description: Bug 1550422 - P24. Remove now unused gfxPrefs. r?jrmuizel! → Bug 1550422 - P23. Remove now unused gfxPrefs. r?jrmuizel!
Attachment #9065900 - Attachment description: Bug 1550422 - P25. Fix style of StaticPrefs. r?njn! → Bug 1550422 - P24. Fix style of StaticPrefs. r?njn!
Attachment #9065901 - Attachment description: Bug 1550422 - P26. Don't make prefs unnecessarily atomic. r?njn! → Bug 1550422 - P25. Don't make prefs unnecessarily atomic. r?njn!
Attachment #9065961 - Attachment description: Bug 1550422 - P27. Make setter only usable on main process. r?njn! → Bug 1550422 - P26. Make setter only usable on main process. r?njn!

:kmag, your r- an earlier revision which is now blocking this queue.

Could you have a look again?

Flags: needinfo?(kmaglione+bmo)
Attachment #9065421 - Attachment is obsolete: true
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
Attached file Bug 1550422 - P27. Fix long line. (obsolete) —

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

Flags: needinfo?(jyavenard)

Additionally, rename some pref constants to improve code clarity as we can no longer rely on using the StaticPrefs accessor.

Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(jyavenard)
Attachment #9066907 - Attachment is obsolete: true
Blocks: 1554198
Blocks: 1554334
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

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

Flags: needinfo?(jyavenard)
Depends on: 1554438

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.

Flags: needinfo?(jyavenard)
Attachment #9065074 - Attachment description: Bug 1550422 - P12. Convert Live gfxPrefs into StaticPrefs. r?jrmuizel! → Bug 1550422 - P12. Convert Live gfxPrefs into StaticPrefs. r=jrmuizel
Attachment #9065075 - Attachment description: Bug 1550422 - P13. Add Skip, Once and Live cached preference policy. r?njn! → Bug 1550422 - P13. Add Skip, Once and Live cached preference policy. r=njn,kmag
Attachment #9065076 - Attachment description: Bug 1550422 - P14. Add GetXXName and GetXXDefault methods to StaticPrefs. r?njn! → Bug 1550422 - P14. Add GetXXName and GetXXDefault methods to StaticPrefs. r=njn
Attachment #9065077 - Attachment description: Bug 1550422 - P15. Move Skip and Once gfxPrefs to StaticPrefs. r?jrmuizel! → Bug 1550422 - P15. Move Skip and Once gfxPrefs to StaticPrefs. r=jrmuizel
Attachment #9065417 - Attachment description: Bug 1550422 - P17. Convert gfxPrefs::GfxLoggingLevel to StaticPrefs. r?jrmuizel! → Bug 1550422 - P17. Convert gfxPrefs::GfxLoggingLevel to StaticPrefs. r=jrmuizel
Attachment #9065418 - Attachment description: Bug 1550422 - P18. Convert gfxPrefs::LayoutFrameRate to StaticPrefs. r?jrmuizel! → Bug 1550422 - P18. Convert gfxPrefs::LayoutFrameRate to StaticPrefs. r=jrmuizel
Attachment #9065419 - Attachment description: Bug 1550422 - P19. Convert gfxPrefs::LayersWindowRecordingPath to gfxVars. r?jrmuizel! → Bug 1550422 - P19. Convert gfxPrefs::LayersWindowRecordingPath to gfxVars. r=jrmuizel
Attachment #9065420 - Attachment description: Bug 1550422 - P20. Add missing namespace. r?mattwoodrow → Bug 1550422 - P20. Add missing namespace. r=mattwoodrow
Attachment #9065423 - Attachment description: Bug 1550422 - P21. Remove gfxPref sync with VR process. r?daoshengmu! → Bug 1550422 - P21. Remove gfxPref sync with VR process. r=daoshengmu
Attachment #9065424 - Attachment description: Bug 1550422 - P22. Remove gfxPref sync with GPU process. r?mattwoodrow → Bug 1550422 - P22. Remove gfxPref sync with GPU process. r=mattwoodrow
Attachment #9065425 - Attachment description: Bug 1550422 - P23. Remove now unused gfxPrefs. r?jrmuizel! → Bug 1550422 - P23. Remove now unused gfxPrefs. r=jrmuizel
Attachment #9065900 - Attachment description: Bug 1550422 - P24. Fix style of StaticPrefs. r?njn! → Bug 1550422 - P24. Fix style of StaticPrefs. r=njn
Attachment #9065901 - Attachment description: Bug 1550422 - P25. Don't make prefs unnecessarily atomic. r?njn! → Bug 1550422 - P25. Don't make prefs unnecessarily atomic. r=njn
Attachment #9065961 - Attachment description: Bug 1550422 - P26. Make setter only usable on main process. r?njn! → Bug 1550422 - P26. Make setter only usable on main process. r=njn
Attachment #9067231 - Attachment description: Bug 1550422 - P27. Do not set WebRender preferences as code don't expect them to exists. r?cpearce! → Bug 1550422 - P27. Do not set WebRender preferences as code don't expect them to exists. r=cpearce
Attachment #9065073 - Attachment description: Bug 1550422 - P11. Add atomic float preferences support. r?njn! → Bug 1550422 - P11. Add atomic float preferences support. r=njn
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

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

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Regressions: 1424433
Regressions: 1554597
Regressions: 1556091
No longer regressions: 1556091
Regressions: 1595420
See Also: → 1577507
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: