[webkit-reviews] review denied: [Bug 47275] Web Inspector: inspector settings/properties/states management should be extracted into separate class. : [Attachment 70720] [patch] third iteration. OOP

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 47275: Web Inspector: inspector settings/properties/states management
should be extracted into separate class.
https://bugs.webkit.org/show_bug.cgi?id=47275

Attachment 70720: [patch] third iteration. OOP
https://bugs.webkit.org/attachment.cgi?id=70720&action=review

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

> WebCore/inspector/InspectorController.cpp:242
> +	   m_state->setBoolean(InspectorState::pauseOnExceptionsState,
m_debuggerAgent->pauseOnExceptionsState());

This should be setNumber.

> WebCore/inspector/InspectorController.cpp:244
> +    *state = m_state->getStateObjectForFrontend();

Nit: "toInspectorObject" ?

> WebCore/inspector/InspectorController.cpp:394
> +    m_state->setString(InspectorState::lastActivePanel, panelName);

Please add // FIXME: move last panel setting to the front-end?