Closed Bug 1384962 Opened 7 years ago Closed 7 years ago

Loading Preferences page resets user set process count value if the value happen to be the default


(Firefox :: Settings UI, defect, P1)

55 Branch
Windows 10



Firefox 56
Tracking Status
firefox55 - verified
firefox56 --- verified


(Reporter: timdream, Assigned: timdream)



(Whiteboard: [photon-preference][e10s-multi:?])


(2 files)

+++ This bug was initially created as a clone of Bug #1382649 +++

STR originally filed in bug 1382649 comment 33.

1. Set dom.ipc.processCount.web to 2 in Nightly
2. Go to Preferences, uncheck "use default performance"
3. Set the process count to 4 with the UI (4 is the default dom.ipc.processCount value in Nightly)
4. Reload the Perferences page


1. process count set to 4


1. process count flip to "2 (default)"

[Tracking Requested - why for this release]: Follow up fix to bug 1382649, which was incomplete.
Unfortunately, I cannot verify my tests at home because of bug 1252976. Try run is scheduled on MozReview. I did manually verify the patch with STR above.
Assignee: nobody → timdream
I think you linked to the wrong bug in comment #2? That bug is fixed.
Comment on attachment 8890903 [details]
Bug 1384962 - Don't set process count for user on Preferences page load,

::: commit-message-658cb:3
(Diff revision 1)
> +Bug 1384962 - Don't set process count for user on Preferences page load, r=jaws
> +
> +This is a follow-up to incompete fix in bug 1382649. In that bug, I attempted to


::: browser/components/preferences/in-content-new/main.js:1117
(Diff revision 1)
>          accelerationPref.value != accelerationPref.defaultValue) {
>        defaultPerformancePref.value = false;
>      }
>    },
> -  updatePerformanceSettingsBox() {
> +  updatePerformanceSettingsBox(changed) {

Please use a object as the argument to the function so we can have "named parameters".

updatePerformanceSettingsBox({duringChangeEvent}) {

Then the calling code would be:
this.updatePerformanceSettingsBox({duringChangeEvent: true});
Attachment #8890903 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)
> I think you linked to the wrong bug in comment #2? That bug is fixed.

I was meant to link to bug 1384153 in comment 2, thanks for spotting that.
Attached patch beta.patchSplinter Review
Approval Request Comment
[Feature/Bug causing the regression]: Incomplete fix in bug 1382649
[User impact if declined]: On Beta, user will see their process count revert to value set by e10s rollout if they set the process count value to 1.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: See STR in comment 0 but set process count to 1 on step 3.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Localized. Only as risky as bug 1382649.
[String changes made/needed]: None.

Patch for beta strips the test case change in in-content-new since the feature is disabled on 55.
Attachment #8891168 - Flags: review+
Attachment #8891168 - Flags: approval-mozilla-beta?
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #0)

Noted to verify this bug on Beta branch, in step 3 of the STR in comment 0, you would need to set the process count to 1.
Pushed by
Don't set process count for user on Preferences page load, r=jaws
Backed out for frequently exceeding maximum run time for browser/components/preferences/in-content/tests/browser_performance.js:

Push with failures:
Failure log:
[task 2017-07-28T05:07:15.506613Z] 05:07:15     INFO - Entering test bound 
[task 2017-07-28T05:07:15.511240Z] 05:07:15     INFO - Buffered messages logged at 05:07:12
[task 2017-07-28T05:07:15.513203Z] 05:07:15     INFO - TEST-PASS | browser/components/preferences/in-content-new/tests/browser_performance.js | General pane was selected - 
[task 2017-07-28T05:07:15.515213Z] 05:07:15     INFO - TEST-PASS | browser/components/preferences/in-content-new/tests/browser_performance.js | performance settings section should be shown - 
[task 2017-07-28T05:07:15.519100Z] 05:07:15     INFO - TEST-PASS | browser/components/preferences/in-content-new/tests/browser_performance.js | pref value is false - 
[task 2017-07-28T05:07:15.525566Z] 05:07:15     INFO - TEST-PASS | browser/components/preferences/in-content-new/tests/browser_performance.js | checkbox should not be checked - 
[task 2017-07-28T05:07:15.527812Z] 05:07:15     INFO - Buffered messages logged at 05:07:13
[task 2017-07-28T05:07:15.530738Z] 05:07:15     INFO - Leaving test bound 
[task 2017-07-28T05:07:15.532618Z] 05:07:15     INFO - Buffered messages finished
[task 2017-07-28T05:07:15.534833Z] 05:07:15     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content-new/tests/browser_performance.js | This test exceeded the timeout threshold. It should be rewritten or split up. If that's not possible, use requestLongerTimeout(N), but only as a last resort. -
Flags: needinfo?(timdream)
New test items moved to another file -- will ask for checked-in when Try confirms added tests works.
Flags: needinfo?(timdream)
Pushed by
Don't set process count for user on Preferences page load, r=jaws
Keywords: checkin-needed
This bug is not reproducible on latest Beta 55.0b13.
Sorry, didn't mean to flip that. Tim, should we let this ride 56 given comment 15?
Flags: needinfo?(timdream)
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
I am pretty sure it's reproducible... Did you check comment 9? The STR is slightly different, and it just a reduced steps from bug 1382649 comment 33. which you have reproduced.
Flags: needinfo?(timdream) → needinfo?(brindusa.tot)
Yes, you are right! With scenario from comment 9, it is reproducible on Beta 55.0b13. Sorry for the misunderstanding.
Flags: needinfo?(brindusa.tot)
Comment on attachment 8891168 [details] [diff] [review]

followup fix for e10s-multi pref ui, beta55+

should be in 55.0 rc1
Attachment #8891168 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed with latest Nightly 56.0a1, build ID 20170730100307 on Windows 10, Ubuntu 16.04 and Mac OS X 10.12.
Verified as fixed with the Release Candidate build 55.0, Build ID 20170731093223, on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.