Closed Bug 1303177 Opened 5 years ago Closed 5 years ago

Turn off sourcemaps if using the new debugger frontend

Categories

(DevTools :: Debugger, defect, P1)

defect

Tracking

(firefox51 fixed)

RESOLVED FIXED
Firefox 51
Tracking Status
firefox51 --- fixed

People

(Reporter: jlong, Assigned: jlong)

References

Details

Attachments

(1 file)

Previously the new debugger turned off sourcemaps by reconfiguring the thread: https://github.com/devtools-html/debugger.html/blob/master/public/js/clients/firefox.js#L103

The problem is that when the thread is reconfigured, the TabSources on the backend clears out all the current sources. This happens because previously we discard all of them and create new (sourcemapped or non-sourcemapped) ones.

That means we can't reconfigure the thread when the debugger is initializing. If we do that, we break the workflow where the thread pauses on an eval script before the debugger is initialized. This happens if you open the console, and type "debugger" or run a function in an eval script that hits a `debugger` statement. Right now  if this happens it selects the debugger but does not go to the source (it works in the old debugger because it never calls `reconfigure` on startup).

It works fine if it breaks in a non-eval source, are any source with a URL. This is because the source actor is being recreated, but it gets the same ID because the debugger server always assigns the same IDs to URLs.
Attached patch 1303177.patchSplinter Review
Not sure who the best reviewer is, if you have too much on your plate bgrins maybe jryans can. 

Hopefully first comment makes it clear why we need to do this.
Assignee: nobody → jlong
Attachment #8791790 - Flags: review?(bgrinstead)
Blocks: 1294139
Priority: -- → P1
Comment on attachment 8791790 [details] [diff] [review]
1303177.patch

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

OK, as long as this works properly when closing and re-opening the toolbox (esp if flipping the new frontend pref in between) this seems fine

::: devtools/client/framework/attach-thread.js
@@ +42,5 @@
>    let { form: { chromeDebugger, actor } } = target;
> +
> +  // Sourcemaps are always turned off when using the new debugger
> +  // frontend. This is because it does sourcemapping on the
> +  // client-side, so the server should do it. It also does not support

should 'not' do it
Attachment #8791790 - Flags: review?(bgrinstead) → review+
This looks good. Thanks James.
`attachThread` is called here: https://github.com/mozilla/gecko-dev/blob/master/devtools/client/framework/toolbox.js#L387 which is in the `open` function (https://github.com/mozilla/gecko-dev/blob/master/devtools/client/framework/toolbox.js#L353), so the thread always detaches on close and re-attaches on open. It should affect re-opening the tools and should be fine if switching between debuggers. I'll test to make sure though.
Pushed by jlong@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/1e2642879de8
disable sourcemaps when using the new debugger r=bgrins
https://hg.mozilla.org/mozilla-central/rev/1e2642879de8
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.