1399673 - Update the debugger frontend (9-13)

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Comment 1

[Jason Laster [:jlast]]

Attachment: 9-13.patch

Comment 2

[Jason Laster [:jlast]]

Attachment: 9-13-2.patch

Comment 3

[Jason Laster [:jlast]]

Attachment: 9-13-3.patch

Comment 4

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c67a5310140aaf2d2e57f3a69cd4988f79dcd0db

Comment 5

[Jason Laster [:jlast]]

Attachment: 9-13-4.patch

Comment 6

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=490d158d1703320a61937a667aa2da5010aff693

Comment 7

[Julian Descottes [:jdescottes]]

Failing mochitests: 

- devtools/client/framework/test/browser_source_map-01.js
- devtools/client/framework/test/browser_source_map-absolute.js

Stack trace:

at chrome://mochitests/content/browser/devtools/client/framework/test/browser_source_map-01.js:45 - TypeError: newLoc is null
Stack trace:
    checkLoc1@chrome://mochitests/content/browser/devtools/client/framework/test/browser_source_map-01.js:45:3
    @chrome://mochitests/content/browser/devtools/client/framework/test/browser_source_map-01.js:30:3
    Tester_execTest@chrome://mochikit/content/browser-test.js:794:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:694:7
    SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
    Tester_execTest@chrome://mochikit/content/browser-test.js:794:9
    Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:694:7
    SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59

Comment 8

[Julian Descottes [:jdescottes]]

I guess we are missing a sourcemap bundle update in m-c mentioned here:

https://github.com/devtools-html/devtools-core/pull/672

Comment 9

[Julian Descottes [:jdescottes]]

Comment on attachment 8907915 [details] [diff] [review]
9-13-4.patch

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

Looks good to me, didn't spot new regressions while testing. Quickly tested function copy, seems to work fine too.

As discussed we need to update some sourcemap tests in order to get this to land. 
I'll let you update the patch and flag Tom for a review of the sourcemap related changes.

Comment 10

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9874d4c4df213c2f5cd25b7858692c42aa6b135

Comment 11

[Jason Laster [:jlast]]

Attachment: 9-13-5.patch

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8908168 [details] [diff] [review]
9-13-5.patch

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

Two files left over, please remove them.
Still one test failure, I'm confident you can come up with a fix. Forwarding the r? to Tom.

Tom: I reviewed the debugger related change. Have a look at the sourcemap changes (mostly test fixes in browser_source_map-absolute.js & browser_source_map-01.js)

::: devtools/client/debugger/test/mochitest/browser_dbg_source-maps-01.js.orig
@@ +1,1 @@
> +/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */

Remove this file

::: devtools/client/shared/source-map/worker.js.orig
@@ +1,1 @@
> +(function webpackUniversalModuleDefinition(root, factory) {

Remove this file.

Comment 13

[Tom Tromey :tromey]

Comment on attachment 8908168 [details] [diff] [review]
9-13-5.patch

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

Thank you.  Splinter review, old school!

Julian, I only looked at the two files you asked me to.  Jason and I discussed these.  The lack of a column in these tests was incidental, and doesn't affect what the tests are intended to exercise.  So this change is ok.

Comment 14

[Jason Laster [:jlast]]

Attachment: 9-13-6.patch

Comment 15

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b7d9770b8ab6cccb8b73f50d03446ef324c1ee9

Comment 16

[Julian Descottes [:jdescottes]]

Comment on attachment 8908269 [details] [diff] [review]
9-13-6.patch

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

R+ with green try.

Could you update the commit message to mention that this also updates source-maps to v0.13.0 ?

Comment 17

[Jason Laster [:jlast]]

Attachment: 9-13-7.patch

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee8939119c17
Update Debugger Frontend and upgrade Source Map Worker to v0.13.0. r=jdescottes

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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