Bug 129204 – Web Inspector: Object Previews in the Console

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Timothy Hatcher]

We should show a preview of object data "{a: 1, b: 2, …}" for objects. I think support for this is supported in the backend, but we don't hook it up on the UI.

[Radar WebKit Bug Importer]

<rdar://problem/16142033>

[Joseph Pecoraro]

Refocusing this on the Console. IndexedDB covered by bug 140813.

[Joseph Pecoraro]

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.

[Build Bot]

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

[Build Bot]

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

[Timothy Hatcher]

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.

[Joseph Pecoraro]

(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.

[Joseph Pecoraro]

(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.

[Joseph Pecoraro]

This landed a while ago:
https://trac.webkit.org/changeset/179019