Closed
Bug 1397415
Opened 7 years ago
Closed 7 years ago
Sync shield-recipe-client v73 from GitHub (commit e96eea7)
Categories
(Firefox :: Normandy Client, defect)
Firefox
Normandy Client
Tracking
()
RESOLVED
FIXED
People
(Reporter: osmose, Assigned: osmose)
References
Details
Attachments
(2 files)
59 bytes,
text/x-review-board-request
|
Gijs
:
review+
|
Details |
60.66 KB,
patch
|
Gijs
:
review+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
There have been 4 PRs since the last sync (bug 1388823) that affected the add-on: - #1002: Fix bug 1393257: Stop in-progress studies when opt-out pref changes. https://github.com/mozilla/normandy/pull/1002 - #1010: Bug 1392738: Update how we open the new preferences UI. https://github.com/mozilla/normandy/pull/1010 - #1024: Bug 1371350: Delay almost all startup tasks until after sessionstore-windows-restored https://github.com/mozilla/normandy/pull/1024 - #1029: Fix #856: Add type parameter to preference experiment annotations. https://github.com/mozilla/normandy/pull/1029 We'd also like to uplift these to beta, but bug 1349689, which landed in mozilla-central, also modified Shield with changes that we don't want to uplift. This mean the uplift patch will be different from the sync one; I'll rebase and apply the patches to mozilla-beta manually and attach a second patch to use for uplift separate from the mozilla-central sync.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8905216 [details] Bug 1397415: Sync shield-recipe-client v73 from GitHub (commit e96eea7) https://reviewboard.mozilla.org/r/177000/#review182034 I mean, rs=me because I think this basically already got reviewed - though I'm a little confused about the vendor changes, especially propTypes where the before and after look basically the same to me, so I'm not sure what changed there... Haven't checked the rest really, let me know if I should (I assume there were no conflicts when merging this? :-)
Attachment #8905216 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 4•7 years ago
|
||
This patch includes the following PRs: - #1002: Fix bug 1393257: Stop in-progress studies when opt-out pref changes. https://github.com/mozilla/normandy/pull/1002 - #1024: Bug 1371350: Delay almost all startup tasks until after sessionstore-windows-restored https://github.com/mozilla/normandy/pull/1024 - #1029: Fix #856: Add type parameter to preference experiment annotations. https://github.com/mozilla/normandy/pull/1029 It also includes the fix from bug 1392648, which one of the PRs relies upon.
Attachment #8905269 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Michael Kelly [:mkelly,:Osmose] from comment #4) > Created attachment 8905269 [details] [diff] [review] > Sync shield-recipe-client v73 from GitHub (commit e96eea7) to beta This is the patch we should use for uplifting this to Beta. It is missing the PR for bug 1392738 (which relied on the changes in mozilla-central when the old preferences organization was removed) and includes the patch for bug 1392648. Otherwise it should be the same as attachment 8905216 [details].
Pushed by mkelly@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4ef48b1ea11b Sync shield-recipe-client v73 from GitHub (commit e96eea7) r=Gijs
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4ef48b1ea11b
Comment 8•7 years ago
|
||
Comment on attachment 8905269 [details] [diff] [review] Sync shield-recipe-client v73 from GitHub (commit e96eea7) to beta Review of attachment 8905269 [details] [diff] [review]: ----------------------------------------------------------------- Again, rs=me because this all got reviewed already.
Attachment #8905269 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 9•7 years ago
|
||
we see some reduced fileIO with this: == Change summary for alert #9262 (as of September 06 2017 21:53 UTC) == Improvements: 7% tp5n nonmain_startup_fileio windows7-32 pgo e10s 1,695,344.50 -> 1,568,891.92 7% tp5n nonmain_startup_fileio windows7-32 opt e10s 1,706,547.15 -> 1,594,771.92 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9262
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8905269 [details] [diff] [review] Sync shield-recipe-client v73 from GitHub (commit e96eea7) to beta Approval Request Comment [Feature/Bug causing the regression]: This is a combination of fixes for four bugs (since Normandy does development in Github and syncs to m-c): - Bug 1393257 - End in-progress add-on studies when the opt-out preference is switched off - Bug 1371350 - shield shouldn't load anything before opening and painting the first browser window - Bug 1392648 - SHIELD browser-chrome tests are going to permaleak when Gecko 57 merges to Beta on 2017-09-20 - #856 - Annotate studies with new experiment type option (Github: https://github.com/mozilla/normandy/issues/856 [User impact if declined]: - Users will not be unenrolled from in-progress feature experiments if they choose to opt-out of experiments globally. - Firefox startup will be slower due to increased fileio from the performance regression. - Telemetry infrastructure for doing automated analysis of feature experiments will continue to be at risk of falling over if we send experiments to large populations. [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes. The performance fix and leak have been verified fixed in automation, and QA has verified the opt-out pref fix as part of their larger test plan for opt-out studies. [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Shouldn't be. [Why is the change risky/not risky?]: All the changes are mostly to non-user-visible functionality, and are easily rolled back with a system add-on update if we find something wrong. [String changes made/needed]: None
Attachment #8905269 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox56:
--- → affected
Comment 11•7 years ago
|
||
Comment on attachment 8905269 [details] [diff] [review] Sync shield-recipe-client v73 from GitHub (commit e96eea7) to beta Fix for some perf and leaking issues in Shield. Let's uplift this for beta 10.
Attachment #8905269 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/9958b8408817
Updated•6 years ago
|
Product: Shield → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•