Changeset 107168 in webkit

Timestamp:

Feb 8, 2012, 7:02:25 PM (15 years ago)

Author:

andersca@apple.com

Message:

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):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/FrameView.cpp (modified) (1 diff)
• WebCore/page/FrameView.h (modified) (1 diff)
• WebCore/rendering/RenderLayer.cpp (modified) (1 diff)
• WebCore/rendering/RenderLayer.h (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/UIProcess/WebPageProxy.cpp (modified) (2 diffs)
• WebKit2/UIProcess/WebPageProxy.h (modified) (2 diffs)
• WebKit2/UIProcess/WebPageProxy.messages.in (modified) (1 diff)
• WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp (modified) (1 diff)
• WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (modified) (1 diff)
• WebKit2/WebProcess/WebPage/WebPage.cpp (modified) (2 diffs)
• WebKit2/WebProcess/WebPage/WebPage.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -322 +322 @@
 }
 
-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()
 {

trunk/Source/WebCore/page/FrameView.h

@@ -87 +87 @@
 
     virtual bool avoidScrollbarCreation() const;
-    virtual void didAddHorizontalScrollbar(Scrollbar*);
-    virtual void willRemoveHorizontalScrollbar(Scrollbar*);
 
     virtual void setContentsSize(const IntSize&);

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -2062 +2062 @@
 }
 
-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)
 {

trunk/Source/WebCore/rendering/RenderLayer.h

@@ -313 +313 @@
     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);

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -196 +196 @@
     , m_mainFrameHasHorizontalScrollbar(false)
     , m_mainFrameHasVerticalScrollbar(false)
+    , m_canShortCircuitHorizontalWheelEvents(true)
     , m_mainFrameIsPinnedToLeftSide(false)
     , m_mainFrameIsPinnedToRightSide(false)
@@ -3457 +3458 @@
 bool WebPageProxy::willHandleHorizontalScrollEvents() const
 {
-    return m_wheelEventHandlerCount > 0;
+    return !m_canShortCircuitHorizontalWheelEvents;
 }
 

trunk/Source/WebKit2/UIProcess/WebPageProxy.h

@@ -720 +720 @@
     void didChangePageCount(unsigned);
     void didFailToInitializePlugin(const String& mimeType);
-    void numWheelEventHandlersChanged(unsigned count) { m_wheelEventHandlerCount = count; }
+    void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
 
     void reattachToWebProcess();
@@ -1010 +1010 @@
     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;

trunk/Source/WebKit2/UIProcess/WebPageProxy.messages.in

@@ -67 +67 @@
     DidChangePageCount(unsigned pageCount);
     DidFailToInitializePlugin(WTF::String mimeType)
-    NumWheelEventHandlersChanged(unsigned count)
+    SetCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents)
 
 #if USE(TILED_BACKING_STORE)

trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp

@@ -781 +781 @@
 void WebChromeClient::numWheelEventHandlersChanged(unsigned count)
 {
-    m_page->send(Messages::WebPageProxy::NumWheelEventHandlersChanged(count));
+    m_page->numWheelEventHandlersChanged(count);
 }
 

trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

@@ -578 +578 @@
     webPage->injectedBundleLoaderClient().didLayoutForFrame(webPage, m_frame);
 
+    webPage->recomputeShortCircuitHorizontalWheelEventsState();
+
     // NOTE: Unlike the other layout notifications, this does not notify the
     // the UIProcess for every call.

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -207 +207 @@
     , m_cachedMainFrameIsPinnedToLeftSide(false)
     , m_cachedMainFrameIsPinnedToRightSide(false)
+    , m_canShortCircuitHorizontalWheelEvents(false)
+    , m_numWheelEventHandlers(0)
     , m_cachedPageCount(0)
     , m_isShowingContextMenu(false)
@@ -3001 +3003 @@
 }
 
+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
 {

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h

@@ -501 +501 @@
 #endif
 
+    void numWheelEventHandlersChanged(unsigned);
+    void recomputeShortCircuitHorizontalWheelEventsState();
+
 private:
     WebPage(uint64_t pageID, const WebPageCreationParameters&);
@@ -757 +760 @@
     bool m_cachedMainFrameIsPinnedToLeftSide;
     bool m_cachedMainFrameIsPinnedToRightSide;
+    bool m_canShortCircuitHorizontalWheelEvents;
+    unsigned m_numWheelEventHandlers;
 
     unsigned m_cachedPageCount;