:Felipe Gomes (needinfo for replies!) Assignee
	Description • 7 years ago

This #ifdef has Nightly-only features that include the checkbox for e10s in about:preferences, and the "Open Non-e10s Window" toolbar button.  Now that e10s is completing its rollout and becomes the de-facto configuration, we can remove these things.

This should land before or together with bug 1406212 because otherwise we'd need to waste time adjusting the code for the checkbox in about:preferences.

	Mike Hommey [:glandium]
	Comment 6 • 7 years ago
mozreview-review

Comment on attachment 8917151 [details]
Bug 1407351 - Remove E10S_TESTING_ONLY defines.

https://reviewboard.mozilla.org/r/188162/#review193454

	Mike de Boer [:mikedeboer]
	Comment 7 • 7 years ago
mozreview-review

Comment on attachment 8917147 [details]
Bug 1407351 - Remove nightly-only e10s testing features from the main browser window.

https://reviewboard.mozilla.org/r/188154/#review193538

This looks good to me (I'm always very grateful for patches that clean things up :-) ), but the r- is because you need to add a migration step for profiles that may have customized that widget.
You can do this in `CustomizableUI._updateForNewVersion()`, which is very similar to `_migrateUI()` in nsBrowserGlue.js. You can find a good example at the bottom of that function where we remove the 'loop' and 'pocket' buttons.

::: browser/base/content/tabbrowser.xml:5535
(Diff revision 1)
>            } else {
>              label = tab._fullLabel || tab.getAttribute("label");
> -            if (AppConstants.E10S_TESTING_ONLY &&
> +            if (AppConstants.NIGHTLY_BUILD &&
>                  tab.linkedBrowser &&
>                  tab.linkedBrowser.isRemoteBrowser) {
>                label += " - e10s";

I'd like you to remove this label addition then, because the tab being e10s is kinda implicit.

::: browser/base/content/tabbrowser.xml:5538
(Diff revision 1)
>                  tab.linkedBrowser &&
>                  tab.linkedBrowser.isRemoteBrowser) {
>                label += " - e10s";
>                if (tab.linkedBrowser.frameLoader &&
>                    Services.appinfo.maxWebProcessCount > 1) {
>                  label += " (" + tab.linkedBrowser.frameLoader.tabParent.osPid + ")";

And somehow denote this string so that it becomes clear to Nightly users what the number means, like 'Nightly Start Page (pid 318)'

	Mike de Boer [:mikedeboer]
	Comment 13 • 7 years ago
mozreview-review

Comment on attachment 8917147 [details]
Bug 1407351 - Remove nightly-only e10s testing features from the main browser window.

https://reviewboard.mozilla.org/r/188154/#review194126

Prima!

	Jared Wein [:jaws] (please needinfo? me)
	Comment 14 • 7 years ago
mozreview-review

Comment on attachment 8917148 [details]
Bug 1407351 - Remove Nightly-only e10s checkbox in about:preferences.

https://reviewboard.mozilla.org/r/188156/#review194180

	Jared Wein [:jaws] (please needinfo? me)
	Comment 15 • 7 years ago
mozreview-review

Comment on attachment 8917149 [details]
Bug 1407351 - Simplify check for e10s in about:preferences.

https://reviewboard.mozilla.org/r/188158/#review194182

	Greg Tatum [:gregtatum]
	Comment 16 • 7 years ago
mozreview-review

Comment on attachment 8917150 [details]
Bug 1407351 - Remove E10S_TESTING_ONLY from devtools.

https://reviewboard.mozilla.org/r/188160/#review194230

I think these changes look fine, thanks.

	Pulsebot
	Comment 17 • 7 years ago

Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3aef2a6367c8
Remove nightly-only e10s testing features from the main browser window. r=mikedeboer
https://hg.mozilla.org/integration/mozilla-inbound/rev/537e32fd3d48
Remove Nightly-only e10s checkbox in about:preferences. r=jaws
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8938454c1f0
Simplify check for e10s in about:preferences. r=jaws
https://hg.mozilla.org/integration/mozilla-inbound/rev/f60f54d7af0c
Remove E10S_TESTING_ONLY from devtools. r=gregtatum
https://hg.mozilla.org/integration/mozilla-inbound/rev/5725fbd3af70
Remove E10S_TESTING_ONLY defines. r=glandium

	Pulsebot
	Comment 18 • 7 years ago

Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a58f8ab311d
Back out to avoid a merge conflict with bug 1406212. I'll reland tomorrow

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 19 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/3aef2a6367c8
https://hg.mozilla.org/mozilla-central/rev/537e32fd3d48
https://hg.mozilla.org/mozilla-central/rev/b8938454c1f0
https://hg.mozilla.org/mozilla-central/rev/f60f54d7af0c
https://hg.mozilla.org/mozilla-central/rev/5725fbd3af70

	Pulsebot
	Comment 25 • 7 years ago

Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/86a360e12b8a
Remove nightly-only e10s testing features from the main browser window. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/a53f34a45f9b
Remove Nightly-only e10s checkbox in about:preferences. r=jaws
https://hg.mozilla.org/integration/autoland/rev/effeee94316e
Simplify check for e10s in about:preferences. r=jaws
https://hg.mozilla.org/integration/autoland/rev/8e04412824b5
Remove E10S_TESTING_ONLY from devtools. r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/e6ee25ad6a9f
Remove E10S_TESTING_ONLY defines. r=glandium

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 26 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/86a360e12b8a
https://hg.mozilla.org/mozilla-central/rev/a53f34a45f9b
https://hg.mozilla.org/mozilla-central/rev/effeee94316e
https://hg.mozilla.org/mozilla-central/rev/8e04412824b5
https://hg.mozilla.org/mozilla-central/rev/e6ee25ad6a9f

	Ryan VanderMeulen [:RyanVM]
	Comment 27 • 7 years ago

Backed out for causing bug 1409395 and breaking today's nightlies.

https://hg.mozilla.org/mozilla-central/rev/f27105b62753c71ecadad2f8d632ec7e5ac96bbd

	Mike de Boer [:mikedeboer]
	Comment 28 • 7 years ago
mozreview-review

Comment on attachment 8917147 [details]
Bug 1407351 - Remove nightly-only e10s testing features from the main browser window.

https://reviewboard.mozilla.org/r/188154/#review195458

::: browser/components/customizableui/CustomizableUI.jsm:461
(Diff revision 3)
> +    // "New non-e10s window" button.
> +    if (currentVersion < 13 && gSavedState.placements) {
> +      for (let placements of Object.values(gSavedState.placements)) {
> +        let buttonIndex = placements.indexOf("e10s-button");
> +        if (buttonIndex != -1) {
> +          placements.spice(buttonIndex, 1);

s/spice/splice/ is your culprit, Felipe... Sorry I didn't catch this as well - only human, after all.

	:Felipe Gomes (needinfo for replies!) Assignee
	Comment 34 • 7 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc78c6804b62663393a1fa79a226f961da5d5cdf

https://tools.taskcluster.net/index/artifacts/gecko.v2.try.revision.dc78c6804b62663393a1fa79a226f961da5d5cdf

	:Felipe Gomes (needinfo for replies!) Assignee
	Comment 35 • 7 years ago

Alright, Ryan confirmed using these try builds that the new patch works, so I'm gonna reland this.

Sorry about the trouble that this may have caused to anyone!

	Pulsebot
	Comment 36 • 7 years ago

Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9e4f906fcfb4
Remove nightly-only e10s testing features from the main browser window. r=mikedeboer
https://hg.mozilla.org/integration/autoland/rev/54dd64f2be2e
Remove Nightly-only e10s checkbox in about:preferences. r=jaws
https://hg.mozilla.org/integration/autoland/rev/8bbabfafd7e2
Simplify check for e10s in about:preferences. r=jaws
https://hg.mozilla.org/integration/autoland/rev/0691578815e5
Remove E10S_TESTING_ONLY from devtools. r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/a509b9a421a1
Remove E10S_TESTING_ONLY defines. r=glandium

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 37 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/9e4f906fcfb4
https://hg.mozilla.org/mozilla-central/rev/54dd64f2be2e
https://hg.mozilla.org/mozilla-central/rev/8bbabfafd7e2
https://hg.mozilla.org/mozilla-central/rev/0691578815e5
https://hg.mozilla.org/mozilla-central/rev/a509b9a421a1

	Ekanan Ketunuti
	Comment 38 • 7 years ago

It seems like some of the backed out patch in comment 27 got merged on top of patches in comment 37.

see http://searchfox.org/mozilla-central/rev/1c4da216e00ac95b38a3f236e010b31cdfaae03b/browser/base/content/tabbrowser.xml#3957-3969
http://searchfox.org/mozilla-central/rev/1c4da216e00ac95b38a3f236e010b31cdfaae03b/browser/base/content/tabbrowser.xml#5547,5549-5553
http://searchfox.org/mozilla-central/rev/1c4da216e00ac95b38a3f236e010b31cdfaae03b/browser/base/content/browser.xul#99-104

any thought?

	:Felipe Gomes (needinfo for replies!) Assignee
	Comment 39 • 7 years ago

(In reply to Ekanan Ketunuti from comment #38)
> It seems like some of the backed out patch in comment 27 got merged on top
> of patches in comment 37.

Thanks for the notice! I fixed these via bug 1410279