| Submitter | Diff | Changes | Open Issues | Last Updated |
|---|---|---|---|---|
| Loading... | ||||
| Error loading review requests: | ||||
|
59 bytes,
text/x-review-board-request
|
jaws
:
review+
|
Details | Review |
|
12.08 KB,
patch
|
timdream
:
review+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
+++ 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 Expected: 1. process count set to 4 Actual: 1. process count flip to "2 (default)" [Tracking Requested - why for this release]: Follow up fix to bug 1382649, which was incomplete.
| Comment hidden (mozreview-request) |
|
(Assignee) | |
Comment 2•28 days ago
|
||
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.
|
||
Comment 3•27 days ago
|
||
I think you linked to the wrong bug in comment #2? That bug is fixed.
|
|
||
Comment 4•27 days ago
|
||
| mozreview-review | ||
Comment on attachment 8890903 [details] Bug 1384962 - Don't set process count for user on Preferences page load, https://reviewboard.mozilla.org/r/162110/#review167590 ::: 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 s/incompete/incomplete/ ::: 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});
|
|
(Assignee) | |
Comment 5•27 days ago
|
||
(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.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
(Assignee) | |
Comment 8•27 days ago
|
||
Created attachment 8891168 [details] [diff] [review] beta.patch 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.
|
|
(Assignee) | |
Comment 9•27 days ago
|
||
(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.
|
||
Comment 10•27 days ago
|
||
Pushed by rchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9bba40accbf6 Don't set process count for user on Preferences page load, r=jaws
|
||
Comment 11•27 days ago
|
||
Backed out for frequently exceeding maximum run time for browser/components/preferences/in-content/tests/browser_performance.js: https://hg.mozilla.org/integration/autoland/rev/09a412e3cc3b1a1fd9a1325e161d36b91018ef24 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9bba40accbf61384715d963df18e05227676591f&filter-searchStr=533f3b0d33f8841996a16249ba630067c1ca4719 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=118816229&repo=autoland [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. -
| Comment hidden (mozreview-request) |
|
|
(Assignee) | |
Comment 13•27 days ago
|
||
New test items moved to another file -- will ask for checked-in when Try confirms added tests works. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f085b2a8e26f01a53ec72d063095138f37f27449
|
|
(Assignee) | |
Updated•27 days ago
|
||
|
|
||
Comment 14•27 days ago
|
||
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/a4d0803e7690 Don't set process count for user on Preferences page load, r=jaws
|
||
Updated•27 days ago
|
||
|
|
||
Comment 15•27 days ago
|
||
This bug is not reproducible on latest Beta 55.0b13.
|
||
Updated•27 days ago
|
||
|
|
||
Comment 16•27 days ago
|
||
Sorry, didn't mean to flip that. Tim, should we let this ride 56 given comment 15?
|
||
Comment 17•26 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a4d0803e7690
|
|
(Assignee) | |
Comment 19•26 days ago
|
||
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.
|
|
||
Comment 20•24 days ago
|
||
Yes, you are right! With scenario from comment 9, it is reproducible on Beta 55.0b13. Sorry for the misunderstanding.
|
||
Comment 21•24 days ago
|
||
Comment on attachment 8891168 [details] [diff] [review] beta.patch followup fix for e10s-multi pref ui, beta55+ should be in 55.0 rc1
|
|
||
Comment 22•24 days ago
|
||
| bugherderuplift | ||
https://hg.mozilla.org/releases/mozilla-beta/rev/f93965089c0a
|
|
||
Comment 23•24 days ago
|
||
Verified as fixed with latest Nightly 56.0a1, build ID 20170730100307 on Windows 10, Ubuntu 16.04 and Mac OS X 10.12.
|
|
||
Updated•24 days ago
|
||
|
|
||
Comment 24•23 days ago
|
||
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.
Description
•