Closed Bug 1368152 Opened 3 years ago Closed 3 years ago

Remove ExtensionManagement.jsm

Categories

(WebExtensions :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

This is one more module for us to load at startup, which causes an unnecessary perf hit, and unnecessary complexity in the wake of recent refactorings.
Comment on attachment 8871884 [details]
Bug 1368152: Part 1 - Move API extension registration to ExtensionAPI.jsm.

https://reviewboard.mozilla.org/r/143406/#review147348
Attachment #8871884 - Flags: review?(aswan) → review+
Comment on attachment 8871885 [details]
Bug 1368152: Part 2 - Move extension policy registration out of ExtensionManagement.jsm.

https://reviewboard.mozilla.org/r/143408/#review147352

::: toolkit/components/extensions/Extension.jsm:80
(Diff revision 1)
> -Cu.import("resource://gre/modules/ExtensionManagement.jsm");
> +XPCOMUtils.defineLazyGetter(
> +  this, "processScript",
> +  () => Cc["@mozilla.org/webextensions/extension-process-script;1"]
> +          .getService().wrappedJSObject);

What's this?  I'm guessing its in some other pending but not-yet-landed patch?
Comment on attachment 8871885 [details]
Bug 1368152: Part 2 - Move extension policy registration out of ExtensionManagement.jsm.

https://reviewboard.mozilla.org/r/143408/#review147352

> What's this?  I'm guessing its in some other pending but not-yet-landed patch?

Yeah, bug 1322235.
Comment on attachment 8871885 [details]
Bug 1368152: Part 2 - Move extension policy registration out of ExtensionManagement.jsm.

https://reviewboard.mozilla.org/r/143408/#review148116
Attachment #8871885 - Flags: review?(mixedpuppy) → review+
Comment on attachment 8871885 [details]
Bug 1368152: Part 2 - Move extension policy registration out of ExtensionManagement.jsm.

https://reviewboard.mozilla.org/r/143408/#review148450

::: toolkit/components/extensions/extension-process-script.js:297
(Diff revision 1)
>  
>        Services.cpmm.addMessageListener("Schema:Add", this);
>      }
>    },
>  
> -  initExtension(data) {
> +  initExtensionPolicy(data, extension) {

this appears to only ever get called with one argument

::: toolkit/components/extensions/extension-process-script.js:384
(Diff revision 1)
>  
>  ExtensionProcessScript.prototype = {
>    classID: Components.ID("{21f9819e-4cdf-49f9-85a0-850af91a5058}"),
>    QueryInterface: XPCOMUtils.generateQI([Ci.mozIExtensionProcessScript]),
>  
> +  get wrappedJSObject() { return this; },

I don't understand the purpose of this line
Comment on attachment 8871886 [details]
Bug 1368152: Part 3 - Remove ExtensionManagement.getURLForExtension.

https://reviewboard.mozilla.org/r/143410/#review148452
Attachment #8871886 - Flags: review?(aswan) → review+
Comment on attachment 8871887 [details]
Bug 1368152: Part 4 - Remove ExtensionManagement.jsm.

https://reviewboard.mozilla.org/r/143412/#review148454
Attachment #8871887 - Flags: review?(aswan) → review+
Comment on attachment 8871885 [details]
Bug 1368152: Part 2 - Move extension policy registration out of ExtensionManagement.jsm.

https://reviewboard.mozilla.org/r/143408/#review148450

> this appears to only ever get called with one argument

Hrm. Yeah, it was supposed to be called with two arguments from ExtensionProcessScript.initExtension. This still works, but is a bit less efficient for the parent process.

> I don't understand the purpose of this line

It's basically a bit of XPConnect magic that lets JS code access the raw, underlying JS object that implements the service rather than going through XPConnect/XPIDL, since it's much more efficient and doesn't require declaring bindings for every JS-only method or property.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3fdbc5921e3815356cc81af1fc7284a2e67bb76
Bug 1368152: Part 1 - Move API extension registration to ExtensionAPI.jsm. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/73737d0a388808aabadd578053374f01a46cd315
Bug 1368152: Part 2 - Move extension policy registration out of ExtensionManagement.jsm. r=aswan,mixedpuppy

https://hg.mozilla.org/integration/mozilla-inbound/rev/1733d96aacacb985f240dd7aae724c9de011b393
Bug 1368152: Part 3 - Remove ExtensionManagement.getURLForExtension. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b3359583493bf0506c4fe1c81573b8a123413ec
Bug 1368152: Part 4 - Remove ExtensionManagement.jsm. r=aswan
I think that this patch might have broken a feature on the add-on uBlock Origin. At the top of the uBO menu if you click on it you are supposed to open the uBO dashboard.  Nothing happens.  This patch is in the regression range.

When I do click on it I see this in my Browser Console:

[Exception... "Illegal value"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://browser/content/ext-tabs.js :: query :: line 479"  data: no]  (unknown)
	query chrome://browser/content/ext-tabs.js:479:29
	next self-hosted:1157:9
tabs is undefined  vapi-background.js:481
	vAPI.tabs.open/< moz-extension://1b84d5a3-273d-4352-b6dc-0f845027c8b2/js/vapi-background.js:481:13
	runSafeSyncWithoutClone resource://gre/modules/ExtensionUtils.jsm:52:14
	runSafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:176:38
	wrapPromise/</< resource://gre/modules/ExtensionCommon.jsm:365:15
	withLastError resource://gre/modules/ExtensionCommon.jsm:303:14
	wrapPromise/< resource://gre/modules/ExtensionCommon.jsm:359:11
Unchecked lastError value: Error: An unexpected error occurred  ExtensionCommon.jsm:306
	withLastError resource://gre/modules/ExtensionCommon.jsm:306:9
	wrapPromise/< resource://gre/modules/ExtensionCommon.jsm:359:11
The issue is with the webext version of uBlock Origin.

Looking at the webext documentation for browser.tabs.query[1], it's as if the method does not accept the second callback parameter, which is what uBO uses to handle the response. From the documentation, a Promise is returned.

If so, that makes the method incompatible with chrome.tabs.query, which accepts a callback and does not return a Promise. Currently, the webext version of uBO is pretty much the same code base as the Chromium version of uBO.

It is this incompatibility by design, or is it something which will be fixed?

[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/query
No, the problem is that tabs.query now only accepts valid match patterns for the `url` parameter, and uBlock is apparently passing an invalid one.
Ah, apparently they're trying to pass their moz-extension: URL as a match pattern, which we don't currently support.
Sorry, dismiss my comment above about callback/Primise, I see the code does handle the callback case.

> uBlock is apparently passing an invalid one.

It's using a URL from what browser.runtime.getURL() returns.

Using the debugger to debug uBO, if I type browser.runtime.getURL('/'), I get "moz-extension://7d41851d-0c47-40b9-82e5-40af4eae19ce/".

When opening the dashboard, uBO passes "moz-extension://7d41851d-0c47-40b9-82e5-40af4eae19ce/dashboard.html", as per the result returned by browser.runtime.getURL().

Why is this not valid?
See bug 1271354.
Attachment #8871885 - Flags: review?(aswan)
Blocks: 1370463
Product: Toolkit → WebExtensions
Depends on: 1485282
No longer depends on: 1485282
You need to log in before you can comment on or make changes to this bug.