Bug 166634 – Web Inspector: convert some DOMTreeManager methods to support promises and unify related test code

• Status: NEW
• Component: Web Inspector

[Blaze Burg]

Splitting from another bug, this got way out of hand.

[Blaze Burg]

Created attachment 297877 [details]
Proposed Fix (depends on 166523, 161358)

[Build Bot]

Comment on attachment 297877 [details]
Proposed Fix (depends on 166523, 161358)

Attachment 297877 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2817256

New failing tests:
inspector/dom/customElementState.html
inspector/dom/domutilities-path-dump.html
inspector/dom/highlight-shape-outside-margin.html
inspector/dom/pseudo-element-dynamic.html
inspector/dom/domutilities-csspath.html
inspector/dom/getOuterHTML.html
inspector/dom/pseudo-element-static.html
http/tests/inspector/console/cross-domain-inspected-node-access.html
inspector/dom/hideHighlight.html
inspector/dom/shadowRootType.html
inspector/console/addInspectedNode.html
inspector/css/shadow-scoped-style.html
inspector/console/command-line-api.html
inspector/css/stylesheet-with-mutations.html
inspector/unit-tests/dom-test-helpers.html
inspector/dom/domutilities-xpath.html
inspector/dom/setOuterHTML.html
inspector/dom/csp-hash.html
inspector/dom/csp-big5-hash.html
inspector/dom/highlightNode.html
inspector/css/generate-css-rule-string.html
inspector/dom/template-content.html

[Build Bot]

Created attachment 297878 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 297877 [details]
Proposed Fix (depends on 166523, 161358)

Attachment 297877 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2817267

New failing tests:
inspector/css/generate-css-rule-string.html
inspector/css/stylesheet-with-mutations.html
inspector/dom/domutilities-path-dump.html
inspector/dom/shadowRootType.html
http/tests/inspector/console/cross-domain-inspected-node-access.html
inspector/unit-tests/dom-test-helpers.html
inspector/dom/customElementState.html
inspector/dom/highlight-shape-outside-margin.html
inspector/dom/domutilities-xpath.html
inspector/dom/setOuterHTML.html
inspector/dom/domutilities-csspath.html
inspector/dom/template-content.html
inspector/dom/hideHighlight.html
inspector/dom/getOuterHTML.html
inspector/dom/csp-hash.html
inspector/dom/pseudo-element-dynamic.html
inspector/dom/csp-big5-hash.html
inspector/dom/highlightNode.html
inspector/css/shadow-scoped-style.html
inspector/dom/pseudo-element-static.html
inspector/console/command-line-api.html

[Build Bot]

Created attachment 297879 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 297877 [details]
Proposed Fix (depends on 166523, 161358)

Attachment 297877 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2817341

New failing tests:
inspector/dom/customElementState.html
inspector/dom/domutilities-path-dump.html
inspector/dom/highlight-shape-outside-margin.html
inspector/dom/pseudo-element-dynamic.html
inspector/dom/domutilities-csspath.html
inspector/dom/getOuterHTML.html
inspector/dom/pseudo-element-static.html
http/tests/inspector/console/cross-domain-inspected-node-access.html
inspector/dom/hideHighlight.html
inspector/dom/shadowRootType.html
inspector/console/addInspectedNode.html
inspector/css/shadow-scoped-style.html
inspector/console/command-line-api.html
inspector/css/stylesheet-with-mutations.html
inspector/unit-tests/dom-test-helpers.html
inspector/dom/domutilities-xpath.html
inspector/dom/setOuterHTML.html
inspector/dom/csp-hash.html
inspector/dom/csp-big5-hash.html
inspector/dom/highlightNode.html
inspector/css/generate-css-rule-string.html
inspector/dom/template-content.html

[Build Bot]

Created attachment 297881 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Yusuke Suzuki]

What do you think of using `async` and `await` for the DOM tests?

[Blaze Burg]

(In reply to comment #8)
> What do you think of using `async` and `await` for the DOM tests?

I am somewhat sheepish on using them everywhere because I don't like adding try/catch blocks. They might be appropriate in a few smaller cases, let me know if you have a few cases in mind!

[Blaze Burg]

Created attachment 298137 [details]
[PATCH] Rebased

[Blaze Burg]

Created attachment 298144 [details]
[PATCH] Proposed Fix v2

[Matt Baker]

Comment on attachment 298144 [details]
[PATCH] Proposed Fix v2

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

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:82
> +        // FIXME: convert clients to chain on this method instead of using a callback.

Please file a bug and add a FIXME.

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:118
> +    querySelector(nodeId, selectors, callback)

Are querySelector and querySelectorAll still in use? I didn't find any references.

> Source/WebInspectorUI/UserInterface/Controllers/DOMTreeManager.js:232
> +        if (payload && "nodeId" in payload) {

Braces aren't needed around one-line branches.

[Blaze Burg]

Comment on attachment 298144 [details]
[PATCH] Proposed Fix v2

Clearing r? as we aren't comfortable replacing more callbacks with promises until our promise debugging tools and test debugging are better.