Closed
Bug 1372295
Opened 7 years ago
Closed 7 years ago
Call SetLocationForGlobal in a single location in the loader
Categories
(Core :: XPConnect, enhancement)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
SetLocationForGlobal is called on globals created for JSMs to give them a nice name for about:memory. Right now this is done in ImportInto and LoadModule, but I think it makes more sense to set the name once, right after the global is created. Calling GetSpec on aURI seems to return the same spec we'd use in these two call sites. This change makes it easier to label the shared JSM global nicely in bug 1186409.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8876817 [details]
Bug 1372295 - Call SetLocationForGlobal in a single location in the loader.
https://reviewboard.mozilla.org/r/148136/#review152538
::: js/xpconnect/loader/mozJSComponentLoader.cpp:524
(Diff revision 1)
>
> CreateLoaderGlobal(aCx, MapURIToAddonID(aURI), &globalObj);
>
> + // Set the location information for the new global, so that tools like
> + // about:memory may use that information
> + xpc::SetLocationForGlobal(globalObj, nativePath);
It seems like maybe we should do this from `CreateLoaderGlobal` (and I guess pass in the native path?) since that's where everything else we do to the shared global (as opposed to the `thisObj`) happens.
But I guess we may want to skip setting the location entirely when we're using a shared global?
Attachment #8876817 -
Flags: review?(kmaglione+bmo) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Kris Maglione [:kmag] (busy; behind on reviews) from comment #2)
> It seems like maybe we should do this from `CreateLoaderGlobal` (and I guess
> pass in the native path?) since that's where everything else we do to the
> shared global (as opposed to the `thisObj`) happens.
Yeah, I can pass in a string to do that. It felt a little silly to add an additional argument just to do a single line function call, but it would be more consistent. And I guess it actually will make the code to support shared globals cleaner.
> But I guess we may want to skip setting the location entirely when we're
> using a shared global?
I'm going to make the location "shared JSM global" for the shared global, rather than just have it be blank.
Comment hidden (mozreview-request) |
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/717306162c3c
Call SetLocationForGlobal in a single location in the loader. r=kmag
Comment 6•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•