Source map of complex JS file requested 200 times

RESOLVED FIXED in Firefox 36

Status

()

Firefox
Developer Tools: Debugger
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Scott, Assigned: fitzgen)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
Firefox 36
x86
Mac OS X
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8509103 [details]
sourcemap-test.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.104 Safari/537.36

Steps to reproduce:

Load the webpage included in the attached zip
Open Developer Tools
Go to the Debugger tab
Reload the page



Actual results:

The sourcemap (batch.js.map) was request 196 times.


Expected results:

The sourcemap should have been requested once.

I have noticed that this seems to be related to the complexity of the JS file being mapped. If you delete half of batch.js the number of requests that are made drops significantly.

Comment 1

3 years ago
There is this recent bug about sourceMap too: bug 1084534.

Comment 2

3 years ago
Thanks for the testcase!

I can reproduce using current Nightly on OS X and couldn't find existing bugs about this.
Status: UNCONFIRMED → NEW
Component: Untriaged → Developer Tools: Debugger
Ever confirmed: true
Keywords: testcase
Version: 36 Branch → Trunk

Comment 3

3 years ago
And in FF33 or older?

Comment 4

3 years ago
Same in Firefox 33. Unless there's any indication it's a regression, I wouldn't bother testing older versions.
Blocks: 771597
Summary: Sourcemap of complex JS file requested 200 times → Source map of complex JS file requested 200 times
Seems like we are _always_ bypassing our cached source map requests inside `ThreadSources.prototype.sourceMap` (or, never checking the cache, only inserting into it)

>.<
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Comment on attachment 8512222 [details] [diff] [review]
source-map-requested-200x.patch

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

::: toolkit/devtools/server/actors/script.js
@@ +5107,4 @@
>  
>    /**
>     * Return a promise of a SourceMapConsumer for the source map for
>     * |aScript|; if we already have such a promise extant, return that.

Yeah, we definitely weren't doing this: "if we already have such a promise extant, return that."

The rest of the patch is just making a crappy test better.
Comment on attachment 8512222 [details] [diff] [review]
source-map-requested-200x.patch

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

::: toolkit/devtools/server/tests/unit/head_dbg.js
@@ +606,5 @@
> + * @returns Promise<response>
> + */
> +function reload(tabClient) {
> +  let deferred = promise.defer();
> +  tabClient._reload({}, deferred.resolve);

How come you are not using tabClient.reload() instead?
Attachment #8512222 - Flags: review?(past) → review+
(In reply to Panos Astithas [:past] (overloaded, please needinfo) from comment #8)
> Comment on attachment 8512222 [details] [diff] [review]
> source-map-requested-200x.patch
> 
> Review of attachment 8512222 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/devtools/server/tests/unit/head_dbg.js
> @@ +606,5 @@
> > + * @returns Promise<response>
> > + */
> > +function reload(tabClient) {
> > +  let deferred = promise.defer();
> > +  tabClient._reload({}, deferred.resolve);
> 
> How come you are not using tabClient.reload() instead?

Because reload doesn't take a callback :/
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/0bd41c3ea259
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/0bd41c3ea259
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
You need to log in before you can comment on or make changes to this bug.