Closed Bug 1156613 Opened 5 years ago Closed 5 years ago

Fix e10s blocks conditioned to NIGHTLY_BUILD (and others) in preparation for Aurora merge

Categories

(Firefox :: General, defect)

defect
Not set
Points:
3

Tracking

()

RESOLVED FIXED
Firefox 40
Iteration:
40.3 - 11 May
Tracking Status
e10s m6+ ---
firefox40 --- fixed

People

(Reporter: Felipe, Assigned: Felipe)

Details

Attachments

(4 files, 1 obsolete file)

There are some parts of the code related to e10s that might be tied to #ifdef NIGHTLY_BUILDS (or similar) blocks and will break when we make the move to Aurora.
We need to find out these cases and ensure they work correctly on Aurora.

There's also E10S_TESTING_ONLY which is currently tied to NIGHTLY_BUILD. It would be good if at least some of the simple cases for these are fixed to do the proper checks of window/browser remoteness instead of the ifdefs.

At a minimum, before the uplift we must:
- find out blocks tied to nightly and make them tied to E10S_TESTING_ONLY
- define E10S_TESTING_ONLY for Aurora


But it would be nice to do a little better than that, at least for a subset of this, depending on the cost/benefit of the change


Something similar was already done one year ago in bug 1016834 but it's likely that in the meantime more cases showed up.
I'm already taking this on the assumption that this will be marked as an m6
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Flags: qe-verify-
Flags: firefox-backlog+
Iteration: --- → 40.2 - 27 Apr
Hi Felipe, can you provide a point value.
Flags: needinfo?(felipc)
Points: --- → 3
Flags: needinfo?(felipc)
Attached patch firefox.jsSplinter Review
Attachment #8596616 - Flags: review?(wmccloskey)
Attached patch nsAppRunner.cppSplinter Review
Attachment #8596617 - Flags: review?(wmccloskey)
Paolo, this came from bug 1021172. With e10s on Aurora, we need to fix this #ifdef'ed blocks. Is there any reason not to simply remove the ifdef and let this code be always on, even when e10s is off (e.g. after riding the trains, on Beta/Release)?
Attachment #8596621 - Flags: review?(paolo.mozmail)
Attachment #8596616 - Flags: review?(wmccloskey) → review+
Attachment #8596617 - Flags: review?(wmccloskey) → review+
Comment on attachment 8596621 [details] [diff] [review]
requestAutoComplete

The entire feature is currently meant to be enabled in Nightly only, so I think there is nothing to do here. Thanks for taking a look!
Attachment #8596621 - Flags: review?(paolo.mozmail)
Bill: by looking at bug 1133099 I assume we don't need to carry https://mxr.mozilla.org/mozilla-central/source/dom/webidl/Navigator.webidl?rev=a6eac2cc6ba7#433 to Aurora, right?

And also bug 1069704?
Flags: needinfo?(wmccloskey)
Attached patch all.js (obsolete) — Splinter Review
Do you want addon-watcher and/or compartment-per-addon in Aurora?

And do you know about the async plugin initilization?
Attachment #8598223 - Flags: review?(wmccloskey)
Luke, what do we want to do about bug 1133099 once e10s goes to Aurora? We've fixed pointer locking and bug 1144906 is a blocker for us going to Aurora. Is that sufficient to say we don't need this flag anymore?
Flags: needinfo?(wmccloskey) → needinfo?(luke)
Aaron, what are we going to do with async plugin init once e10s goes to Aurora? We anticipate that FF40 will have e10s enabled when it's on Aurora and then we'll disable e10s when it goes to beta.
Flags: needinfo?(aklotz)
Yoric, same question as above: what will we do about the addon-watcher code when e10s goes to Aurora?
Flags: needinfo?(dteller)
(In reply to Bill McCloskey (:billm) from comment #9)
Yes, with those fixed, I don't know of any need for that property.
Flags: needinfo?(luke)
Iteration: 40.2 - 27 Apr → 40.3 - 11 May
Attached patch alljsSplinter Review
So what I talked with Aklotz is that I'll change the ifdef for correctness sake (in order to fix this bug and in case the async plugin work takes longer), but he will most likely remove the ifdef altogether before the merge with bug 1145838.

The addon-watcher I'd like to leave untouched here and left to its own bug, because I think it makes sense to wait a few days of things running smoothly in Aurora before enabling it. But that discussion can happen elsewhere.
Attachment #8598223 - Attachment is obsolete: true
Attachment #8598223 - Flags: review?(wmccloskey)
Flags: needinfo?(dteller)
Flags: needinfo?(aklotz)
Attachment #8600038 - Flags: review?(wmccloskey)
Attachment #8600038 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/mozilla-central/rev/2357d7d9c25e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
You need to log in before you can comment on or make changes to this bug.