[webkit-reviews] review denied: [Bug 62985] Web Inspector: introduce InspectorFrontendAPI for actions initiated from the application menu. : [Attachment 97964] Patch working both on Mac and Windows.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 21 06:31:04 PDT 2011


Yury Semikhatsky <yurys at chromium.org> has denied Pavel Feldman
<pfeldman at chromium.org>'s request for review:
Bug 62985: Web Inspector: introduce InspectorFrontendAPI for actions initiated
from the application menu.
https://bugs.webkit.org/show_bug.cgi?id=62985

Attachment 97964: Patch working both on Mac and Windows.
https://bugs.webkit.org/attachment.cgi?id=97964&action=review

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

> Source/WebCore/ChangeLog:36
> +   warning: LF will be replaced by CRLF in
Source/WebCore/WebCore.vcproj/WebCore.vcproj.

Something wrong with this changelog entry, please fix.

> Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:175
> +    m_frontendClient = new WebInspectorFrontendClient(m_inspectedWebView,
m_inspectedWebViewHwnd, frontendHwnd, frontendWebView, frontendWebViewHwnd,
this, createFrontendSettings());

Plseas use adoptPtr(new WebInspectorFrontendClient(...)) and .leakPtr() below.

> Source/WebKit/win/WebCoreSupport/WebInspectorClient.h:71
> +    void releaseFrontendClient();

Can we use one releaseFrontendXXX method instead?

> Source/WebKit/win/WebView.cpp:2642
> +    m_inspectorClient = new WebInspectorClient(this);

There should be adoptPtr().


More information about the webkit-reviews mailing list