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 InspectorAssignee: Blaze Burg <bburg>
Status: NEW    
Severity: Normal CC: bburg, buildbot, commit-queue, dbates, inspector-bugzilla-changes, mkwst, rniwa, ysuzuki
Priority: P2    
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
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
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
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
[PATCH] Rebased (148.39 KB, patch)
2017-01-05 14:39 PST, Blaze Burg
no flags
[PATCH] Proposed Fix v2 (153.60 KB, patch)
2017-01-05 15:27 PST, Blaze Burg
no flags
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.