Don't use the wheel event handler count to track if a page has horizontal scrollbars

https://bugs.webkit.org/show_bug.cgi?id=78192

Reviewed by Andreas Kling.

Source/WebCore:

Stop calling Document::didAddWheelEventHandler and Document::didRemoveWheelEventHandler when
adding and removing scrollbars.

* page/FrameView.cpp:
* page/FrameView.h:
(FrameView):
* rendering/RenderLayer.cpp:
* rendering/RenderLayer.h:

Source/WebKit2:

Prior to this change, we were incrementing and decrementing the wheel event handler count
whenever a scrollable area gained or lost a horizontal scrollbar, so we could use the count
to determine if Safari can handle horizontal wheel events directly or whether they have to be sent
to the web process first.

What this meant was that whenever a page had horizontal scrollbars we'd have to send all scroll wheel events
to the main thread instead of the scrolling thread, regardless of whether there were any wheel event handlers.

After this change, we traverse the tree of scrollable areas after every layout and check if any of them
have a horizontal scrollbar. (We still also check if there are wheel event handlers).

If traversing the tree after every layout is deemed to slow we can go back to caching the number of horizontal scrollbars
in a page, but the number of subframes in a page is usually very small and the number of other scrollable areas is even smaller.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::willHandleHorizontalScrollEvents):
* UIProcess/WebPageProxy.h:
(WebKit::WebPageProxy::setCanShortCircuitHorizontalWheelEvents):
(WebPageProxy):
* UIProcess/WebPageProxy.messages.in:
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::numWheelEventHandlersChanged):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDidLayout):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::WebPage):
(WebKit::WebPage::numWheelEventHandlersChanged):
(WebKit):
(WebKit::hasEnabledHorizontalScrollbar):
(WebKit::pageContainsAnyHorizontalScrollbars):
(WebKit::WebPage::recomputeShortCircuitHorizontalWheelEventsState):
* WebProcess/WebPage/WebPage.h:
(WebPage):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107168 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 04d16f0..af2ff69 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2012-02-08  Anders Carlsson  <andersca@apple.com>
+
+        Don't use the wheel event handler count to track if a page has horizontal scrollbars
+        https://bugs.webkit.org/show_bug.cgi?id=78192
+
+        Reviewed by Andreas Kling.
+
+        Stop calling Document::didAddWheelEventHandler and Document::didRemoveWheelEventHandler when
+        adding and removing scrollbars.
+
+        * page/FrameView.cpp:
+        * page/FrameView.h:
+        (FrameView):
+        * rendering/RenderLayer.cpp:
+        * rendering/RenderLayer.h:
+
 2012-02-08  Igor Oliveira  <igor.o@sisa.samsung.com>
 
         Implement reverse animation direction

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index 4335e26..baba068 100644 (file)
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -321,21 +321,6 @@ void FrameView::detachCustomScrollbars()
     }
 }
 
-void FrameView::didAddHorizontalScrollbar(Scrollbar* scrollbar)
-{
-    if (m_frame && m_frame->document())
-        m_frame->document()->didAddWheelEventHandler();
-    ScrollView::didAddHorizontalScrollbar(scrollbar);
-}
-
-void FrameView::willRemoveHorizontalScrollbar(Scrollbar* scrollbar)
-{
-    ScrollView::willRemoveHorizontalScrollbar(scrollbar);
-    // FIXME: maybe need a separate ScrollableArea::didRemoveHorizontalScrollbar callback?
-    if (m_frame && m_frame->document())
-        m_frame->document()->didRemoveWheelEventHandler();
-}
-
 void FrameView::recalculateScrollbarOverlayStyle()
 {
     ScrollbarOverlayStyle oldOverlayStyle = scrollbarOverlayStyle();

diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
index 71242a2..51fe7c3 100644 (file)
--- a/Source/WebCore/page/FrameView.h
+++ b/Source/WebCore/page/FrameView.h
@@ -86,8 +86,6 @@ public:
     virtual PassRefPtr<Scrollbar> createScrollbar(ScrollbarOrientation);
 
     virtual bool avoidScrollbarCreation() const;
-    virtual void didAddHorizontalScrollbar(Scrollbar*);
-    virtual void willRemoveHorizontalScrollbar(Scrollbar*);
 
     virtual void setContentsSize(const IntSize&);
 

diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
index 8bfad44..ee6e8be 100644 (file)
--- a/Source/WebCore/rendering/RenderLayer.cpp
+++ b/Source/WebCore/rendering/RenderLayer.cpp
@@ -2061,19 +2061,6 @@ bool RenderLayer::allowsScrolling() const
     return (m_hBar && m_hBar->enabled()) || (m_vBar && m_vBar->enabled());
 }
 
