Re-prompt nightly users to enable e10s

RESOLVED FIXED in Firefox 35

Status

()

Firefox
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mossop, Assigned: Felipe)

Tracking

unspecified
Firefox 35
Points:
3
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(e10sm4+)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Now that we've fixed more bugs it is time to re-prompt nightly users to try e10s again. We should probably include some new wording in the prompt to talk about some of the things we've fixed since the last time (crashers, pdfjs, ... ideas?) so users who disabled it before might be more tempted to try again.
Flags: firefox-backlog+
(Assignee)

Updated

4 years ago
Assignee: nobody → felipc
Status: NEW → ASSIGNED

Updated

4 years ago
Flags: qe-verify?

Comment 1

4 years ago
I still see the existing prompt every time I ./mach run (creating a new profile by default). Can we fix that when changing this new prompt? It's really annoying.
tracking-e10s: ? → m4+
(Assignee)

Updated

4 years ago
Flags: qe-verify? → qe-verify-
I hope we fix bug 1072980 first, I tried enabling E10s and my browser entered a perma-crash situation, forcing me to manually edit prefs.js to disable E10s.
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #2)
> I hope we fix bug 1072980 first, I tried enabling E10s and my browser
> entered a perma-crash situation, forcing me to manually edit prefs.js to
> disable E10s.

ah looks like it's due to forceRTL add-on, so, likely not a real blocker.
(Assignee)

Comment 4

4 years ago
(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #2)
> I hope we fix bug 1072980 first, I tried enabling E10s and my browser
> entered a perma-crash situation, forcing me to manually edit prefs.js to
> disable E10s.

Fwiw, you can use Safe mode which disables e10s and then you can edit the pref or uncheck the checkbox in the preferences dialog.
(In reply to :Felipe Gomes from comment #4)
> Fwiw, you can use Safe mode which disables e10s and then you can edit the
> pref or uncheck the checkbox in the preferences dialog.

yeah, but I was a little bit upset and forgot about the shift+launch option :)
Hi Felipe, can you provide a point value.
Iteration: --- → 35.3
Flags: needinfo?(felipc)
(Assignee)

Comment 7

4 years ago
Created attachment 8501914 [details]
Listing enhancements

What do people think of the simple approach of showing the list of enhancements in the popup?

Chris, what should we fill this list with?

feedbacking Madhava but anyone who cares to chime in, please do
Attachment #8501914 - Flags: feedback?(madhava)
Flags: needinfo?(cpeterson)
I think it's a good idea because it will give people who opted-out a reason to try e10s again.
Flags: needinfo?(cpeterson)
(Assignee)

Updated

4 years ago
Points: --- → 3
Flags: needinfo?(felipc)
(Assignee)

Comment 9

4 years ago
Created attachment 8501935 [details] [diff] [review]
Re-prompt patch

This improves on the previous code as the following:

- easier to bump number for other rounds of re-prompting, and it will clean up after the previous pref
- won't display for local builds as requested in comment 1 (I couldn't use OFFICIAL_BRANDING check because that's also false for Nightly, so I used the update channel)
- added check for hardware acceleration so it's not gonna be offered for users who would be blocked from running e10s

The message param for doorhangers doesn't accept multi-line text, so we need to use the popupnotificationcontent feature to add more elements. I can't create that element dinamically because when the popup is dismissed, it would be destroyed, and not show up again when retrieved from the URLbar icon. So I had to add it to popup-notifications.inc.

Note that with this patch I'm also activating the notice added in bug 1063842. It will show up once for every user running with e10s autostart.
Attachment #8501935 - Flags: review?(mconley)
Attachment #8501935 - Flags: review?(dtownsend+bugmail)
Comment on attachment 8501935 [details] [diff] [review]
Re-prompt patch

Review of attachment 8501935 [details] [diff] [review]:
-----------------------------------------------------------------

This looks OK to me, but can I see a screenshot of the popup?

::: browser/components/nsBrowserGlue.js
@@ +2272,5 @@
>    // Increase this number each time we want to roll out an
>    // e10s testing period to Nightly users.
> +  CURRENT_NOTICE_COUNT: 1,
> +  CURRENT_PROMPT_PREF: "browser.displayedE10SPrompt.1",
> +  PREVIOUS_PROMPT_PREF: "browser.displayedE10SPrompt",

As these will probably stack up, should this be an array of old prompt prefs?
(Assignee)

Comment 11

4 years ago
The screenshot is attached :) attachment 8501914 [details]

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #10)
> ::: browser/components/nsBrowserGlue.js
> @@ +2272,5 @@
> >    // Increase this number each time we want to roll out an
> >    // e10s testing period to Nightly users.
> > +  CURRENT_NOTICE_COUNT: 1,
> > +  CURRENT_PROMPT_PREF: "browser.displayedE10SPrompt.1",
> > +  PREVIOUS_PROMPT_PREF: "browser.displayedE10SPrompt",
> 
> As these will probably stack up, should this be an array of old prompt prefs?

