Closed Bug 1357486 Opened 8 years ago Closed 7 years ago

Turn on OOP extensions by default on Windows

Categories

(WebExtensions :: General, enhancement, P1)

Product:
WebExtensions
Component:
General
Type:
enhancement

Tracking

(Performance Impact:high, firefox56 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla56
Project Flags:
Performance Impact high
Tracking Flags:
Tracking Status
firefox56 --- fixed
Blocking Flags:
webextensions +

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 1 open bug)

Details

(Whiteboard: triaged)

Attachments

(9 files)

Bug 1357486: Enable OOP extensions by default on Windows.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Bug 1357486: Part 0 - Fix permissions tests with OOP extensions.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Bug 1357486: Part 0b - Fix inline options browser tests with OOP extensions.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Bug 1357486: Part 0c - Propagate addonId to parent process with console messages.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Bug 1357486: Part 0d - Propagate clonable console message args to the parent process.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Bug 1357486: Part 0e - Support legacy extensions in OOP mode.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Bug 1357486: Part 0f - Run some chrome tests in in-process mode.
59 bytes, text/x-review-board-request
aswan
: review+
Details
Screenshot with patch enabled
15.11 KB, image/png
Details
Screenshot with patch disabled
35.08 KB, image/png
Details 
      No description provided.
Blocks: webext-oop
Blocks: 1357487
webextensions: --- → ?
webextensions: ? → +
Priority: -- → P1
Whiteboard: triaged
Marking as quantum flow p1 because this is about the ability to run webextensions in a seperate process.
Whiteboard: triaged → triaged[qf:p1]
Depends on: 1379046
Comment on attachment 8884165 [details]
Bug 1357486: Enable OOP extensions by default on Windows.

https://reviewboard.mozilla.org/r/155076/#review160144

The bug for this patch mentions Windows and OSX but the patch is Windows only?
Attachment #8884165 - Flags: review?(aswan) → review+
Comment on attachment 8884165 [details]
Bug 1357486: Enable OOP extensions by default on Windows.

https://reviewboard.mozilla.org/r/155076/#review160144

I was going to move OS-X support to a separate bug. If it works as-is after the last dependency for this bug lands, I'll turn it on. Otherwise, it's going to have to wait until after 57.
Comment on attachment 8884381 [details]
Bug 1357486: Part 0 - Fix permissions tests with OOP extensions.

https://reviewboard.mozilla.org/r/155306/#review160466
Attachment #8884381 - Flags: review?(aswan) → review+
Comment on attachment 8884438 [details]
Bug 1357486: Part 0b - Fix inline options browser tests with OOP extensions.

https://reviewboard.mozilla.org/r/155342/#review160470
Attachment #8884438 - Flags: review?(aswan) → review+
Comment on attachment 8884439 [details]
Bug 1357486: Part 0c - Propagate addonId to parent process with console messages.

https://reviewboard.mozilla.org/r/155344/#review160472
Attachment #8884439 - Flags: review?(aswan) → review+
Comment on attachment 8884441 [details]
Bug 1357486: Part 0e - Support legacy extensions in OOP mode.

https://reviewboard.mozilla.org/r/155348/#review160474

