Closed
Bug 1399673
Opened 7 years ago
Closed 7 years ago
Update the debugger frontend (9-13)
Categories
(DevTools :: Debugger, enhancement)
DevTools
Debugger
Tracking
(firefox57 fixed)
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: jlast, Assigned: jlast)
References
Details
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8907868 -
Flags: review?(jdescottes)
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8907868 -
Attachment is obsolete: true
Attachment #8907868 -
Flags: review?(jdescottes)
Attachment #8907870 -
Flags: review?(jdescottes)
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8907870 -
Attachment is obsolete: true
Attachment #8907870 -
Flags: review?(jdescottes)
Attachment #8907881 -
Flags: review?(jdescottes)
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8907881 -
Attachment is obsolete: true
Attachment #8907881 -
Flags: review?(jdescottes)
Attachment #8907915 -
Flags: review?(jdescottes)
Assignee | ||
Comment 6•7 years ago
|
||
Comment 7•7 years ago
|
||
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•7 years ago
|
||
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•7 years ago
|
||
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.
Attachment #8907915 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 10•7 years ago
|
||
Assignee | ||
Comment 11•7 years ago
|
||
Attachment #8907915 -
Attachment is obsolete: true
Attachment #8908168 -
Flags: review?(jdescottes)
Comment 12•7 years ago
|
||
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.
Attachment #8908168 -
Flags: review?(jdescottes) → review?(ttromey)
Comment 13•7 years ago
|
||
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.
Attachment #8908168 -
Flags: review?(ttromey) → review+
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8908168 -
Attachment is obsolete: true
Attachment #8908269 -
Flags: review?(jdescottes)
Assignee | ||
Comment 15•7 years ago
|
||
Updated•7 years ago
|
Blocks: source-map-bundle-updates
Comment 16•7 years ago
|
||
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 ?
Attachment #8908269 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 17•7 years ago
|
||
Attachment #8908269 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8908298 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Whiteboard: checkin-needed
Comment 18•7 years ago
|
||
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee8939119c17
Update Debugger Frontend and upgrade Source Map Worker to v0.13.0. r=jdescottes
Assignee | ||
Updated•7 years ago
|
Whiteboard: checkin-needed
Comment 19•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•6 years ago
|
Assignee: nobody → jlaster
You need to log in
before you can comment on or make changes to this bug.
Description
•