-void RenderLayer::didAddHorizontalScrollbar(Scrollbar* scrollbar)
-{
-    m_renderer->document()->didAddWheelEventHandler();
-    ScrollableArea::didAddHorizontalScrollbar(scrollbar);
-}
-
-void RenderLayer::willRemoveHorizontalScrollbar(Scrollbar* scrollbar)
-{
-    ScrollableArea::willRemoveHorizontalScrollbar(scrollbar);
-    // FIXME: maybe need a separate ScrollableArea::didRemoveHorizontalScrollbar callback?
-    m_renderer->document()->didRemoveWheelEventHandler();
-}
-
 void RenderLayer::setHasHorizontalScrollbar(bool hasScrollbar)
 {
     if (hasScrollbar == (m_hBar != 0))

diff --git a/Source/WebCore/rendering/RenderLayer.h b/Source/WebCore/rendering/RenderLayer.h
index 776da1e..bb4797b 100644 (file)
--- a/Source/WebCore/rendering/RenderLayer.h
+++ b/Source/WebCore/rendering/RenderLayer.h
@@ -312,8 +312,6 @@ public:
     bool scrollsOverflow() const;
     bool allowsScrolling() const; // Returns true if at least one scrollbar is visible and enabled.
     bool hasScrollbars() const { return m_hBar || m_vBar; }
-    virtual void didAddHorizontalScrollbar(Scrollbar*);
-    virtual void willRemoveHorizontalScrollbar(Scrollbar*);
     void setHasHorizontalScrollbar(bool);
     void setHasVerticalScrollbar(bool);
 

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 3d24552..c0cd8e4 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,45 @@
+2012-02-08  Anders Carlsson  <andersca@apple.com>
+
+        Don't use the wheel event handler count to track if a page has horizontal scrollbars
+        https://bugs.webkit.org/show_bug.cgi?id=78192
+
+        Reviewed by Andreas Kling.
+
+        Prior to this change, we were incrementing and decrementing the wheel event handler count
+        whenever a scrollable area gained or lost a horizontal scrollbar, so we could use the count
+        to determine if Safari can handle horizontal wheel events directly or whether they have to be sent
+        to the web process first.
+
+        What this meant was that whenever a page had horizontal scrollbars we'd have to send all scroll wheel events
+        to the main thread instead of the scrolling thread, regardless of whether there were any wheel event handlers.
+        
+        After this change, we traverse the tree of scrollable areas after every layout and check if any of them
+        have a horizontal scrollbar. (We still also check if there are wheel event handlers).
+
+        If traversing the tree after every layout is deemed to slow we can go back to caching the number of horizontal scrollbars
+        in a page, but the number of subframes in a page is usually very small and the number of other scrollable areas is even smaller.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::WebPageProxy):
+        (WebKit::WebPageProxy::willHandleHorizontalScrollEvents):
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::setCanShortCircuitHorizontalWheelEvents):
+        (WebPageProxy):
+        * UIProcess/WebPageProxy.messages.in:
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::numWheelEventHandlersChanged):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::dispatchDidLayout):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::WebPage):
+        (WebKit::WebPage::numWheelEventHandlersChanged):
+        (WebKit):
+        (WebKit::hasEnabledHorizontalScrollbar):
+        (WebKit::pageContainsAnyHorizontalScrollbars):
+        (WebKit::WebPage::recomputeShortCircuitHorizontalWheelEventsState):
+        * WebProcess/WebPage/WebPage.h:
+        (WebPage):
+
 2012-02-08  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r106920, r106924, r106933, r106939,

