Summary: | Web Inspector: Object Previews in the Console | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | beidson, buildbot, graouts, joepeck, rniwa, timothy, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Timothy Hatcher
2014-02-22 08:16:29 PST
Refocusing this on the Console. IndexedDB covered by bug 140813. Created attachment 245205 [details]
[PATCH] Proposed Fix
First patch. Here are some things to look into after this patch.
1. Better array handling.
- we currently ignore array previews. We should just not send it.
- or better, send a complete list of properties instead of previews.
2. Unify Getter / Setter / Accessor handling
- in previews, accessors are sent without values and ignored
should we just drop accessors from previews? Or should we
evaluate getters that may have side-effects. We should unify
this paradigm with RemoteObject.getProperties, which currently
evaluates the getter regardless of possible side-effects.
3. Support more types
- Map/Set and Promises come to mind
4. Better iOS 8 support
- iOS 7 works well.
- unfortunately iOS 8 has backend issues with native bindings so previews
of native objects (window.screen, window.navigator, event) are poor.
Should we re-implement previews by proactively requesting properties
like we do with arrays? Should we turn it off?
Note, I plan on addressing issues (1), (2), and (3) as follow-ups as we redesign the console.
As for (4), I'm not sure yet what is best without experimentation. Requesting a complete list of properties is likely too heavy to always do, and there is no support in the backend for requesting a partial list of properties.
Comment on attachment 245205 [details] [PATCH] Proposed Fix Attachment 245205 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5795527627112448 New failing tests: inspector/model/remote-object.html Created attachment 245210 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 245205 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=245205&action=review > Source/JavaScriptCore/inspector/InjectedScriptSource.js:39 > + // We don't use String(obj) because inspectedGlobalObject.String is undefined if owning frame navigated to another page. True still for JSC/WebKit too? > Source/JavaScriptCore/inspector/InjectedScriptSource.js:779 > + // Show array items inherited from prototype. > + if (!descriptor.enumerable && !descriptor.isOwn && !(this.subtype === "array" && isUInt32(name))) > + continue; Shouldn't the comment say: "Don't show…"? > Source/JavaScriptCore/inspector/InjectedScriptSource.js:802 > + // Fix type of document.all. > + if (type === "undefined" && injectedScript._isHTMLAllCollection(value)) > + type = "object"; Seems like this should happen at another level. > Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js:69 > + DebuggerAgent.evaluateOnCallFrame.invoke({callFrameId: WebInspector.debuggerManager.activeCallFrame.id, expression: expression, objectGroup: objectGroup, includeCommandLineAPI: includeCommandLineAPI, doNotPauseOnExceptionsAndMuteConsole: doNotPauseOnExceptionsAndMuteConsole, returnByValue: returnByValue, generatePreview: generatePreview}, evalCallback.bind(this)); Can't we do this yet? {callFrameId: WebInspector.debuggerManager.activeCallFrame.id, expression, objectGroup, includeCommandLineAPI, doNotPauseOnExceptionsAndMuteConsole, returnByValue, generatePreview} > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js:384 > - // Make black quotes. > elem.classList.remove("console-formatted-string"); > elem.appendChild(document.createTextNode("\"")); > elem.appendChild(span); Quotes should be the red string color for us to match our CodeMirror styles and Xcode. (In reply to comment #6) > Comment on attachment 245205 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245205&action=review > > > Source/JavaScriptCore/inspector/InjectedScriptSource.js:39 > > + // We don't use String(obj) because inspectedGlobalObject.String is undefined if owning frame navigated to another page. > > True still for JSC/WebKit too? I think we can remove these comments (there are a few). I don't know how this can come about anyways. > > Source/JavaScriptCore/inspector/InjectedScriptSource.js:779 > > + // Show array items inherited from prototype. > > + if (!descriptor.enumerable && !descriptor.isOwn && !(this.subtype === "array" && isUInt32(name))) > > + continue; > > Shouldn't the comment say: "Don't show…"? Came from: https://chromium.googlesource.com/chromium/blink/+/9006001086bee4a96a02412ea6d28ad636d1f280%5E%21/ Not important right now since we skip array previews, but I'll clarify. > > Source/JavaScriptCore/inspector/InjectedScriptSource.js:802 > > + // Fix type of document.all. > > + if (type === "undefined" && injectedScript._isHTMLAllCollection(value)) > > + type = "object"; > > Seems like this should happen at another level. Immediately after we typeof, we need to check if "undefined" is HTMLAllCollection and instead make it "object". I think here, we wait a few statements for some possible quick bails. > > Source/WebInspectorUI/UserInterface/Controllers/RuntimeManager.js:69 > > + DebuggerAgent.evaluateOnCallFrame.invoke({callFrameId: WebInspector.debuggerManager.activeCallFrame.id, expression: expression, objectGroup: objectGroup, includeCommandLineAPI: includeCommandLineAPI, doNotPauseOnExceptionsAndMuteConsole: doNotPauseOnExceptionsAndMuteConsole, returnByValue: returnByValue, generatePreview: generatePreview}, evalCallback.bind(this)); > > Can't we do this yet? Seem that is now in the ES6 Spec drafts: https://people.mozilla.org/~jorendorff/es6-draft.html#sec-object-initializer-runtime-semantics-propertydefinitionevaluation But JSC doesn't yet support it. JSC supports destructuring (extracting values) and computed properties (when building), but not yet these "identifier properties" (or whatever they should be called). > {callFrameId: WebInspector.debuggerManager.activeCallFrame.id, expression, > objectGroup, includeCommandLineAPI, doNotPauseOnExceptionsAndMuteConsole, > returnByValue, generatePreview} > > > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js:384 > > - // Make black quotes. > > elem.classList.remove("console-formatted-string"); > > elem.appendChild(document.createTextNode("\"")); > > elem.appendChild(span); > > Quotes should be the red string color for us to match our CodeMirror styles > and Xcode. I'll fix. (In reply to comment #4) > Comment on attachment 245205 [details] > [PATCH] Proposed Fix > > Attachment 245205 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/5795527627112448 > > New failing tests: > inspector/model/remote-object.html Ugh, it is maddening that the bots occasionally timeout with no useful data on inspector tests. I'll skip the test. We have to address this issue sometime though. This landed a while ago: https://trac.webkit.org/changeset/179019 |