Bug 177042 – Web Inspector: RemoteObject.resolveNode should return a Promise

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

[Matt Baker]

.

[Matt Baker]

Created attachment 321020 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 321020 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Protocol/RemoteObject.js:138
> -    static resolveNode(node, objectGroup, callback)
> +    static resolveNode(node, objectGroup)
>      {
> -        DOMAgent.resolveNode(node.id, objectGroup, function(error, object) {
> -            if (!callback)
> -                return;
> +        return new Promise((resolve, reject) => {
> +            DOMAgent.resolveNode(node.id, objectGroup, function(error, object) {
> +                if (error || !object) {
> +                    reject(error || "Could not resolve DOM node.");
> +                    return;
> +                }
>  
> -            if (error || !object)
> -                callback(null);
> -            else
> -                callback(WI.RemoteObject.fromPayload(object, WI.mainTarget));
> +                resolve(WI.RemoteObject.fromPayload(object, WI.mainTarget));
> +            });

I'm a little confused by this. DOMAgent.resolveNode can produce a promise, lets use that promise (which handles errors) instead of creating our own:

    return DOMAgent.resolveNode(node.id, objectGroup)
        .then(({object}) => WI.RemoteObject.fromPayload(object, WI.mainTarget));

> Source/WebInspectorUI/UserInterface/Views/BoxModelDetailsSectionRow.js:408
> +            function toggleInlineStyleProperty(property, value) {

Name: inspectedPage_node_toggleInlineStyleProperty

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:313
> +            function dimensions() {

This should be named:

    inspectedPage_node_dimensions

Since we are running this code on the `inspectedPage` and the this value is a `node`.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:508
> +        function injectStyleAndToggleClass() {

Same thing here:

    inspectedPage_node_injectStyleAndToggleClass

[Matt Baker]

Created attachment 321117 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 321117 [details]
Patch

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

rs=me

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.js:508
> +        function injectStyleAndToggleClass() {

Style: Name of this should have "inspectedPage_node_" prefix.

[Matt Baker]

Created attachment 321141 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 321141 [details]
Patch for landing

Clearing flags on attachment: 321141

Committed r222188: <http://trac.webkit.org/changeset/222188>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693236>