[webkit-reviews] review denied: [Bug 99660] checkbox to toggle FPS counter in the inspector's settings : [Attachment 173319] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied egraether at chromium.org's
request for review:
Bug 99660: checkbox to toggle FPS counter in the inspector's settings
https://bugs.webkit.org/show_bug.cgi?id=99660

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

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173319&action=review


> Source/WebCore/inspector/InspectorPageAgent.cpp:721
> +    m_client->setShowFPSCounter(show);

Ok, now that I look at it, I can see that there are two important pieces
missing. Sorry for not spotting them earlier:

1) You need to m_client->setShowFPSCounter(false) from disable(ErrorString).
It is necessary to remove the FPS counter when inspector is closed. No
inspector settings should affect running page when inspector is closed.

2) You need to store enabled state in the m_state via
m_state->setBoolean(PageAgentState::showFPSCounter, show) here. 
Then, in enable(ErrorString) you should check it via m_state->getBoolean and in
case it is true, you should m_client->setShowFPSCounter(true);

This code path will be used in case when navigation changes renderer process.
You basically store your settings in the m_state cookie that is then passed
into the restore of the other renderer renderer. 

You can follow the setScriptExecutionDisabled and mimic its behavior.