[webkit-reviews] review denied: [Bug 107027] [Chromium] Hide the location bar on WebKit scrolls. : [Attachment 191249] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied John Knottenbelt
<jknotten at chromium.org>'s request for review:
Bug 107027: [Chromium] Hide the location bar on WebKit scrolls.
https://bugs.webkit.org/show_bug.cgi?id=107027

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191249&action=review


Where are your tests?

> Source/WebKit/chromium/src/WebViewImpl.cpp:4046
> +    // We want to hide the URL bar on ordinary load types (not back-forward
> +    // navigation / page reload) that are not originating from the
compositor.
> +    if (loader->loadType() == FrameLoadTypeStandard &&
!m_scrollFromCompositor) {

You seem to be conflating scrolls from the compositor with user-initiated
scrolls, but that's simply not the case.  The compositor will only handle
user-initiated scrolls but in several cases we fall back to the main thread to
handle the scroll itself.  How do those behave with this code?