::: toolkit/components/extensions/ExtensionParent.jsm:225
(Diff revision 2)
> +
> +    // If we have a remote, embedded extension, the legacy side is
> +    // running in a different process than the WebExtension side.
> +    // As a result, we need to dispatch the message to both the parent
> +    // and extension processes, and manually merge the results.
> +    if (extension.isEmbedded && extension.remote) {

nit: I think this would be easily to read if you just return `promise1` here in the common case of not having to deal with hybrid extensions and avoid putting the code below inside a block.
Attachment #8884441 - Flags: review?(aswan) → review+
Comment on attachment 8884440 [details]
Bug 1357486: Part 0d - Propagate clonable console message args to the parent process.

https://reviewboard.mozilla.org/r/155346/#review160476

::: toolkit/components/processsingleton/ContentProcessSingleton.js:82
(Diff revision 2)
> +          try {
> +            arg = Cu.cloneInto(arg, {});
> +          } catch (e) {
> -          arg = unavailString;
> +            arg = unavailString;
> +          }

can you add a comment explaining this
Attachment #8884440 - Flags: review?(aswan) → review+
Comment on attachment 8884442 [details]
Bug 1357486: Part 0f - Run some chrome tests in in-process mode.

https://reviewboard.mozilla.org/r/155350/#review160478

can you either mark this bug leave-open or a file a separate bug for these?  (I can take a look at them if you like)
Attachment #8884442 - Flags: review?(aswan) → review+
(In reply to Andrew Swan [:aswan] from comment #23)
> can you either mark this bug leave-open or a file a separate bug for these? 
> (I can take a look at them if you like)

The fixes should probably be almost identical to what's needed for bug 1352239, so if you could fix it along with that, that would be great.
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa4a2714ae9b05ed28e76e1b61570c97ed69a81d
Bug 1357486: Part 0a - Fix permissions tests with OOP extensions. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e252b1de0d02d24067ac817d4d4de6351e4e039
Bug 1357486: Part 0b - Fix inline options browser tests with OOP extensions. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/c72d36a91e448d73a48c7f0e37175537530089b8
Bug 1357486: Part 0c - Propagate addonId to parent process with console messages. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/0159b012fa2d1135647f31689824ca6e31497a7d
Bug 1357486: Part 0d - Propagate clonable console message args to the parent process. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/44ab0cca4956dec0ea97cfce9d1503f43d3b9916
Bug 1357486: Part 0e - Support legacy extensions in OOP mode. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/5fba1531fbaf630674b295684a25164b64aa8fe5
Bug 1357486: Part 0f - Run some chrome tests in in-process mode. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/7124641a168ffaa20461764f2f554992dfd856e6
Bug 1357486: Enable OOP extensions by default on Windows. r=aswan
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb1aa3b7bdf373c86efde1face922b02eadb94fb
Bug 1357486: Follow-up: Skip optional permissions test with e10s disabled.
Backed out for frequently failing xpcshell's toolkit/mozapps/extensions/test/xpcshell/test_delay_update_webextension.js on Windows debug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ee92fd6fac7e89be7317f36b31167b5c757ffbb
https://hg.mozilla.org/integration/mozilla-inbound/rev/0068662edb966b29283c8a1a3b30eaaf4d06d99a
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc11bba5e51f7fae68cbbeb3703ddf4b0c200085
https://hg.mozilla.org/integration/mozilla-inbound/rev/85e8fc25e21d6cb340bad5f4ca551ff92e20c2e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/9761dc56f9178fc96aefe20bfe2a764d9e56223b
https://hg.mozilla.org/integration/mozilla-inbound/rev/40566f684eddedbdfe0b77a109d645cae07a4248
https://hg.mozilla.org/integration/mozilla-inbound/rev/664ef7ecc0589aa3d716d2530a2de6a3999bd480
https://hg.mozilla.org/integration/mozilla-inbound/rev/f13478a0064a7726cdd5a257fc8d6f26d8a527ba

Follow-up push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=bb1aa3b7bdf373c86efde1face922b02eadb94fb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=success&filter-searchStr=xpcshell

Failure log xpcshell: https://treeherder.mozilla.org/logviewer.html#?job_id=112757035&repo=mozilla-inbound

Is it caused by this?
01:12:50     INFO -  PID 9556 | 1499501568333	addons.xpi	ERROR	Failed to remove file c:\\users\\cltbld\\appdata\\local\\temp\\xpc-profile-qex9qa\\extensions\\trash\\test_no_update_webext@tests.mozilla.org.xpi: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: recursiveRemove :: line 1054"  data: no] Stack trace: recursiveRemove()@resource://gre/modules/addons/XPIProvider.jsm:1054 < recursiveRemove()@resource://gre/modules/addons/XPIProvider.jsm:1068 < uninstallAddon()@resource://gre/modules/addons/XPIProvider.jsm:6254 < uninstallAddon()@resource://gre/modules/addons/XPIProvider.jsm:4658 < uninstall()@resource://gre/modules/addons/XPIProvider.jsm:5515 < unload()@resource://testing-common/ExtensionXPCShellUtils.jsm:303
01:12:50     INFO -  PID 9556 | 1499501568340	addons.xpi	WARN	Failed to remove trash directory when uninstalling test_no_update_webext@tests.mozilla.org: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: recursiveRemove :: line 1054"  data: no] Stack trace: recursiveRemove()@resource://gre/modules/addons/XPIProvider.jsm:1054 < recursiveRemove()@resource://gre/modules/addons/XPIProvider.jsm:1068 < uninstallAddon()@resource://gre/modules/addons/XPIProvider.jsm:6254 < uninstallAddon()@resource://gre/modules/addons/XPIProvider.jsm:4658 < uninstall()@resource://gre/modules/addons/XPIProvider.jsm:5515 < unload()@resource://testing-common/ExtensionXPCShellUtils.jsm:303
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(kmaglione+bmo)
Summary: Turn on OOP extensions by default on Windows and OS-X → Turn on OOP extensions by default on Windows
https://hg.mozilla.org/integration/mozilla-inbound/rev/07cb3b166564bcf859976c5941761a194b97b695
Bug 1357486: Part 0a - Fix permissions tests with OOP extensions. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa1b70cb93415506b63604d44163bf697784a068
Bug 1357486: Part 0b - Fix inline options browser tests with OOP extensions. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/df8b19189f4c2ae7f2e91d07ca473c471310618b
Bug 1357486: Part 0c - Propagate addonId to parent process with console messages. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/312a55aa2bcc69882ab3388a15390dce6c0c6530
Bug 1357486: Part 0d - Propagate clonable console message args to the parent process. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/6cf94b3218de0e695b970cf204569bcc94b95a1a
Bug 1357486: Part 0e - Support legacy extensions in OOP mode. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bf31f8f11ef551180d5b7ae358217f049cddc68
Bug 1357486: Part 0f - Run some chrome tests in in-process mode. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/d308c26c9b213fb957c8a5192bd425674f4cbd0d
Bug 1357486: Part 0g - Run remote debugger host browser in same TabGroup as extension pages. r=me

https://hg.mozilla.org/integration/mozilla-inbound/rev/a625a2e9b3333a8e76982ea65f077cfded6ac224
Bug 1357486: Enable OOP extensions by default on Windows. r=aswan
https://hg.mozilla.org/integration/mozilla-inbound/rev/6664e29e12ac9e96265d8425a6ff86dc16a16c06
Bug 1357486: Follow-up - Run flaky update tests in in-process mode on win32 debug.
It appears that this patch has partially broken at least one WE add-on I use.  It is called Zoom Page WE. The dropdown lists on the main menu to change font size and zoom level do not display with this patch.  I have notified the developer of this problem.
UA:  Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

I set extensions.webextensions.remote to false as a workaround.
(In reply to Gary [:streetwolf] from comment #31)
> It appears that this patch has partially broken at least one WE add-on I
> use.  It is called Zoom Page WE. The dropdown lists on the main menu to
> change font size and zoom level do not display with this patch.  I have
> notified the developer of this problem.

This patch hasn't merged to central or appeared in any nightly yet. Where are you seeing this breakage?
I'm on inbound.
When Clicking on the items circled in red a drop down list of font sizes and zoom level percentages should appear.  With patch no list.  With pref set to false I get the list.
Pref set to false.  Zoom level also has a drop down list.
https://hg.mozilla.org/integration/mozilla-inbound/rev/23c63ef286cc35e1e31aa6a9afcaf00d227bf9d6
Bug 1357486: Follow-up: Wait for extension shutdown before starting storage shutdown. r=rhelmer
(In reply to Gary [:streetwolf] from comment #35)
> Created attachment 8884659 [details]
> Screenshot with patch enabled
> 
> When Clicking on the items circled in red a drop down list of font sizes and
> zoom level percentages should appear.  With patch no list.  With pref set to
> false I get the list.

Ah, I guess we fail to paint select boxes for OOP popups. Can you file a bug?
Depends on: 1379508
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7c72fa8ccd849c4cc09ca1994b6e22b5ea4bc17
Bug 1357486: Follow-up: Don't propagate rejections to AsyncShutdown.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bafb677d3d3c40cb824ef591a5bd096291ee373a
Bug 1357486: Follow-up: Fix error on shutdown after incomplete startup.
Depends on: 1379721
Depends on: 1380109
some perf improvements:
== Change summary for alert #7870 (as of July 12 2017 17:06 UTC) ==

Improvements:

 12%  tp5o_webext responsiveness windows10-64 pgo e10s     4.16 -> 3.64
  6%  tp5o responsiveness windows10-64 opt e10s            4.06 -> 3.79
  3%  tp5o_webext responsiveness windows10-64 opt e10s     4.26 -> 4.12

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7870
Depends on: 1380449
Depends on: 1381097
Depends on: 1383489
Depends on: 1389496
Depends on: 1382953
Product: Toolkit → WebExtensions
Performance Impact: --- → P1
Whiteboard: triaged[qf:p1] → triaged
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: