[webkit-reviews] review granted: [Bug 45974] Web Inspector: refactoring. getBackendSettings was renamed to getInspectorState. : [Attachment 68077] [patch] second iteration

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 20 07:33:12 PDT 2010


Pavel Feldman <pfeldman at chromium.org> has granted Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 45974: Web Inspector: refactoring. getBackendSettings was renamed to
getInspectorState.
https://bugs.webkit.org/show_bug.cgi?id=45974

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

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

r+ with nits.

> WebCore/inspector/InspectorController.cpp:130
> +static const char* const timelineProfilerEnabledStateName =
"timelineProfilerEnabled";

Can we extract these into a separate class?

> WebCore/inspector/InspectorController.cpp:261
> +void InspectorController::saveInspectorState()

updateInspectorStateCookie

> WebCore/inspector/InspectorController.cpp:267
> +    m_client->saveInspectorState(state->toJSONString());

Save is a bad name for this method. m_client->updateInspectorStateCookie?

> WebCore/inspector/InspectorController.cpp:270
> +void InspectorController::restoreInspectorState(const String&
inspectorStateString)

restoreInspectorStateFromCookie ?

> WebCore/inspector/InspectorController.cpp:503
> +void InspectorController::setMonitoringXHREnabled(bool enable, bool*
newState)

nit: enabled


More information about the webkit-reviews mailing list