Turn on OOP extensions by default on Windows

RESOLVED FIXED in Firefox 56

Status

()

Toolkit
WebExtensions: General
P1
normal
RESOLVED FIXED
3 months ago
3 days ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

(Depends on: 4 bugs, Blocks: 2 bugs)

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: triaged[qf:p1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(9 attachments)

Comment hidden (empty)
(Assignee)

Updated

3 months ago
Blocks: 1190679
(Assignee)

Updated

3 months ago
Blocks: 1357487
(Assignee)

Updated

3 months ago
webextensions: --- → ?

Updated

3 months ago
webextensions: ? → +
Priority: -- → P1
Whiteboard: triaged

Comment 1

3 months ago
Marking as quantum flow p1 because this is about the ability to run webextensions in a seperate process.
Whiteboard: triaged → triaged[qf:p1]
(Assignee)

Updated

21 days ago
Depends on: 1379046
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

21 days ago
mozreview-review
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+
(Assignee)

Comment 5

21 days ago
mozreview-review-reply
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 18

20 days ago
mozreview-review
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 19

20 days ago
mozreview-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 20

20 days ago
mozreview-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 21

20 days ago
mozreview-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 22

20 days ago
mozreview-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 23

20 days ago
mozreview-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+
(Assignee)

Comment 24

20 days ago
(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.
(Assignee)

Comment 25

20 days ago
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
(Assignee)

Comment 26

20 days ago
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)
This causes also these devtools failures: https://treeherder.mozilla.org/logviewer.html#?job_id=112757307&repo=mozilla-inbound
(Assignee)

Updated

19 days ago
Flags: needinfo?(kmaglione+bmo)
Summary: Turn on OOP extensions by default on Windows and OS-X → Turn on OOP extensions by default on Windows
(Assignee)

Comment 29

19 days ago
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
(Assignee)

Comment 30

19 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6664e29e12ac9e96265d8425a6ff86dc16a16c06
Bug 1357486: Follow-up - Run flaky update tests in in-process mode on win32 debug.

Comment 31

19 days ago
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.

Comment 32

19 days ago
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.
(Assignee)

Comment 33

18 days ago
(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?

Comment 34

18 days ago
I'm on inbound.

Comment 35

18 days ago
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.

Comment 36

18 days ago
Created attachment 8884660 [details]
Screenshot with patch disabled

Pref set to false.  Zoom level also has a drop down list.
(Assignee)

Comment 37

18 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/23c63ef286cc35e1e31aa6a9afcaf00d227bf9d6
Bug 1357486: Follow-up: Wait for extension shutdown before starting storage shutdown. r=rhelmer
(Assignee)

Comment 38

18 days ago
(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?

Updated

18 days ago
Depends on: 1379508
(Assignee)

Comment 39

18 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7c72fa8ccd849c4cc09ca1994b6e22b5ea4bc17
Bug 1357486: Follow-up: Don't propagate rejections to AsyncShutdown.
(Assignee)

Comment 40

18 days ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bafb677d3d3c40cb824ef591a5bd096291ee373a
Bug 1357486: Follow-up: Fix error on shutdown after incomplete startup.

Comment 41

18 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/07cb3b166564
https://hg.mozilla.org/mozilla-central/rev/fa1b70cb9341
https://hg.mozilla.org/mozilla-central/rev/df8b19189f4c
https://hg.mozilla.org/mozilla-central/rev/312a55aa2bcc
https://hg.mozilla.org/mozilla-central/rev/6cf94b3218de
https://hg.mozilla.org/mozilla-central/rev/1bf31f8f11ef
https://hg.mozilla.org/mozilla-central/rev/d308c26c9b21
https://hg.mozilla.org/mozilla-central/rev/a625a2e9b333
https://hg.mozilla.org/mozilla-central/rev/6664e29e12ac
https://hg.mozilla.org/mozilla-central/rev/23c63ef286cc
https://hg.mozilla.org/mozilla-central/rev/b7c72fa8ccd8
https://hg.mozilla.org/mozilla-central/rev/bafb677d3d3c
Status: NEW → RESOLVED
Last Resolved: 18 days ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56

Updated

17 days ago
Depends on: 1379721

Updated

16 days ago
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: 1380406
Depends on: 1380449
(Assignee)

Updated

13 days ago
Depends on: 1381097

Updated

3 days ago
Depends on: 1383489
You need to log in before you can comment on or make changes to this bug.