Closed Bug 1372295 Opened 7 years ago Closed 7 years ago

Call SetLocationForGlobal in a single location in the loader

Categories

(Core :: XPConnect, enhancement)

enhancement
Not set
normal

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 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+
(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.
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/717306162c3c Call SetLocationForGlobal in a single location in the loader. r=kmag
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: