808960 - Can't fetch sources packed in jar bundles when debugging chrome

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Victor Porof [:vporof][:vp]]

STR:

1. Open browser debugger
2. Select l10n.js from the dropdown

Promise: Promise rejection ignored and silently dropped Error: Request failed 
Promise: Error originating at chrome://global/content/devtools/dbg-script-actors.js , line 1367 
Promise: Attempting to extract exception stack undefinedSA__loadSource@chrome://global/content/devtools/dbg-script-actors.js:1367
SA_onSource@chrome://global/content/devtools/dbg-script-actors.js:1280
DSC_onPacket@chrome://global/content/devtools/dbg-server.js:639
@chrome://global/content/devtools/dbg-transport.js:169

 
Promise: Attempting to extract current stack JS frame :: resource://gre/modules/devtools/_Promise.jsm :: <TOP_LEVEL> :: line 202
JS frame :: resource://gre/modules/devtools/_Promise.jsm :: <TOP_LEVEL> :: line 161
JS frame :: chrome://global/content/devtools/dbg-script-actors.js :: SA__loadSource :: line 1367
JS frame :: chrome://global/content/devtools/dbg-script-actors.js :: SA_onSource :: line 1280
JS frame :: chrome://global/content/devtools/dbg-server.js :: DSC_onPacket :: line 639
JS frame :: chrome://global/content/devtools/dbg-transport.js :: <TOP_LEVEL> :: line 169
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

Comment 1

[Victor Porof [:vporof][:vp]]

I mistakenly though l10n.js is a browser file. In fact, it's part of an addon by Erik Vold: https://addons.mozilla.org/en-us/firefox/addon/restartless-restart/

Afaict, all the files that exhibit this behaviour are in addon jars pointed at by XPIProvider (other examples: bootstrap.js, prefs.js).

Comment 2

[Rob Campbell [:rc] (:robcee)]

Mihai suggests that we might have to unpack the jar file before loading the scripts.

Filter on TAKETHATPAUL.

Comment 3

[Rob Campbell [:rc] (:robcee)]

15:12 < victorporof> i'm guessing this: { "url": "resource://gre/modules/XPIProvider.jsm -> 
jar:file://.../Profiles/l18yxlgz.development/extensions/[e-mail]!/bootstrap.js -> jar:file://.../Firefox/Profiles/l18yxlgz.development/extensions/[e-mail]!/includes/l10n.js" }
...
15:16 <@robcee> not sure why we're identifying it like that
15:16 <@robcee> is it because it has to be unique?
15:17 < Mossop> And in this case each url there is actually in its own sandbox too
15:17 <@robcee> and might refer to a specific compartment?
15:17 <@robcee> ah yeah
15:17 <@robcee> so it's more of a description of the owning sandbox / global / whatever where the 
                file's living
15:18 <@robcee> though the addon identifier should be enough to keep that unique, I think
15:18 < Mossop> It's for debugging purposes. If you load the same file from two different scripts one 
                might work and one might not. Without an actual debugger the only useful thing to do 
                is at least say what did the loading
15:18 <@robcee> unless an addon can load more than one instance of the same file
15:18 < Mossop> Sure it can
15:18 < victorporof> any smarter way of parsing that instead of .split(" -> ") ?

Comment 4

[Victor Porof [:vporof][:vp]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> < victorporof> any smarter way of parsing that instead of .split(" -> ") ?

Splitting is good enough.

Comment 5

[Girish Sharma [:Optimizer]]

Is it possible to add a unique identifier to a file, like with url a -> b -> c, instead of using just c as the identifier for that, use a part of b or a with that. This might help distinguish a -> b -> c with a -> d -> c.

If I am not making any sense, please ignore me :)

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v1

Thanks for the detective work guys. The fix is just 4 lines. Can you try it on all your use cases that weren't working before?

Jim, I can't think of a way to write a test for this, since AFAIK it relies on the presence of special add-ons. I think it's telling that the original bug (bug 418356) didn't have tests either. I'm really not that worried about such a simple change, to be honest.

Comment 7

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 716464 [details] [diff] [review]
Patch v1

it's so ugly!

Comment 8

[Rob Campbell [:rc] (:robcee)]

I tested 3 different addons with varying degrees of Jetpack in them. I was successful in all of my tests. This looks like it'll fix a good chunk of the problems debugging addons.

Highlight: debugging Firebug's console and inspector.

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Patch v2

Rebased.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 724664 [details] [diff] [review]
Patch v2

Eddy's patch from bug 820012 breaks this, so we will probably need to fix it in the debugger server code.

Comment 11

[Eddy Bruel [:ejpbruel]]

Attachment: Patch

As we discussed today, with my fix for bug 820012 in place, this fix won't work anymore. As a quick solution, I'd like to propose the following patch.

Note that the changes in debugger-panes.js are necessary. Without them, add-on scripts (and other scripts that use that weird URL form) won't appear in the list of sources 50% of the time.

Note that we cannot change the displayed URL in the debugger, because the same URL is passed to the debugger when setting things like breakpoints. If we want to improve on this, we should do something smarter like distinguishing between URL and displayURL. I leave it up to you if you want to file a follow-up bug for this.

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 725256 [details] [diff] [review]
Patch

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

Looks good to me.
I noticed that the Browser Debugger still displayed the weird URLs when debugging the Firefox OS Simulator, which is not a problem in and of itself, but nevertheless, I'd like Victor to take a look at this.

Comment 13

[Victor Porof [:vporof][:vp]]

Comment on attachment 725256 [details] [diff] [review]
Patch

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

(In reply to Panos Astithas [:past] from comment #12)
> 
> I noticed that the Browser Debugger still displayed the weird URLs when
> debugging the Firefox OS Simulator, which is not a problem in and of itself,
> but nevertheless, I'd like Victor to take a look at this.

Yeah, this is a broader and different problem. Short story: if you rebuild then things will work as expected. I suspect that recent cache changes in the profile folder may have made caching more aggressive than expected.

The patch looks good.

Comment 14

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/4b35a698dae2