986757 - JS modules loaded while the add-on debugger is open won't show up

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Dave Townsend [:mossop]]

Because we don't set the __URI__ object before we fire the onNewGlobal event.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

In bug 986639 we want to be able to detect JSM globals created by add-ons. The easiest way to do that for now is to check the __URI__ property that we set on all such globals and tie that back to the add-on. We know it isn't perfect but it's simple and works for 99% of the cases for now.

The problem is that since we call JS_FireOnNewGlobal before setting the __URI__ property we'll miss new globals created after the debugger is opened. This patch defers calling that until after the property is set. Bug 986639 will have a patch verifying that that works. We only do this for the case where we aren't reusing globals for modules since in that case the global doesn't have the __URI__ property itself and at least right now we don't care about b2g for extension debugging.

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8395834 [details] [diff] [review]
patch rev 1

Review of attachment 8395834 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comments addressed.

(In reply to Dave Townsend (:Mossop) from comment #1)
> We only do this for the
> case where we aren't reusing globals for modules since in that case the
> global doesn't have the __URI__ property itself and at least right now we
> don't care about b2g for extension debugging.

Is there any reason not to do it the same way in both places though? I'd prefer consistent behavior here, even if it means having bool createdNewGlobal = false (and setting it to true in the |if (!mLoaderGlobal)| case). We'll end up with the same number of locals anyway, because we can remove |flags| and just inline the bit.

::: js/xpconnect/loader/mozJSComponentLoader.cpp
@@ +659,4 @@
>          rv = xpc->InitClassesWithNewWrappedGlobal(aCx,
>                                                    static_cast<nsIGlobalObject *>(backstagePass),
>                                                    mSystemPrincipal,
> +                                                  flags,

Please include the useful parts of comment 1 as a comment here, explaining why we defer OnNewGlobalObject.

Comment 3

[Dave Townsend [:mossop]]

https://hg.mozilla.org/integration/fx-team/rev/e94649af80c8

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e94649af80c8