200109 – Web Inspector: Page: don't allow the domain to be disabled

RESOLVED FIXED 200109

Web Inspector: Page: don't allow the domain to be disabled

https://bugs.webkit.org/show_bug.cgi?id=200109

Summary Web Inspector: Page: don't allow the domain to be disabled

Devin Rousso

Reported 2019-07-24 20:42:34 PDT

Attachments
Patch (28.14 KB, patch) 2019-07-24 20:59 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (28.23 KB, patch) 2019-07-24 21:15 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (28.55 KB, patch) 2019-07-24 21:21 PDT, Devin Rousso	ews-watchlist: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (367.75 KB, application/zip) 2019-07-24 22:05 PDT, EWS Watchlist	no flags	Details
Patch (19.13 KB, patch) 2019-07-24 22:12 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews104 for mac-highsierra-wk2 (2.27 MB, application/zip) 2019-07-24 23:16 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews116 for mac-highsierra (1.43 MB, application/zip) 2019-07-24 23:31 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews102 for mac-highsierra (2.20 MB, application/zip) 2019-07-24 23:33 PDT, EWS Watchlist	no flags	Details
Patch (31.69 KB, patch) 2019-08-01 16:42 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews114 for mac-highsierra (3.29 MB, application/zip) 2019-08-01 19:42 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews102 for mac-highsierra (3.62 MB, application/zip) 2019-08-01 20:39 PDT, EWS Watchlist	no flags	Details
Patch (31.35 KB, patch) 2019-08-02 01:15 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Patch (31.18 KB, patch) 2019-08-08 16:18 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Show Obsolete (12) View All Add attachment proposed patch, testcase, etc.

Devin Rousso

Comment 1 2019-07-24 20:59:53 PDT

Created attachment 374861 [details] Patch

EWS Watchlist

Comment 2 2019-07-24 21:02:14 PDT Comment hidden (obsolete)

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

Devin Rousso

Comment 3 2019-07-24 21:15:51 PDT

Created attachment 374865 [details] Patch

Devin Rousso

Comment 4 2019-07-24 21:21:55 PDT

Created attachment 374867 [details] Patch

EWS Watchlist

Comment 5 2019-07-24 22:05:13 PDT Comment hidden (obsolete)

Comment on attachment 374867 [details] Patch Attachment 374867 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12807157 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 6 2019-07-24 22:05:15 PDT Comment hidden (obsolete)

Created attachment 374871 [details] Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6

Devin Rousso

Comment 7 2019-07-24 22:12:53 PDT

Created attachment 374873 [details] Patch

EWS Watchlist

Comment 8 2019-07-24 23:16:35 PDT Comment hidden (obsolete)

Comment on attachment 374873 [details] Patch Attachment 374873 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12807476 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 9 2019-07-24 23:16:36 PDT Comment hidden (obsolete)

Created attachment 374878 [details] Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6

EWS Watchlist

Comment 10 2019-07-24 23:31:06 PDT Comment hidden (obsolete)

Comment on attachment 374873 [details] Patch Attachment 374873 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12807465 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 11 2019-07-24 23:31:08 PDT Comment hidden (obsolete)

Created attachment 374879 [details] Archive of layout-test-results from ews116 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-highsierra Platform: Mac OS X 10.13.6

EWS Watchlist

Comment 12 2019-07-24 23:33:09 PDT Comment hidden (obsolete)

Comment on attachment 374873 [details] Patch Attachment 374873 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12807549 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 13 2019-07-24 23:33:11 PDT Comment hidden (obsolete)

Created attachment 374881 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6

Joseph Pecoraro

Comment 14 2019-07-29 12:04:21 PDT

