[webkit-reviews] review granted: [Bug 68530] Web Inspector: introduce Page.enable and Page.disable : [Attachment 108151] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has granted Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 68530: Web Inspector: introduce Page.enable and Page.disable
https://bugs.webkit.org/show_bug.cgi?id=68530

Attachment 108151: Patch
https://bugs.webkit.org/attachment.cgi?id=108151&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108151&action=review


> Source/WebCore/ChangeLog:5
> +

Is it possible to describe this change in a few words here?

> Source/WebCore/inspector/Inspector.json:147
> +		       { "name": "newWindow", "optional": true, "type":
"boolean", "description": "If true, opens given URL in a new window or tab.",
"hidden": true }

Let's remove this method altogether since the same effect can be reached using
the evaluate() command and there is no much sense in having a dedicated command
for this functionality.

> Source/WebCore/inspector/Inspector.json:548
> +		   "description": "Always send extra HTTP headers with the
requests from this page.",

style: should be something like "Specifies whether to always send..." for
consistency with other descriptions.

> Source/WebCore/inspector/InspectorController.cpp:355
> +    m_pageAgent->restore();

Shouldn't this call go before m_domAgent->restore?