[webkit-reviews] review denied: [Bug 46802] Web Inspector: extract consoleMessages related stuff from populateScriptObjects into separate function. : [Attachment 69605] [patch] second iteration

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 4 02:25:02 PDT 2010


Yury Semikhatsky <yurys at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 46802: Web Inspector: extract consoleMessages related stuff from
populateScriptObjects into separate function.
https://bugs.webkit.org/show_bug.cgi?id=46802

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

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

> WebCore/inspector/InspectorController.cpp:127
> +static const char* const consoleMessagesNotificationsStateName =
"consoleMessagesNotificationsEnabled";

Should we move this constants into a class/namespace InspectorSettingNames
instead of using *StateName suffix?

> WebCore/inspector/InspectorController.cpp:162
> +    , m_consoleMessagesNotifications(false)

m_consoleMessagesNotifications -> m_consoleMessagesNotificationsEnabled

> WebCore/inspector/InspectorController.cpp:603
> +    m_consoleMessagesNotifications = false;

This will be called only on close of the detached window. When attached window
is closed only disconnectFrontend will be invoked. r- for this.

> WebCore/inspector/InspectorController.h:150
> +    void setConsoleMessagesNotificationsEnabled(bool enabled);

should be private


More information about the webkit-reviews mailing list