Closed
Bug 1474739
Opened 5 years ago
Closed 5 years ago
Kill the remaining consumers of XPT shims
Categories
(Core :: XPConnect, enhancement, P3)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
(Depends on 1 open bug)
Details
Attachments
(2 files)
When moving to the new XPT format, the mechanism for defining "shim" interfaces - interfaces which are not scriptable but expose an `Ci.XXX` object. was moved over to be a `shim` attribute in *.idl files. Thanks to :bz and other's hard work, many of these interfaces have been eliminated completely, taking down the [shim] attribute with them! Currently there are a total of 4 consumers of [shim] left in tree, which seems like a small enough number that we should kill them & remove XPT shims entirely. They are: 1. nsIMessageSender https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/dom/base/nsIMessageManager.idl#10 2. nsIContentFrameMessageManager https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/dom/base/nsIMessageManager.idl#16 3. nsIDOMOfflineResourceList https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/dom/interfaces/offline/nsIDOMOfflineResourceList.idl#8 4. nsIListBoxObject https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/layout/xul/nsIListBoxObject.idl#12 For each of these messages I found the following consumers: 1. nsIMessageSender : a) service getter : https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/toolkit/modules/Services.jsm#64 - Should be possible to replace with just "nsISupports" and get the same impact, as the type is WebIDL already. b) instanceof check : https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/toolkit/components/extensions/MessageManagerProxy.jsm#31 - Switch to an instanceof check against the "MessageSender" webidl interface (although there may be problems with cross-compartment object references. c) to get parent-side messagemanager from ContentParent object : https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/toolkit/components/extensions/ExtensionParent.jsm#790 - This appears to be the only use of the nsIInterfaceRequestor implementation on ContentParent. - It's also a touch odd that the ContentParent object is exposed to JS at all - the nsIObserver nsISupports base was used in order to allow it to be the subject of the "ipc:content-created" observer notification. - Perhaps provide some other mechanism for getting this object beyond going through the getInterface codepath? We could add another nsIFoo interface which ContentParent implements, or even just add a ChromeUtils method which does the getting for us? I'm not sure what the best approach to take here is. 2. nsIContentFrameMessageManager : a) Many occurances in `docShell.getInterface()` situations. It appears that the only way to get this MessageManager right now from a docShell is to go through getInterface. b) We should probably add an alternative getter method and switch to it. 3. nsIDOMOfflineResourceList : a) test : https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/dom/tests/mochitest/ajax/offline/foreign2.html#49 - Usage should be unnecessary b) test : https://searchfox.org/mozilla-central/rev/a80651653faa78fa4dfbd238d099c2aad1cec304/dom/tests/mochitest/ajax/offline/test_updatingManifest.html#159,213,262,306,309 - Usage should be unnecessary 4. nsIListBoxObject : No consumers of this interface from JS found. Should be easy to remove. NOTE: If the uses of the nsIDOMOfflineResourceList code could be modified to stop accessing constants on Ci.*, it would be possible to remove the bulk of the complexity of the [shim] attribute, which is forwarding XPIDL constant access to WebIDL. The GetInterface consumers could equally-well be handled by an empty `[scriptable, builtinclass, uuid(...)]` interface, which would have almost the same behaviour.
Updated•5 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•5 years ago
|
||
Depends On D2624
Comment 3•5 years ago
|
||
Comment on attachment 8996831 [details] Bug 1474739 - Part 1: Stop using XPT shims in any xpidl interfaces, r=bzbarsky Boris Zbarsky [:bz] (no decent commit message means r-) has approved the revision. https://phabricator.services.mozilla.com/D2624
Attachment #8996831 -
Flags: review+
Comment 4•5 years ago
|
||
Comment on attachment 8996832 [details] Bug 1474739 - Part 2: Remove all code for XPT shims, r=bzbarsky Boris Zbarsky [:bz] (no decent commit message means r-) has approved the revision. https://phabricator.services.mozilla.com/D2625
Attachment #8996832 -
Flags: review+
Pushed by nika@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1a5c1ee48c91 Part 1: Stop using XPT shims in any xpidl interfaces, r=bzbarsky https://hg.mozilla.org/integration/mozilla-inbound/rev/da7308cd7b0f Part 2: Remove all code for XPT shims, r=bzbarsky
Pushed by nika@thelayzells.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/62a0e59c7e1a Part 3: Linting fixes on a CLOSED TREE, a=bustage
Comment 7•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1a5c1ee48c91 https://hg.mozilla.org/mozilla-central/rev/da7308cd7b0f https://hg.mozilla.org/mozilla-central/rev/62a0e59c7e1a
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•5 years ago
|
Assignee: nobody → nika
You need to log in
before you can comment on or make changes to this bug.
Description
•