Turn on OOP extensions by default on Windows

RESOLVED FIXED in Firefox 56

Status

P1
normal
RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
mozilla56
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: triaged[qf:p1])

Attachments

(9 attachments)

Comment hidden (empty)
(Assignee)

Updated

2 years ago
Blocks: 1190679
(Assignee)

Updated

2 years ago
Blocks: 1357487
(Assignee)

Updated

2 years ago
webextensions: --- → ?

Updated

2 years ago
webextensions: ? → +
Priority: -- → P1
Whiteboard: triaged

Comment 1

2 years 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

2 years ago
Depends on: 1379046
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

2 years 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

2 years 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

2 years 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

2 years 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

2 years 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

2 years 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

2 years 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

2 years 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

2 years 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 26

2 years 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)
(Assignee)

Updated

2 years 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

2 years 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

2 years 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.
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.
(Assignee)

Comment 33

2 years 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?
I'm on inbound.
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.
Created attachment 8884660 [details]
Screenshot with patch disabled

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

Comment 37

2 years 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

2 years 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

2 years ago
Depends on: 1379508
Depends on: 1379721

Updated

2 years 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

2 years ago
Depends on: 1381097

Updated

2 years ago
Depends on: 1383489

Updated

2 years ago
Depends on: 1389496
Depends on: 1382953

Updated

8 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.