I was planning on just clearing the immediate previous pref, not all the past ones. We don't even need to do it, but it's just a bit nicer to not leave these prefs poluting the profile.
Comment on attachment 8501935 [details] [diff] [review]
Re-prompt patch

Ah, yes - somehow missed the screenshot there. My bad.

(In reply to :Felipe Gomes from comment #11)
> The screenshot is attached :) attachment 8501914 [details]
> 
> (In reply to Mike Conley (:mconley) - Needinfo me! from comment #10)
> > ::: browser/components/nsBrowserGlue.js
> > @@ +2272,5 @@
> > >    // Increase this number each time we want to roll out an
> > >    // e10s testing period to Nightly users.
> > > +  CURRENT_NOTICE_COUNT: 1,
> > > +  CURRENT_PROMPT_PREF: "browser.displayedE10SPrompt.1",
> > > +  PREVIOUS_PROMPT_PREF: "browser.displayedE10SPrompt",
> > 
> > As these will probably stack up, should this be an array of old prompt prefs?
> 
> I was planning on just clearing the immediate previous pref, not all the
> past ones. We don't even need to do it, but it's just a bit nicer to not
> leave these prefs poluting the profile.

Right, and I'm all for un-polluting user profiles where we can. The scenario I'm thinking of is that a user enables e10s, and then decides to change to a different channel (or doesn't use their Nightly) for some time so that we skip clearing one or more prefs. It's not a big deal, but it'd be nice to maybe clean-up where we can.

Otherwise, I thinks this looks good as a E10S_TESTING_ONLY sorta thing.
Attachment #8501935 - Flags: review?(mconley) → review+
Comment on attachment 8501914 [details]
Listing enhancements

I'm OK with this, but let's
- keep A, B, and C short (one line each)
- and no more than three -- possibly fewer?

And this _isn't_ a precedent for starting to do this in release.
Attachment #8501914 - Flags: feedback?(madhava) → feedback+
Felipe: here is a list of bugs fixed since we first prompted Nightly users to test e10s on November 12:

http://is.gd/FBsTJW

Three notable fixes:

* Improved add-on compatibility
* Fixed many Developer Tools bugs
* Fixed spell checker

I don't think we should include bug numbers because these are meta-issues that include many fixes.
(Assignee)

Comment 15

4 years ago
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #12)
> Comment on attachment 8501935 [details] [diff] [review]
> Re-prompt patch
> 
> Ah, yes - somehow missed the screenshot there. My bad.
> 
> (In reply to :Felipe Gomes from comment #11)
> > The screenshot is attached :) attachment 8501914 [details]
> > 
> > (In reply to Mike Conley (:mconley) - Needinfo me! from comment #10)
> > > ::: browser/components/nsBrowserGlue.js
> > > @@ +2272,5 @@
> > > >    // Increase this number each time we want to roll out an
> > > >    // e10s testing period to Nightly users.
> > > > +  CURRENT_NOTICE_COUNT: 1,
> > > > +  CURRENT_PROMPT_PREF: "browser.displayedE10SPrompt.1",
> > > > +  PREVIOUS_PROMPT_PREF: "browser.displayedE10SPrompt",
> > > 
> > > As these will probably stack up, should this be an array of old prompt prefs?
> > 
> > I was planning on just clearing the immediate previous pref, not all the
> > past ones. We don't even need to do it, but it's just a bit nicer to not
> > leave these prefs poluting the profile.
> 
> Right, and I'm all for un-polluting user profiles where we can. The scenario
> I'm thinking of is that a user enables e10s, and then decides to change to a
> different channel (or doesn't use their Nightly) for some time so that we
> skip clearing one or more prefs. It's not a big deal, but it'd be nice to
> maybe clean-up where we can.

Yeah, good point.. I'll postpone doing this the next time when there's a second pref to clean-up :)
(Assignee)

Comment 16

4 years ago
I combined everyone's suggestions and landed the final text as:

Would you like to help us test multiprocess Nightly (e10s)? You can also enable e10s in Nightly preferences. Notable fixes:

  Less crashing!
  Improved add-on compatibility and DevTools
  PDF.js, Web Console, Spellchecking, WebRTC now work
(Assignee)

Updated

4 years ago
Attachment #8501935 - Flags: review?(dtownsend+bugmail)
This merged to m-c, and then I backed this out in https://hg.mozilla.org/mozilla-central/rev/f5557f04db0b at Felipe's request so it doesn't ship in a Nightly on Friday or over the weekend.
(Assignee)

Comment 19

4 years ago
Relanded for showing up on Monday's Nightly: https://hg.mozilla.org/integration/mozilla-inbound/rev/facebebfea4d
https://hg.mozilla.org/mozilla-central/rev/facebebfea4d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
You need to log in before you can comment on or make changes to this bug.