Web Inspector: ES6: Improved Console Format for Set and Map Objects (like Arrays)

[Joseph Pecoraro]

JSC supports ES6 Set objects. We should have an improved Console Formatting for Sets, like we do Arrays. Currently the console.log is cumbersome.

    var set = new Set(1,2,1,2,3);
    // Possible UI: { 1, 2, 3 }

[Radar WebKit Bug Importer]

<rdar://problem/15235745>

[Timothy Hatcher]

{ 1, 2, 3 } sounds good, what about Maps? { foo: 1, bar: 2, bas: 3 }? Too much like an object literal?

[Michał Gołębiowski-Owczarek]

(In reply to comment #2)
> { 1, 2, 3 } sounds good, what about Maps? { foo: 1, bar: 2, bas: 3 }? Too much like an object literal?

What about Maps with non-primitive keys? { foo: 1, { bar: 4 }: 2, bas: 3 }? Or will it just be omitted? Such a notation would create a weird case where map({ bar: 4 }) === undefined even though you see this object serialization in the console.

[Joseph Pecoraro]

I'll be doing this soon.

[Joseph Pecoraro]

Created attachment 245403 [details]
[PATCH] Proposed Fix

[WebKit Commit Bot]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[Joseph Pecoraro]

+ Some JavaScriptCore folks.

Can you review the JSC WeakMapData portions of the patch?

    runtime/WeakMapData.h
    inspector/JSInjectedScriptHostPrototype.cpp:
    inspector/JSInjectedScriptHost.h:
    inspector/JSInjectedScriptHost.cpp:

[Joseph Pecoraro]

Comment on attachment 245403 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=245403&action=review

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:793
> +        this.preview = this._generatePreview(object, objectGroupName, undefined, columnNames);

Oops, this objectGroupName addition is now no longer necessary. I will remove it here and in subsequent sections.

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:966
> +    _appendEntryPreviews: function(object, objectGroupName, preview)

Like here, where it is no longer used.

[Joseph Pecoraro]

Comment on attachment 245403 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=245403&action=review

> Source/WebInspectorUI/UserInterface/Views/ConsoleMessageImpl.js:282
> -            var lossless = this._appendObjectPreview(titleElement, obj);
> +            var lossless = this._appendPreview(titleElement, obj.preview);

Oops, I think this will break iOS 7/8. I will have to augment the type/subtype/description onto the the preview for those. Should be easy:

    if (!preview.type) {
        preview.type = obj.type;
        preview.subtype = obj.subtype;
    }

But maybe I should invest in creating a real WebInspector.ObjectPreview model object to abstract this away.

[Joseph Pecoraro]

Comment on attachment 245403 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=245403&action=review

> LayoutTests/TestExpectations:101
> +# Garbage Collection is missing an object.
> +webkit.org/b/140919 inspector/model/remote-object-weak-collection.html [ Skip ]

We got to the bottom of this and can change the test to get consistent expected results. Unfortunately, I think I'll still need to skip the test because the bots still timeout randomly on inspector tests but bug 140919 has been closed.

[Joseph Pecoraro]

Created attachment 245475 [details]
[PATCH] Proposed Fix

[Timothy Hatcher]

Comment on attachment 245475 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=245475&action=review

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:737
> +        }
> +        return entries;

Nit: Newline.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/179349