Bug 166634

Summary:

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

Product:

WebKit

Reporter:

Blaze Burg <bburg>

Component:

Web Inspector

Assignee:

Blaze Burg <bburg>

Status:

NEW

Severity:

Normal

CC:

bburg, buildbot, commit-queue, dbates, inspector-bugzilla-changes, mkwst, rniwa, ysuzuki

Priority:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

161358, 166523

Bug Blocks:

Attachments:

Description	Flags
Proposed Fix (depends on 166523, 161358)	none
Archive of layout-test-results from ews102 for mac-elcapitan	none
Archive of layout-test-results from ews117 for mac-elcapitan	none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2	none
[PATCH] Rebased	none
[PATCH] Proposed Fix v2	none

Blaze Burg

Reported 2017-01-01 19:51:45 PST

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

Attachments
Proposed Fix (depends on 166523, 161358) (147.04 KB, patch) 2017-01-01 22:05 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-elcapitan (1.02 MB, application/zip) 2017-01-01 23:22 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews117 for mac-elcapitan (1.67 MB, application/zip) 2017-01-01 23:35 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (2.15 MB, application/zip) 2017-01-02 00:35 PST, Build Bot	no flags	Details
[PATCH] Rebased (148.39 KB, patch) 2017-01-05 14:39 PST, Blaze Burg	no flags	Details Formatted Diff Diff
[PATCH] Proposed Fix v2 (153.60 KB, patch) 2017-01-05 15:27 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Blaze Burg

Comment 1 2017-01-01 22:05:23 PST

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

Build Bot

Comment 2 2017-01-01 23:22:55 PST

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

Comment 3 2017-01-01 23:22:58 PST

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 4 2017-01-01 23:35:10 PST

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

Comment 5 2017-01-01 23:35:14 PST

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 6 2017-01-02 00:35:46 PST

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

Comment 7 2017-01-02 00:35:50 PST

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

Comment 8 2017-01-02 02:09:11 PST

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

Blaze Burg

Comment 9 2017-01-03 09:58:48 PST

(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

Comment 10 2017-01-05 14:39:19 PST

Created attachment 298137 [details] [PATCH] Rebased

Blaze Burg

Comment 11 2017-01-05 15:27:02 PST

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

Matt Baker

Comment 12 2017-01-05 15:56:34 PST

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 13 2017-02-02 20:40:16 PST

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.

Note You need to log in before you can comment on or make changes to this bug.