[webkit-reviews] review denied: [Bug 107026] Allow WebKit scrolls to hide URL bar in Chrome on Android. : [Attachment 188607] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 15 14:55:19 PST 2013


Adam Barth <abarth at webkit.org> has denied John Knottenbelt
<jknotten at chromium.org>'s request for review:
Bug 107026: Allow WebKit scrolls to hide URL bar in Chrome on Android.
https://bugs.webkit.org/show_bug.cgi?id=107026

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

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


> Source/WebCore/page/scrolling/chromium/ScrollingCoordinatorChromium.cpp:304
> +    ChromeClientChromium* client = static_cast<ChromeClientChromium*>(
> +	   m_page->chrome()->client());

This static_cast looks pretty dubious....

> Source/WebKit/chromium/src/WebViewImpl.cpp:4209
> +    m_insideCompositor = true;

These sort of state variable tend to cause maintenance	problems.  We have a
bunch of them in FrameLoader and they cause a mess.  It's better for the
controlflow to be explicit rather than implicit.


More information about the webkit-reviews mailing list