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

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

.

[Devin Rousso]

Created attachment 374861 [details]
Patch

[EWS Watchlist]

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

[Devin Rousso]

Created attachment 374865 [details]
Patch

[Devin Rousso]

Created attachment 374867 [details]
Patch

[EWS Watchlist]

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]

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]

Created attachment 374873 [details]
Patch

[EWS Watchlist]

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]

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 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]

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 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]

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 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]

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

[Devin Rousso]

Created attachment 375365 [details]
Patch

[EWS Watchlist]

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]

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 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]

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]

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 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 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 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 [e-mail]...
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]

Created attachment 375854 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 375854 [details]
Patch

Clearing flags on attachment: 375854

Committed r248454: <https://trac.webkit.org/changeset/248454>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54105611>

[Truitt Savell]

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]

(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]

(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>