diff --git a/Source/WebKit2/UIProcess/WebPageProxy.cpp b/Source/WebKit2/UIProcess/WebPageProxy.cpp
index 72a2c1a..85b7d54 100644 (file)
--- a/Source/WebKit2/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit2/UIProcess/WebPageProxy.cpp
@@ -195,6 +195,7 @@ WebPageProxy::WebPageProxy(PageClient* pageClient, PassRefPtr<WebProcessProxy> p
     , m_mainFrameHasCustomRepresentation(false)
     , m_mainFrameHasHorizontalScrollbar(false)
     , m_mainFrameHasVerticalScrollbar(false)
+    , m_canShortCircuitHorizontalWheelEvents(true)
     , m_mainFrameIsPinnedToLeftSide(false)
     , m_mainFrameIsPinnedToRightSide(false)
     , m_pageCount(0)
@@ -3456,7 +3457,7 @@ void WebPageProxy::didFailToInitializePlugin(const String& mimeType)
 
 bool WebPageProxy::willHandleHorizontalScrollEvents() const
 {
-    return m_wheelEventHandlerCount > 0;
+    return !m_canShortCircuitHorizontalWheelEvents;
 }
 
 void WebPageProxy::didFinishLoadingDataForCustomRepresentation(const String& suggestedFilename, const CoreIPC::DataReference& dataReference)

diff --git a/Source/WebKit2/UIProcess/WebPageProxy.h b/Source/WebKit2/UIProcess/WebPageProxy.h
index 8fc6555..2e64d16 100644 (file)
--- a/Source/WebKit2/UIProcess/WebPageProxy.h
+++ b/Source/WebKit2/UIProcess/WebPageProxy.h
@@ -719,7 +719,7 @@ private:
     void didChangeScrollOffsetPinningForMainFrame(bool pinnedToLeftSide, bool pinnedToRightSide);
     void didChangePageCount(unsigned);
     void didFailToInitializePlugin(const String& mimeType);
-    void numWheelEventHandlersChanged(unsigned count) { m_wheelEventHandlerCount = count; }
+    void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
 
     void reattachToWebProcess();
     void reattachToWebProcessWithItem(WebBackForwardListItem*);
@@ -1009,7 +1009,9 @@ private:
 
     bool m_mainFrameHasHorizontalScrollbar;
     bool m_mainFrameHasVerticalScrollbar;
-    int m_wheelEventHandlerCount;
+
+    // Whether horizontal wheel events can be handled directly for swiping purposes.
+    bool m_canShortCircuitHorizontalWheelEvents;
 
     bool m_mainFrameIsPinnedToLeftSide;
     bool m_mainFrameIsPinnedToRightSide;

diff --git a/Source/WebKit2/UIProcess/WebPageProxy.messages.in b/Source/WebKit2/UIProcess/WebPageProxy.messages.in
index 717853b..9ba1d52 100644 (file)
--- a/Source/WebKit2/UIProcess/WebPageProxy.messages.in
+++ b/Source/WebKit2/UIProcess/WebPageProxy.messages.in
@@ -66,7 +66,7 @@ messages -> WebPageProxy {
     DidChangeScrollOffsetPinningForMainFrame(bool hasHorizontalScrollbar, bool hasVerticalScrollbar)
     DidChangePageCount(unsigned pageCount);
     DidFailToInitializePlugin(WTF::String mimeType)
-    NumWheelEventHandlersChanged(unsigned count)
+    SetCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents)
 
 #if USE(TILED_BACKING_STORE)
     PageDidRequestScroll(WebCore::IntPoint point)

diff --git a/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
index 44c0c00..5a48fff 100644 (file)
--- a/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
+++ b/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
@@ -780,7 +780,7 @@ bool WebChromeClient::shouldRubberBandInDirection(WebCore::ScrollDirection direc
 
 void WebChromeClient::numWheelEventHandlersChanged(unsigned count)
 {
-    m_page->send(Messages::WebPageProxy::NumWheelEventHandlersChanged(count));
+    m_page->numWheelEventHandlersChanged(count);
 }
 
 } // namespace WebKit

diff --git a/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
index 37e261f..be24684 100644 (file)
--- a/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
+++ b/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
@@ -577,6 +577,8 @@ void WebFrameLoaderClient::dispatchDidLayout()
     // Notify the bundle client.
     webPage->injectedBundleLoaderClient().didLayoutForFrame(webPage, m_frame);
 
+    webPage->recomputeShortCircuitHorizontalWheelEventsState();
+
     // NOTE: Unlike the other layout notifications, this does not notify the
     // the UIProcess for every call.
 

diff --git a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
index f8a86ae..4aa1e45 100644 (file)
--- a/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
+++ b/Source/WebKit2/WebProcess/WebPage/WebPage.cpp
@@ -206,6 +206,8 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
     , m_isRunningModal(false)
     , m_cachedMainFrameIsPinnedToLeftSide(false)
     , m_cachedMainFrameIsPinnedToRightSide(false)
+    , m_canShortCircuitHorizontalWheelEvents(false)
+    , m_numWheelEventHandlers(0)
     , m_cachedPageCount(0)
     , m_isShowingContextMenu(false)
 #if PLATFORM(WIN)
@@ -3000,6 +3002,68 @@ void WebPage::confirmCompositionForTesting(const String& compositionString)
     frame->editor()->confirmComposition(compositionString);
 }
 
