[webkit-reviews] review denied: [Bug 60484] ChromeClient::webView() is only needed for Chromium port : [Attachment 92811] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 9 12:00:12 PDT 2011


Adam Barth <abarth at webkit.org> has denied David Kilzer (ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 60484: ChromeClient::webView() is only needed for Chromium port
https://bugs.webkit.org/show_bug.cgi?id=60484

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=92811&action=review

> Source/WebCore/page/ChromeClient.h:137
> +#if PLATFORM(CHROMIUM)
>	   virtual void* webView() const = 0;
> +#endif

Why is this feature Chromium-specific?	Having a WebView is a general concept
that's shared by most ports.  It would be nice to have more explanation in the
changelog for why we're making this change.

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.h:39
> -    virtual void* webView() const { return static_cast<void*>(m_webView); }
> +    WebView* webView() const { return m_webView; }

It's confusing that this client has a method that's exactly the same name as
one in its superclass but that's ifdefed away on this port.

> Source/WebKit/win/WebCoreSupport/WebChromeClient.h:43
> +    WebView* webView() const { return m_webView; }

Same here


More information about the webkit-reviews mailing list