Comment on attachment 374873 [details] Patch This potentially changes the order in which the frontend receives events. Previously they would come as part of `Page.enable` and now they are just sent immediately when a frontend connects. I don't think this should cause problems, the frontend should have been able to setup domain observers before it lets backend messages in: // Signal that the frontend is now ready to receive messages. WI.whenTargetsAvailable().then(() => { InspectorFrontendAPI.loadCompleted(); }); But I can't explain why so many tests are failing with lines like: CONSOLE MESSAGE: line 1: ReferenceError: Can't find variable: InspectorFrontendAPI ... There are also many cases of tests directly performing this that would need to be updated: page/media-query-list-listener-exception.html 13: InspectorProtocol.sendCommand("Page.enable", {}); page/frameScheduledNavigation.html 16: InspectorProtocol.sendCommand("Page.enable", {}); page/frameScheduledNavigation-async-delegates.html 19: InspectorProtocol.sendCommand("Page.enable", {}); page/archive.html 7: InspectorProtocol.sendCommand("Page.enable", {}); page/frameStartedLoading.html 17: InspectorProtocol.sendCommand("Page.enable", {}); timeline/line-column.html 39: InspectorProtocol.sendCommand("Page.enable"); A smaller step forward might be just removing `disable`. Almost this entire patch could be reused.

Joseph Pecoraro

Comment 15 2019-07-29 12:45:40 PDT

Also note that ITMLKit would need to rework their code now that `Page.enable` no longer exists.

Devin Rousso

Comment 16 2019-08-01 16:42:50 PDT

Created attachment 375365 [details] Patch

EWS Watchlist

Comment 17 2019-08-01 19:42:20 PDT Comment hidden (obsolete)

Comment on attachment 375365 [details] Patch Attachment 375365 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12849554 New failing tests: inspector/debugger/breakpoint-eval-with-exception.html inspector/debugger/setBreakpointByUrl-sourceURL.html inspector/debugger/evaluateOnCallFrame-errors.html inspector/debugger/pause-on-assert.html inspector/debugger/setBreakpoint-condition.html inspector/debugger/regress-133182.html inspector/debugger/setBreakpoint-column.html

EWS Watchlist

Comment 18 2019-08-01 19:42:22 PDT Comment hidden (obsolete)

Created attachment 375383 [details] Archive of layout-test-results from ews114 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6

EWS Watchlist

Comment 19 2019-08-01 20:39:50 PDT Comment hidden (obsolete)

Comment on attachment 375365 [details] Patch Attachment 375365 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12849885 New failing tests: inspector/debugger/break-in-constructor-before-super.html inspector/debugger/breakpoint-eval-with-exception.html inspector/debugger/setBreakpointByUrl-sourceURL.html inspector/debugger/evaluateOnCallFrame-errors.html inspector/debugger/pause-on-assert.html inspector/debugger/setBreakpoint-condition.html inspector/debugger/regress-133182.html inspector/debugger/setBreakpoint-column.html

EWS Watchlist

Comment 20 2019-08-01 20:39:51 PDT Comment hidden (obsolete)

Created attachment 375386 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6

Devin Rousso

Comment 21 2019-08-02 01:15:00 PDT

Created attachment 375399 [details] Patch The tests were failing because `WebCore::ScriptController::executeScript` doesn't actually do anything if it's paused, whereas `WebCore::ScriptController::evaluate` does.

Blaze Burg

Comment 22 2019-08-08 11:08:06 PDT

Comment on attachment 375399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375399&action=review r=me > Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js:79 > + // COMPATIBILITY (iOS 13): Page.enable was removed. Should this say iOS 13 or 13.1?

Devin Rousso

Comment 23 2019-08-08 11:14:28 PDT

Comment on attachment 375399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375399&action=review >> Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js:79 >> + // COMPATIBILITY (iOS 13): Page.enable was removed. > > Should this say iOS 13 or 13.1? From what I've come to understand, we put the last iOS version that did NOT support this change, meaning that if we drop support for it, we could remove the check.

WebKit Commit Bot

Comment 24 2019-08-08 11:59:47 PDT Comment hidden (obsolete)

