Closed
Bug 1695243
Opened 4 years ago
Closed 4 years ago
Check if we can get rid of "{}" when importing the addDebuggerToGlobal in Runtime.jsm
Categories
(Remote Protocol :: CDP, task, P3)
Remote Protocol
CDP
Tracking
(firefox88 fixed)
RESOLVED
FIXED
88 Branch
Tracking | Status | |
---|---|---|
firefox88 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
Details
Attachments
(1 file)
Follow-up from https://phabricator.services.mozilla.com/D106602#inline-595792. Here what Julian wrote:
I am curious about this one. Do we really need to pass this object here?
I think this is the only call site in mozilla-central where we import addDebuggerToGlobal from jsdebugger.jsm this way.
https://searchfox.org/mozilla-central/search?path=&q=%7B%20addDebuggerToGlobal%20%7D%20%3D%20ChromeUtils.import
It doesn't seem the item was discussed at all during the initial review (https://phabricator.services.mozilla.com/D27525), so maybe it's just unnecessary? We could try to get rid of it in a followup.
https://searchfox.org/mozilla-central/source/remote/cdp/domains/content/Runtime.jsm#9-12,25-26
Alexandre, can you remember why this was necessary?
Assignee | ||
Updated•4 years ago
|
Mentor: poirot.alex
Flags: needinfo?(poirot.alex)
Priority: -- → P3
Comment 1•4 years ago
|
||
To be clear, I was only referring to the additional {}
passed to ChromeUtils.import, not the whole import. Basically the question is, can we change:
const { addDebuggerToGlobal } = ChromeUtils.import(
"resource://gre/modules/jsdebugger.jsm",
{}
);
to
const { addDebuggerToGlobal } = ChromeUtils.import(
"resource://gre/modules/jsdebugger.jsm"
);
Assignee | ||
Comment 2•4 years ago
|
||
Oh sorry. Thanks for the clarification.
Summary: Check if we can get rid of the addDebuggerToGlobal import in Runtime.jsm → Check if we can get rid of "{}" when importing the addDebuggerToGlobal in Runtime.jsm
Assignee | ||
Updated•4 years ago
|
Flags: needinfo?(poirot.alex)
Assignee | ||
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Comment 4•4 years ago
|
||
That should be fine, it looks like pure legacy inherited from Components.utils.import and was used to avoid polluting the global scope.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1f6ce1f03e84
[remote] Remove 3rd argument when importing "addDebuggerToGlobal". r=remote-protocol-reviewers,jdescottes
Comment 6•4 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox88:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
Updated•4 years ago
|
Component: CDP: Runtime → CDP
You need to log in
before you can comment on or make changes to this bug.
Description
•