Closed
Bug 1408562
Opened 7 years ago
Closed 7 years ago
Update Debugger Frontend (10-13)
Categories
(DevTools :: Debugger, enhancement, P3)
Tracking
(firefox58 fixed)
RESOLVED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: jlast, Assigned: jlast)
References
Details
Attachments
(1 file, 11 obsolete files)
(deleted),
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•7 years ago
|
||
Attachment #8918465 -
Flags: review?(jdescottes)
Updated•7 years ago
|
Assignee: nobody → jlaster
Severity: normal → enhancement
Status: NEW → ASSIGNED
Priority: -- → P3
Comment 4•7 years ago
|
||
I think the try failures are linked to the fact that we used --artifact for the try run. We introduced new telemetry probes in https://bugzilla.mozilla.org/show_bug.cgi?id=1405584 and I guess the artifact build retrieved did not have the probes.
Here are 2 new runs:
- with artifact https://treeherder.mozilla.org/#/jobs?repo=try&revision=5390422469f144d21bce95cb21c6659b47abf5f4
- without artifact https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5036f2157b255e239e5facba762b251b2801f6d
Comment 5•7 years ago
|
||
Comment on attachment 8918465 [details] [diff] [review]
patch-10-13-1.patch
Review of attachment 8918465 [details] [diff] [review]:
-----------------------------------------------------------------
R+ if the try runs above are green.
Fixes the breapoint issue raised during previous bundle review, thanks!
Attachment #8918465 -
Flags: review?(jdescottes) → review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a52945a497c8
Update Debugger frontend (10-13). r=jdescottes
Keywords: checkin-needed
Comment 8•7 years ago
|
||
Backed out for leak in dt1 job on Linux x64 asan:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e862d9aa21e366753220bbcc1f9c9aaacace95d5
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=a52945a497c89cca6b8edb746837218dbd64e61d&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=137271899&repo=mozilla-inbound
[task 2017-10-16T20:22:03.338Z] 20:22:03 ERROR - GECKO(1095) | ==1232==ERROR: LeakSanitizer: detected memory leaks
[task 2017-10-16T20:22:03.342Z] 20:22:03 INFO - GECKO(1095) | Direct leak of 56 byte(s) in 1 object(s) allocated from:
[task 2017-10-16T20:22:03.345Z] 20:22:03 INFO - GECKO(1095) | #0 0x4bc44c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-10-16T20:22:03.382Z] 20:22:03 INFO - GECKO(1095) | #1 0x7f119be53c98 in js_malloc /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Utility.h:358:12
[task 2017-10-16T20:22:03.402Z] 20:22:03 INFO - GECKO(1095) | #2 0x7f119be53c98 in js_pod_malloc<unsigned char> /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Utility.h:549
[task 2017-10-16T20:22:03.410Z] 20:22:03 INFO - GECKO(1095) | #3 0x7f119be53c98 in maybe_pod_malloc<unsigned char> /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:54
[task 2017-10-16T20:22:03.414Z] 20:22:03 INFO - GECKO(1095) | #4 0x7f119be53c98 in unsigned char* js::MallocProvider<JS::Zone>::pod_malloc<unsigned char>(unsigned long) /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:87
[task 2017-10-16T20:22:03.421Z] 20:22:03 INFO - GECKO(1095) | #5 0x7f119d2db48c in new_<js::WasmBreakpointSite, js::wasm::DebugState *, unsigned int &> /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:187:5
[task 2017-10-16T20:22:03.442Z] 20:22:03 INFO - GECKO(1095) | #6 0x7f119d2db48c in js::wasm::DebugState::getOrCreateBreakpointSite(JSContext*, unsigned int) /builds/worker/workspace/build/src/js/src/wasm/WasmDebug.cpp:420
[task 2017-10-16T20:22:03.460Z] 20:22:03 INFO - GECKO(1095) | #7 0x7f119ccfd7e3 in js::Debugger::onTrap(JSContext*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Debugger.cpp:2049:53
[task 2017-10-16T20:22:03.464Z] 20:22:03 INFO - GECKO(1095) | #8 0x7f119d2bd5d5 in WasmHandleDebugTrap() /builds/worker/workspace/build/src/js/src/wasm/WasmBuiltins.cpp:147:31
[task 2017-10-16T20:22:03.474Z] 20:22:03 INFO - GECKO(1095) | -----------------------------------------------------
[task 2017-10-16T20:22:03.476Z] 20:22:03 INFO - GECKO(1095) | Suppressions used:
[task 2017-10-16T20:22:03.479Z] 20:22:03 INFO - GECKO(1095) | count bytes template
[task 2017-10-16T20:22:03.488Z] 20:22:03 INFO - GECKO(1095) | 8 256 libc.so
[task 2017-10-16T20:22:03.496Z] 20:22:03 INFO - GECKO(1095) | 789 24912 nsComponentManagerImpl
[task 2017-10-16T20:22:03.500Z] 20:22:03 INFO - GECKO(1095) | 6 1056 mozJSComponentLoader::LoadModule
[task 2017-10-16T20:22:03.510Z] 20:22:03 INFO - GECKO(1095) | 611 17713 libfontconfig.so
[task 2017-10-16T20:22:03.521Z] 20:22:03 INFO - GECKO(1095) | 12 416 style::gecko::global_style_data
[task 2017-10-16T20:22:03.524Z] 20:22:03 INFO - GECKO(1095) | 1 72 nss_ClearErrorStack
[task 2017-10-16T20:22:03.529Z] 20:22:03 INFO - GECKO(1095) | 16 2316 libglib-2.0.so
[task 2017-10-16T20:22:03.544Z] 20:22:03 INFO - GECKO(1095) | -----------------------------------------------------
[task 2017-10-16T20:22:03.548Z] 20:22:03 INFO - GECKO(1095) | SUMMARY: AddressSanitizer: 56 byte(s) leaked in 1 allocation(s).
[task 2017-10-16T20:22:07.218Z] 20:22:07 INFO - GECKO(1095) | -----------------------------------------------------
[task 2017-10-16T20:22:07.220Z] 20:22:07 INFO - GECKO(1095) | Suppressions used:
[task 2017-10-16T20:22:07.223Z] 20:22:07 INFO - GECKO(1095) | count bytes template
[task 2017-10-16T20:22:07.227Z] 20:22:07 INFO - GECKO(1095) | 710 22616 nsComponentManagerImpl
[task 2017-10-16T20:22:07.232Z] 20:22:07 INFO - GECKO(1095) | 50 8800 mozJSComponentLoader::LoadModule
[task 2017-10-16T20:22:07.234Z] 20:22:07 INFO - GECKO(1095) | 1 384 pixman_implementation_lookup_composite
[task 2017-10-16T20:22:07.236Z] 20:22:07 INFO - GECKO(1095) | 612 17514 libfontconfig.so
[task 2017-10-16T20:22:07.238Z] 20:22:07 INFO - GECKO(1095) | 2 64 libcairo.so
[task 2017-10-16T20:22:07.240Z] 20:22:07 INFO - GECKO(1095) | 1 32 libdl.so
[task 2017-10-16T20:22:07.242Z] 20:22:07 INFO - GECKO(1095) | 17 4348 libglib-2.0.so
[task 2017-10-16T20:22:07.244Z] 20:22:07 INFO - GECKO(1095) | 1 40 libpulsecommon-8.0.so
[task 2017-10-16T20:22:07.246Z] 20:22:07 INFO - GECKO(1095) | -----------------------------------------------------
[task 2017-10-16T20:22:07.624Z] 20:22:07 INFO - TEST-INFO | Main app process: exit 0
[task 2017-10-16T20:22:07.627Z] 20:22:07 INFO - TEST-INFO | LeakSanitizer | To show the addresses of leaked objects add report_objects=1 to LSAN_OPTIONS
[task 2017-10-16T20:22:07.631Z] 20:22:07 INFO - TEST-INFO | LeakSanitizer | This can be done in testing/mozbase/mozrunner/mozrunner/utils.py
[task 2017-10-16T20:22:07.635Z] 20:22:07 ERROR - TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at js_pod_malloc, maybe_pod_malloc, js::wasm::DebugState::getOrCreateBreakpointSite, js::Debugger::onTrap
Flags: needinfo?(jdescottes)
Comment 9•7 years ago
|
||
Sorry about that.
Looks like browser_dbg-wasm-sourcemaps.js (which was just unskipped in this Bug) surfaced a leak. Stack trace:
[task 2017-10-16T20:22:03.338Z] 20:22:03 ERROR - GECKO(1095) | ==1232==ERROR: LeakSanitizer: detected memory leaks
[task 2017-10-16T20:22:03.342Z] 20:22:03 INFO - GECKO(1095) | Direct leak of 56 byte(s) in 1 object(s) allocated from:
[task 2017-10-16T20:22:03.345Z] 20:22:03 INFO - GECKO(1095) | #0 0x4bc44c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
[task 2017-10-16T20:22:03.382Z] 20:22:03 INFO - GECKO(1095) | #1 0x7f119be53c98 in js_malloc /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Utility.h:358:12
[task 2017-10-16T20:22:03.402Z] 20:22:03 INFO - GECKO(1095) | #2 0x7f119be53c98 in js_pod_malloc<unsigned char> /builds/worker/workspace/build/src/obj-firefox/dist/include/js/Utility.h:549
[task 2017-10-16T20:22:03.410Z] 20:22:03 INFO - GECKO(1095) | #3 0x7f119be53c98 in maybe_pod_malloc<unsigned char> /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:54
[task 2017-10-16T20:22:03.414Z] 20:22:03 INFO - GECKO(1095) | #4 0x7f119be53c98 in unsigned char* js::MallocProvider<JS::Zone>::pod_malloc<unsigned char>(unsigned long) /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:87
[task 2017-10-16T20:22:03.421Z] 20:22:03 INFO - GECKO(1095) | #5 0x7f119d2db48c in new_<js::WasmBreakpointSite, js::wasm::DebugState *, unsigned int &> /builds/worker/workspace/build/src/js/src/vm/MallocProvider.h:187:5
[task 2017-10-16T20:22:03.442Z] 20:22:03 INFO - GECKO(1095) | #6 0x7f119d2db48c in js::wasm::DebugState::getOrCreateBreakpointSite(JSContext*, unsigned int) /builds/worker/workspace/build/src/js/src/wasm/WasmDebug.cpp:420
[task 2017-10-16T20:22:03.460Z] 20:22:03 INFO - GECKO(1095) | #7 0x7f119ccfd7e3 in js::Debugger::onTrap(JSContext*, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Debugger.cpp:2049:53
[task 2017-10-16T20:22:03.464Z] 20:22:03 INFO - GECKO(1095) | #8 0x7f119d2bd5d5 in WasmHandleDebugTrap() /builds/worker/workspace/build/src/js/src/wasm/WasmBuiltins.cpp:147:31
Flags: needinfo?(jdescottes)
Assignee | ||
Comment 10•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8918465 -
Attachment is obsolete: true
Assignee | ||
Comment 11•7 years ago
|
||
Attachment #8919085 -
Flags: review?(jlaster)
Assignee | ||
Comment 12•7 years ago
|
||
disregard this patch...
Assignee | ||
Comment 13•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919085 -
Attachment is obsolete: true
Attachment #8919085 -
Flags: review?(jlaster)
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8919098 -
Flags: review?(jlaster)
Assignee | ||
Comment 15•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919098 -
Attachment is obsolete: true
Attachment #8919098 -
Flags: review?(jlaster)
Assignee | ||
Comment 16•7 years ago
|
||
Attachment #8919359 -
Flags: review?(jlaster)
Assignee | ||
Comment 17•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919359 -
Attachment is obsolete: true
Attachment #8919359 -
Flags: review?(jlaster)
Assignee | ||
Comment 18•7 years ago
|
||
Attachment #8919361 -
Flags: review?(jlaster)
Assignee | ||
Comment 19•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919361 -
Attachment is obsolete: true
Attachment #8919361 -
Flags: review?(jlaster)
Assignee | ||
Comment 20•7 years ago
|
||
Attachment #8919371 -
Flags: review?(jlaster)
Comment 21•7 years ago
|
||
Comment on attachment 8919371 [details] [diff] [review]
patch-10-13-10.patch
Review of attachment 8919371 [details] [diff] [review]:
-----------------------------------------------------------------
Something is wrong, have you based your patch off Beta?
The diff is huge and contains a lot of unrelated changes.
Also the r? points to you.
Attachment #8919371 -
Flags: review?(jlaster) → review-
Assignee | ||
Comment 22•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919371 -
Attachment is obsolete: true
Assignee | ||
Comment 23•7 years ago
|
||
Attachment #8919376 -
Flags: review?(jlaster)
Assignee | ||
Comment 24•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919376 -
Attachment is obsolete: true
Attachment #8919376 -
Flags: review?(jlaster)
Assignee | ||
Comment 25•7 years ago
|
||
Attachment #8919741 -
Flags: review?(jlaster)
Comment 26•7 years ago
|
||
Comment on attachment 8919741 [details] [diff] [review]
patch-10-13-21.patch
Review of attachment 8919741 [details] [diff] [review]:
-----------------------------------------------------------------
As discussed, icons are missing in the status bar (blackboxing + prettify)
CSS points to:
- chrome://devtools/skin/images/debugger/prettyPrint.svg
- chrome://devtools/skin/images/debugger/blackBox.svg
Both missing from the patch
Other than this, didn't see anything broken here.
R+ with green try + fixed reviewer in commit message.
Side note: the last patch is almost 3MB big (the bundle seems to be shuffled around a lot). The first patch was less than 200kB.
Just mentioning it in case this was unexpected.
Attachment #8919741 -
Flags: review?(jlaster) → review+
Assignee | ||
Comment 27•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919741 -
Attachment is obsolete: true
Assignee | ||
Comment 28•7 years ago
|
||
Attachment #8919864 -
Flags: review?(jdescottes)
Assignee | ||
Comment 29•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919864 -
Attachment is obsolete: true
Attachment #8919864 -
Flags: review?(jdescottes)
Assignee | ||
Comment 30•7 years ago
|
||
Attachment #8919878 -
Flags: review?(jdescottes)
Comment 31•7 years ago
|
||
Comment on attachment 8919878 [details] [diff] [review]
patch-10-13-26.patch
Review of attachment 8919878 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM
Attachment #8919878 -
Flags: review?(jdescottes) → review+
Assignee | ||
Comment 32•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919878 -
Attachment is obsolete: true
Assignee | ||
Comment 33•7 years ago
|
||
Attachment #8919893 -
Flags: review?(jdescottes)
Assignee | ||
Comment 34•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8919893 -
Attachment is obsolete: true
Attachment #8919893 -
Flags: review?(jdescottes)
Assignee | ||
Comment 35•7 years ago
|
||
Attachment #8919975 -
Flags: review?(jdescottes)
Comment 36•7 years ago
|
||
Comment on attachment 8919975 [details] [diff] [review]
patch-10-13-31.patch
Review of attachment 8919975 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM
Attachment #8919975 -
Flags: review?(jdescottes) → review+
Comment 37•7 years ago
|
||
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47bb399c0621
Update Debugger frontend (10-13). r=jdescottes
Comment 38•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment 39•7 years ago
|
||
editor.addConditionalBreakpoint string content has been changed without id update, https://hg.mozilla.org/mozilla-central/diff/47bb399c0621/devtools/client/locales/en-US/debugger.properties
Flags: needinfo?(francesco.lodolo)
Comment 40•7 years ago
|
||
(In reply to Stefan Plewako [:stef] from comment #39)
> editor.addConditionalBreakpoint string content has been changed without id
> update,
> https://hg.mozilla.org/mozilla-central/diff/47bb399c0621/devtools/client/
> locales/en-US/debugger.properties
Please ignore it, it's already been reverted in GitHub and will return to the previous value in the next export.
https://github.com/devtools-html/debugger.html/commit/e5e246fa6cd7521b7a7773be4da71acff2bdfd47
Since we have been stuck for over a week in exposing new strings, I decided to not block further because of this change.
Flags: needinfo?(francesco.lodolo)
Comment 41•7 years ago
|
||
It's being reverted in Bug 1411727
Comment 42•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/diff/47bb399c0621/devtools/client/locales/en-US/debugger.properties
> # LOCALIZATION NOTE (expressions.placeholder): Placeholder text for expression
> # input element
> expressions.placeholder=Add watch expression
> +expressions.placeholder.accesskey=e
Is it possible now or sth is wrong here?
Comment 43•7 years ago
|
||
(In reply to Stefan Plewako [:stef] from comment #42)
> https://hg.mozilla.org/mozilla-central/diff/47bb399c0621/devtools/client/
> locales/en-US/debugger.properties
> > # LOCALIZATION NOTE (expressions.placeholder): Placeholder text for expression
> > # input element
> > expressions.placeholder=Add watch expression
> > +expressions.placeholder.accesskey=e
>
> Is it possible now or sth is wrong here?
Likely. Either the comment is completely off (no accesskey on a placeholder), or the string is used in multiple places.
Seems the latter
https://github.com/devtools-html/debugger.html/blob/6660e0ddd273fb09e8b8a725a299a6f601279482/src/components/SecondaryPanes/Expressions.js#L182
https://github.com/devtools-html/debugger.html/blob/4114a97f1584097c64d85b3d02e51946f4533caf/src/components/Editor/EditorMenu.js#L91
Comment 44•7 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #43)
> (In reply to Stefan Plewako [:stef] from comment #42)
> > https://hg.mozilla.org/mozilla-central/diff/47bb399c0621/devtools/client/
> > locales/en-US/debugger.properties
> > > # LOCALIZATION NOTE (expressions.placeholder): Placeholder text for expression
> > > # input element
> > > expressions.placeholder=Add watch expression
> > > +expressions.placeholder.accesskey=e
> >
> > Is it possible now or sth is wrong here?
>
> Likely. Either the comment is completely off (no accesskey on a
> placeholder), or the string is used in multiple places.
https://github.com/devtools-html/debugger.html/issues/4513
Thanks for catching it.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•