Comment on attachment 375399 [details] Patch Rejecting attachment 375399 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 375399, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=375399&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=200109&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Processing patch 375399 from bug 200109. Fetching: https://bugs.webkit.org/attachment.cgi?id=375399 Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Brian Burg']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Parsed 24 diffs from patch file(s). patching file Source/JavaScriptCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebInspectorUI/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/JavaScriptCore/inspector/protocol/Page.json patching file Source/WebCore/inspector/agents/InspectorApplicationCacheAgent.cpp Hunk #1 succeeded at 85 (offset 16 lines). Hunk #2 succeeded at 96 (offset 16 lines). Hunk #3 succeeded at 108 (offset 16 lines). Hunk #4 succeeded at 117 (offset 16 lines). Hunk #5 succeeded at 127 (offset 16 lines). patching file Source/WebCore/inspector/agents/InspectorCSSAgent.cpp patching file Source/WebCore/inspector/agents/InspectorDOMAgent.cpp patching file Source/WebCore/inspector/agents/InspectorPageAgent.cpp patching file Source/WebCore/inspector/agents/InspectorPageAgent.h patching file Source/WebCore/inspector/agents/InspectorTimelineAgent.cpp Hunk #1 succeeded at 740 (offset 28 lines). patching file Source/WebCore/inspector/agents/page/PageNetworkAgent.cpp patching file Source/WebCore/inspector/agents/page/PageRuntimeAgent.cpp patching file Source/WebCore/testing/Internals.cpp patching file Source/WebInspectorUI/UserInterface/Controllers/NetworkManager.js patching file Source/WebInspectorUI/UserInterface/Test/Test.js Hunk #1 succeeded at 77 (offset 5 lines). Hunk #2 succeeded at 91 (offset 5 lines). Hunk #3 succeeded at 145 with fuzz 1 (offset 12 lines). Hunk #4 succeeded at 172 (offset 12 lines). patching file Source/WebInspectorUI/UserInterface/Test/TestStub.js patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/http/tests/inspector/page/loading-iframe-document-node.html patching file LayoutTests/inspector/page/archive.html patching file LayoutTests/inspector/page/frameScheduledNavigation-async-delegates.html patching file LayoutTests/inspector/page/frameScheduledNavigation.html patching file LayoutTests/inspector/page/frameStartedLoading.html patching file LayoutTests/inspector/page/media-query-list-listener-exception.html patching file LayoutTests/inspector/timeline/line-column.html Hunk #1 FAILED at 36. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/inspector/timeline/line-column.html.rej Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Brian Burg']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/12880305

Devin Rousso

Comment 25 2019-08-08 16:18:37 PDT

Created attachment 375854 [details] Patch

WebKit Commit Bot

Comment 26 2019-08-08 18:10:47 PDT

Comment on attachment 375854 [details] Patch Clearing flags on attachment: 375854 Committed r248454: <https://trac.webkit.org/changeset/248454>

WebKit Commit Bot

Comment 27 2019-08-08 18:10:49 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 28 2019-08-08 18:11:22 PDT

<rdar://problem/54105611>

Truitt Savell

Comment 29 2019-08-09 11:49:20 PDT

It looks like the changes in https://trac.webkit.org/changeset/248454/webkit broke inspector/css/force-page-appearance.html on Debug and release wk1 History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fcss%2Fforce-page-appearance.html I was able to reproduce this locally by running the test on 248454 and it fails, on 248452 is passes. Can this be looked at today?

Devin Rousso

Comment 30 2019-08-09 13:07:20 PDT

(In reply to Truitt Savell from comment #29) > It looks like the changes in https://trac.webkit.org/changeset/248454/webkit > > broke inspector/css/force-page-appearance.html on Debug and release wk1 > > History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=inspector%2Fcss%2Fforce-page-appearance.html > > I was able to reproduce this locally by running the test on 248454 and it > fails, on 248452 is passes. > > Can this be looked at today? Looking now.

Devin Rousso

Comment 31 2019-08-09 14:15:07 PDT

(In reply to Devin Rousso from comment #30) > (In reply to Truitt Savell from comment #29) > > It looks like the changes in https://trac.webkit.org/changeset/248454/webkit > > > > broke inspector/css/force-page-appearance.html on Debug and release wk1 > > > > History: > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > html#showAllRuns=true&tests=inspector%2Fcss%2Fforce-page-appearance.html > > > > I was able to reproduce this locally by running the test on 248454 and it fails, on 248452 is passes. > > > > Can this be looked at today? > Looking now. <https://webkit.org/b/200587>

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

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS All

Product WebKit

Component Web Inspector

Assignee

Devin Rousso

Reported

2019-07-24 20:42 PDT

Modified

2019-08-17 00:11 PDT History

CC List

14 users Show

URL

Keywords InRadar

Depends on

Blocks

200110 200587

Dependencies

tree graph