Closed
Bug 1266213
Opened 8 years ago
Closed 8 years ago
[e10s] Block users of GTK+ 3.20+ from e10s experiments on Beta
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: Felipe, Assigned: Felipe)
References
Details
Attachments
(2 files)
3.53 KB,
patch
|
jimm
:
review+
stransky
:
feedback+
|
Details | Diff | Splinter Review |
1.96 KB,
patch
|
Felipe
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1264454 +++ There's a new top crasher that affects e10s users of GTK+ 3.20 (bug 1264454). It's unlikely that it can be fixed before we merge 47 to beta next week (and thus starting a new e10s testing phase). In order to not let that crasher skew the stability numbers during our experiment, it's been suggested to block these users from the experiment. This is yet to be decided but I wanted to file and write a patch ahead of time since the merge is just a few days away
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8743541 -
Flags: review?(karlt)
Attachment #8743541 -
Flags: feedback?(stransky)
Comment 2•8 years ago
|
||
Comment on attachment 8743541 [details] [diff] [review] Patch The gtk3 test looks sane although I have no idea how the e10s check works in general.
Attachment #8743541 -
Flags: feedback?(stransky) → feedback+
Updated•8 years ago
|
Assignee | ||
Updated•8 years ago
|
Attachment #8743541 -
Flags: review?(jmathies)
Assignee | ||
Comment 3•8 years ago
|
||
Note: since this is a very temporary thing, and there's ongoing progress in bug 1264454, I'm inclined to not land the new string here that says "Disabled for GTK..". If blocked, it will show a standard placeholder text "Unknown" in about:support, which is likely fine for the small amount of users and time that this will be in place.
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad2f84047d97
Comment 5•8 years ago
|
||
Comment on attachment 8743541 [details] [diff] [review] Patch This appears correct to me based on the docs I found here - https://developer.gnome.org/gtk3/stable/gtk3-Feature-Test-Macros.html I'm assuming we target GTK 3 so we wouldn't be linked to an older library and get an older version of gtk_check_version.
Attachment #8743541 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 6•8 years ago
|
||
I sent to tryserver [1] a version of this patch that doesn't contain the #ifdef RELEASE_BUILD part, in order for this code to take effect. When the builds are ready [2], could someone who is using GTK 3.20 verify that it works as intended? i.e., e10s was blocked from running. The e10s checkbox in about:preferences should be grayed out. [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6c0a9663bd8 [2] https://archive.mozilla.org/pub/firefox/try-builds/felipc@gmail.com-b6c0a9663bd85c3353c35558cf6fab7592ef3bab/
Assignee | ||
Comment 7•8 years ago
|
||
New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=401a29b015f6 https://archive.mozilla.org/pub/firefox/try-builds/felipc@gmail.com-401a29b015f637d7fc435a6838f65cf2167b502b/
Assignee | ||
Comment 8•8 years ago
|
||
Johnny said he is running GTK 3.20 and can try it. Johnny, the right build is from Comment 7. Please run it, go to about:support and check if Multiprocess Windows says "0/0 (Unknown status)".
Flags: needinfo?(jst)
Assignee | ||
Comment 10•8 years ago
|
||
Thanks! the patch works as intended to block GTK 3.20. I had said "0/0" but that was a typo. Try build also passed M-e10s(1) so I think it's safe to say that the test machines are not on 3.20, and we can land the patch without special workarounds for tests. I'll land it now in order to ask for aurora approval in time for the merge to beta.
Assignee | ||
Updated•8 years ago
|
Attachment #8743541 -
Flags: review?(karlt)
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e08681234156
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 13•8 years ago
|
||
Approval Request Comment [Feature/regressing bug #]: GTK 3.20 API chance that causes a crash [User impact if declined]: E10s experiment on beta 47 might be crashier than intended if we don't block GTK 3.20 users from it [Describe test coverage new/current, TreeHerder]: landed on central [Risks and why]: limited to gtk and the e10s switch [String/UUID change made/needed]: none
Attachment #8744648 -
Flags: review+
Attachment #8744648 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 14•8 years ago
|
||
I per-emptively pushed this before approval because I want this to make it to the merge, and I've already talked about this patch to ritu. https://hg.mozilla.org/releases/mozilla-aurora/rev/8551b253f406
status-firefox47:
--- → ?
Comment on attachment 8744648 [details] [diff] [review] Patch for aurora, r=jimm This was pre-approved.
Attachment #8744648 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•