Bug 141932 – Web Inspector: New Object Tree View UI

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
Revamp the Object Tree View UI.

[Radar WebKit Bug Importer]

<rdar://problem/19929599>

[Joseph Pecoraro]

Created attachment 247157 [details]
[PATCH] Part 1 - Majority of the UI

[Joseph Pecoraro]

Created attachment 247158 [details]
[PATCH] Part 2 - Provide better parameter lists for native functions

[Joseph Pecoraro]

Created attachment 247159 [details]
[PATCH] Part 3 - Better handle errors when invoking getters

[Joseph Pecoraro]

Created attachment 247160 [details]
[IMAGE] Error cases (expanding objects after navigation or no properties)

[Joseph Pecoraro]

Created attachment 247161 [details]
[IMAGE] Object with All Types

[Joseph Pecoraro]

Created attachment 247162 [details]
[IMAGE] Realistic Object - With Getters Invoked

[Joseph Pecoraro]

Created attachment 247163 [details]
[IMAGE] Realistic Native Object - window.navigator

[Joseph Pecoraro]

So, in order for this to work better with Native objects, I have a patch for bug 141587 that goes on top of these patches. The screenshots are with that patch applied.

[Joseph Pecoraro]

Comment on attachment 247157 [details]
[PATCH] Part 1 - Majority of the UI

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

> Source/WebInspectorUI/UserInterface/Images/TypeNull.svg:2
> +<!-- Copyright © 2013 Apple Inc. All rights reserved. -->

I never actually looked at these. But I should probably update the 2013 to 2015.

[Timothy Hatcher]

Comment on attachment 247157 [details]
[PATCH] Part 1 - Majority of the UI

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

>> Source/WebInspectorUI/UserInterface/Images/TypeNull.svg:2
>> +<!-- Copyright © 2013 Apple Inc. All rights reserved. -->
> 
> I never actually looked at these. But I should probably update the 2013 to 2015.

Opps, sorry. I just copied this file from Native.svg when I sent them to you. (Maybe we should consolidate dupes for these images some day.)

> Source/WebInspectorUI/UserInterface/Images/TypeSymbol.svg:2
> +<!-- Copyright © 2013 Apple Inc. All rights reserved. -->

Ditto.

> Source/WebInspectorUI/UserInterface/Images/TypeUndefined.svg:2
> +<!-- Copyright © 2013 Apple Inc. All rights reserved. -->

Ditto.

> Source/WebInspectorUI/UserInterface/Models/PropertyPath.js:26
> +WebInspector.PropertyPath = function(object, pathComponent, parent, isPrototype)

Very cool!

> Source/WebInspectorUI/UserInterface/Models/PropertyPath.js:50
> +    CollectionIndex: "@collection[?]",
> +    InternalPropertyName: "@internal",
> +    SymbolPropertyName: "@symbol",
> +    GetterPropertyName: "@getter",

What do these look like in practice? Got an example?

> Source/WebInspectorUI/UserInterface/Models/PropertyPath.js:117
> +        return this._pathComponent && this._pathComponent[0] === "@";

.startsWith? Though this might be faster. Maybe startsWith should have a single char optimization.

> Source/WebInspectorUI/UserInterface/Protocol/RemoteObject.js:422
> +        function backendInvokeGetter(getter) {

Nit: Newline

> Source/WebInspectorUI/UserInterface/Views/ObjectTreePropertyTreeElement.js:219
> +            // FIXME: What if just a setter?

Hmm, I could have sworn that wasn't allowed but I guess it is. Maybe we need a eye icon that is crossed out?

[Timothy Hatcher]

Comment on attachment 247158 [details]
[PATCH] Part 2 - Provide better parameter lists for native functions

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

> Source/WebInspectorUI/UserInterface/Models/NativeFunctionParameters.js:44
> +        __proto__: null,

Why is this needed?

> Source/WebInspectorUI/UserInterface/Models/NativeFunctionParameters.js:204
> +        setHours: "hours, [minutes=getMinutes()], [seconds=getSeconds()], [ms=getMilliseconds]",

Missing ().

> Source/WebInspectorUI/UserInterface/Models/NativeFunctionParameters.js:212
> +        setUTCHours: "hours, [minutes=getUTCMinutes()], [seconds=getUTCSeconds()], [ms=getUTCMilliseconds]",

Ditto.

[Joseph Pecoraro]

> > Source/WebInspectorUI/UserInterface/Models/PropertyPath.js:50
> > +    CollectionIndex: "@collection[?]",
> > +    InternalPropertyName: "@internal",
> > +    SymbolPropertyName: "@symbol",
> > +    GetterPropertyName: "@getter",
> 
> What do these look like in practice? Got an example?

Right now they are hidden, because the '@' character symbolizes this is an "impossible path component", so we fall back to a UIString saying unable to compute path. I haven't actually seen what it would look like.

But as an example:

  js> dir([{promise: Promise.resolve({get value() { return 10; }})}])
  <-  ▼ 0: Object
        ▼ promise: Promise
          ▼ result: Object
            value: (...)
      ...

In this case, that inner "value" would have a property path:

  obj[0].promise@internal[result].value

Which should probably be a getter lookup since, unresolved, this is a getter func:

  obj[0].promise@internal[result].__lookupGetter__("value")

After resolving the getter, the path starts new:

  @getter

But that should probably just be:

  obj[0].promise@internal[result].value

Depending on type / state of the property, the path can change. For instance if you want the path to a function on a prototype you'll need __proto__ but if want the resolved value of a getter you should not use .__proto__, and worst case if you want the resolved value of an overridden getter you'd need .__proto__.call(someRootObject) which I think gets too complex, (but hilariously we can do that with PropertyPaths, since "someRootObject" is just an earlier parent PropertyPath's fullPath).

I want to smooth out these edge cases in a follow-up. I have noticed this looks poor for getters.


> > Source/WebInspectorUI/UserInterface/Models/PropertyPath.js:117
> > +        return this._pathComponent && this._pathComponent[0] === "@";
> 
> .startsWith? Though this might be faster. Maybe startsWith should have a
> single char optimization.

I can look into that.

[Joseph Pecoraro]

(In reply to comment #12)
> Comment on attachment 247158 [details]
> [PATCH] Part 2 - Provide better parameter lists for native functions
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247158&action=review
> 
> > Source/WebInspectorUI/UserInterface/Models/NativeFunctionParameters.js:44
> > +        __proto__: null,
> 
> Why is this needed?

This eliminates the Object.prototype functions on these objects. So properties like "toString" won't show up on it.

Because of the way I did lookups and output:

>    var params = WebInspector.NativePrototypeFunctionParameters[this._prototypeName][this._property.name];
>    return params ? "(" + params + ")" : "()";

If the property name was "toString", then without __proto__:null "params" would be Object.prototype.toString, and string coercion would make the output:

    "(function toString() {\n    [native code]\n})"

Alternative solutions to __proto__:null would have been using hasOwnProperty() here, or Map instead of {} dictionaries.



> > Source/WebInspectorUI/UserInterface/Models/NativeFunctionParameters.js:204
> > +        setHours: "hours, [minutes=getMinutes()], [seconds=getSeconds()], [ms=getMilliseconds]",
> 
> Missing ().
> 
> > Source/WebInspectorUI/UserInterface/Models/NativeFunctionParameters.js:212
> > +        setUTCHours: "hours, [minutes=getUTCMinutes()], [seconds=getUTCSeconds()], [ms=getUTCMilliseconds]",
> 
> Ditto.

Nice catches!

[Joseph Pecoraro]

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