[webkit-reviews] review denied: [Bug 200109] Web Inspector: Page: don't allow the domain to be disabled : [Attachment 374873] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 200109: Web Inspector: Page: don't allow the domain to be disabled
https://bugs.webkit.org/show_bug.cgi?id=200109

Attachment 374873: Patch

https://bugs.webkit.org/attachment.cgi?id=374873&action=review




--- Comment #14 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 374873
  --> https://bugs.webkit.org/attachment.cgi?id=374873
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.