+void WebPage::numWheelEventHandlersChanged(unsigned numWheelEventHandlers)
+{
+    if (m_numWheelEventHandlers == numWheelEventHandlers)
+        return;
+
+    m_numWheelEventHandlers = numWheelEventHandlers;
+    recomputeShortCircuitHorizontalWheelEventsState();
+}
+
+static bool hasEnabledHorizontalScrollbar(ScrollableArea* scrollableArea)
+{
+    if (Scrollbar* scrollbar = scrollableArea->horizontalScrollbar())
+        return scrollbar->enabled();
+
+    return false;
+}
+
+static bool pageContainsAnyHorizontalScrollbars(Frame* mainFrame)
+{
+    if (FrameView* frameView = mainFrame->view()) {
+        if (hasEnabledHorizontalScrollbar(frameView))
+            return true;
+    }
+
+    for (Frame* frame = mainFrame; frame; frame = frame->tree()->traverseNext()) {
+        FrameView* frameView = frame->view();
+        if (!frameView)
+            continue;
+
+        const HashSet<ScrollableArea*>* scrollableAreas = frameView->scrollableAreas();
+        if (!scrollableAreas)
+            continue;
+
+        for (HashSet<ScrollableArea*>::const_iterator it = scrollableAreas->begin(), end = scrollableAreas->end(); it != end; ++it) {
+            ScrollableArea* scrollableArea = *it;
+            ASSERT(scrollableArea->isOnActivePage());
+
+            if (hasEnabledHorizontalScrollbar(scrollableArea))
+                return true;
+        }
+    }
+
+    return false;
+}
+
+void WebPage::recomputeShortCircuitHorizontalWheelEventsState()
+{
+    bool canShortCircuitHorizontalWheelEvents = !m_numWheelEventHandlers;
+
+    if (canShortCircuitHorizontalWheelEvents) {
+        // Check if we have any horizontal scroll bars on the page.
+        if (pageContainsAnyHorizontalScrollbars(mainFrame()))
+            canShortCircuitHorizontalWheelEvents = false;
+    }
+
+    if (m_canShortCircuitHorizontalWheelEvents == canShortCircuitHorizontalWheelEvents)
+        return;
+
+    m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents;
+    send(Messages::WebPageProxy::SetCanShortCircuitHorizontalWheelEvents(m_canShortCircuitHorizontalWheelEvents));
+}
+
 Frame* WebPage::mainFrame() const
 {
     return m_page ? m_page->mainFrame() : 0;

diff --git a/Source/WebKit2/WebProcess/WebPage/WebPage.h b/Source/WebKit2/WebProcess/WebPage/WebPage.h
index b5dd560..aa1e57f 100644 (file)
--- a/Source/WebKit2/WebProcess/WebPage/WebPage.h
+++ b/Source/WebKit2/WebProcess/WebPage/WebPage.h
@@ -500,6 +500,9 @@ public:
     void gestureEvent(const WebGestureEvent&);
 #endif
 
+    void numWheelEventHandlersChanged(unsigned);
+    void recomputeShortCircuitHorizontalWheelEventsState();
+
 private:
     WebPage(uint64_t pageID, const WebPageCreationParameters&);
 
@@ -756,6 +759,8 @@ private:
 
     bool m_cachedMainFrameIsPinnedToLeftSide;
     bool m_cachedMainFrameIsPinnedToRightSide;
+    bool m_canShortCircuitHorizontalWheelEvents;
+    unsigned m_numWheelEventHandlers;
 
     unsigned m_cachedPageCount;