Summary: | Web Inspector: ES6: Show Symbol properties on Objects | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bfulgham, burg, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer, ysuzuki | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | 141106, 143424 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Joseph Pecoraro
2015-02-04 19:15:20 PST
Waiting on: bug 141106 Created attachment 250168 [details]
[PATCH] Proposed Fix
First version.
- Symbol properties do not have custom Preview / ObjectTree look, just have stringified Symbol(foo) names.
- Worked around a JSC issue that appears to be JIT related, can't really reduce it with Symbols disabled by default though...
- Added context menu to Object Trees allowing developers to access the symbol itself
(In reply to comment #3) > Created attachment 250168 [details] > [PATCH] Proposed Fix > > First version. > > - Symbol properties do not have custom Preview / ObjectTree look, just > have stringified Symbol(foo) names. > - Worked around a JSC issue that appears to be JIT related, can't really > reduce it with Symbols disabled by default though... > - Added context menu to Object Trees allowing developers to access the > symbol itself Maybe it's my fault. When landing Symbol, I updated C++ StringConstructor to adapt symbols. However, seeing DFG code, there's special inlining path for String constructor and it's not changed to adapt symbols. (In reply to comment #4) > (In reply to comment #3) > > Created attachment 250168 [details] > > [PATCH] Proposed Fix > > > > First version. > > > > - Symbol properties do not have custom Preview / ObjectTree look, just > > have stringified Symbol(foo) names. > > - Worked around a JSC issue that appears to be JIT related, can't really > > reduce it with Symbols disabled by default though... > > - Added context menu to Object Trees allowing developers to access the > > symbol itself > > Maybe it's my fault. > When landing Symbol, I updated C++ StringConstructor to adapt symbols. > However, seeing DFG code, there's special inlining path for String > constructor and it's not changed to adapt symbols. Ooh, interesting. That sounds like exactly what I was seeing. function toString(o) { return String(o); } It seemed like this was occasionally throwing a TypeError when seeing a Symbol after seeing a lot of Strings. Yep, seems related to DFGByteCodeParser's ByteCodeParser::handleConstantInternalFunction path for StringConstructor. If I comment out that code I don't hit the error. Bug 143427 covers the issue Comment on attachment 250168 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=250168&action=review > Source/WebInspectorUI/UserInterface/Views/ObjectTreeBaseTreeElement.js:163 > + var text = WebInspector.UIString("Selected Symbol Property"); Selected Symbol Property Key? Selected Symbol Key? Property alone says "Value" to me. > Source/WebInspectorUI/UserInterface/Views/ObjectTreeBaseTreeElement.js:192 > + contextMenu.appendItem(WebInspector.UIString("Log Symbol Property"), this._logSymbolProperty.bind(this)); Log Symbol Key? Comment on attachment 250168 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=250168&action=review > Source/JavaScriptCore/inspector/InjectedScriptSource.js:673 > + var name = property.toString(); Landed! http://trac.webkit.org/changeset/182433 So we can use `toString(property)` :) (In reply to comment #8) > Comment on attachment 250168 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250168&action=review > > > Source/WebInspectorUI/UserInterface/Views/ObjectTreeBaseTreeElement.js:163 > > + var text = WebInspector.UIString("Selected Symbol Property"); > > Selected Symbol Property Key? Selected Symbol Key? > > Property alone says "Value" to me. > > > Source/WebInspectorUI/UserInterface/Views/ObjectTreeBaseTreeElement.js:192 > > + contextMenu.appendItem(WebInspector.UIString("Log Symbol Property"), this._logSymbolProperty.bind(this)); > > Log Symbol Key? Hmm, I think "Key" is misleading. Instead of a string property name it is a Symbol property name. "Key" sounds like something from a Map. Tell that to Object.keys(). ;) I hear ya though. Maybe just "Selected Symbol" and "Log Symbol". (In reply to comment #11) > Tell that to Object.keys(). ;) Hah, good point! > I hear ya though. > > Maybe just "Selected Symbol" and "Log Symbol". That works for me! These tests all fail on Windows. Can someone take a look? In general, it seems like the inspector protocol stuff on Windows is kind of unstable. It would be great if someone (Matt Baker!) could devote a little time to troubleshooting some of this. (In reply to comment #14) > These tests all fail on Windows. Can someone take a look? I thought these tests were skipped anyways. I run them manually. > In general, it seems like the inspector protocol stuff on Windows is kind of > unstable. It would be great if someone (Matt Baker!) could devote a little > time to troubleshooting some of this. I've been waiting until my next bot-watching week to investigate. |