Remove scrolling tree dependency on wheel event handler counts, and use fast scrollin...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Apr 2015 18:30:39 +0000 (18:30 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Apr 2015 18:30:39 +0000 (18:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143288
rdar://problem/16049624

Reviewed by Beth Dakin.

Remove the wheel event counting that Document does, and passes into the scrolling tree.
The ScrollingTree now just uses the non-fast scrollable region to determine when to
fast scroll on pages with wheel event handlers.

If a handler includes position:fixed renderers, we just cover the whole document
with the slow-scrolling region currently. This could be improved.

* dom/Document.cpp:
(WebCore::Document::Document):
(WebCore::Document::didBecomeCurrentDocumentInFrame):
(WebCore::Document::wheelEventHandlersChanged):
(WebCore::Document::didAddWheelEventHandler):
(WebCore::Document::didRemoveWheelEventHandler):
(WebCore::Document::wheelEventHandlerCount):
(WebCore::Document::touchEventHandlerCount):
(WebCore::Document::absoluteRegionForEventTargets): Changed to return a pair<Region, bool>
where the bool indicates whether any handler includes position:fixed content.
(WebCore::pageWheelEventHandlerCountChanged): Deleted.
(WebCore::wheelEventHandlerCountChanged): Deleted.
* dom/Document.h:
(WebCore::Document::wheelEventHandlerCount): Deleted.
* loader/EmptyClients.h:
* page/ChromeClient.h:
* page/DebugPageOverlays.cpp:
(WebCore::MouseWheelRegionOverlay::updateRegion):
* page/scrolling/AsyncScrollingCoordinator.cpp:
(WebCore::AsyncScrollingCoordinator::frameViewNonFastScrollableRegionChanged):
(WebCore::AsyncScrollingCoordinator::recomputeWheelEventHandlerCountForFrameView): Deleted.
* page/scrolling/AsyncScrollingCoordinator.h:
* page/scrolling/ScrollingCoordinator.cpp:
(WebCore::ScrollingCoordinator::computeNonFastScrollableRegion):
(WebCore::ScrollingCoordinator::frameViewRootLayerDidChange):
(WebCore::ScrollingCoordinator::computeCurrentWheelEventHandlerCount): Deleted.
(WebCore::ScrollingCoordinator::frameViewWheelEventHandlerCountChanged): Deleted.
* page/scrolling/ScrollingCoordinator.h:
(WebCore::ScrollingCoordinator::recomputeWheelEventHandlerCountForFrameView): Deleted.
* page/scrolling/ScrollingStateFrameScrollingNode.cpp:
(WebCore::ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode):
(WebCore::ScrollingStateFrameScrollingNode::setWheelEventHandlerCount): Deleted.
* page/scrolling/ScrollingStateFrameScrollingNode.h:
* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::ScrollingTree):
(WebCore::ScrollingTree::shouldHandleWheelEventSynchronously):
(WebCore::ScrollingTree::commitNewTreeState):
* page/scrolling/ScrollingTree.h:
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::updateTiledScrollingIndicator):
* page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
(WebCore::ScrollingTreeFrameScrollingNodeMac::updateBeforeChildren):
(WebCore::logWheelEventHandlerCountChanged): Deleted.
* testing/Internals.cpp:
(WebCore::Internals::touchEventHandlerCount):

Source/WebKit/mac:
Remove scrolling tree dependency on wheel event handler counts, and use fast scrolling even when there are wheel handlers
https://bugs.webkit.org/show_bug.cgi?id=143288

Reviewed by Beth Dakin.

Remove the wheel event counting that Document does, and passes into the scrolling tree.
The ScrollingTree now just uses the non-fast scrollable region to determine when to
fast scroll on pages with wheel event handlers.

* WebCoreSupport/WebChromeClient.h:

Source/WebKit2:
Remove scrolling tree dependency on wheel event handler counts, and use fast scrolling even when there are wheel handlers
https://bugs.webkit.org/show_bug.cgi?id=143288
rdar://problem/16049624

Reviewed by Beth Dakin.

Remove the wheel event counting that Document does, and passes into the scrolling tree.
The ScrollingTree now just uses the non-fast scrollable region to determine when to
fast scroll on pages with wheel event handlers.

If a handler includes position:fixed renderers, we just cover the whole document
with the slow-scrolling region currently. This could be improved.

* Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:
(ArgumentCoder<ScrollingStateFrameScrollingNode>::encode):
(ArgumentCoder<ScrollingStateFrameScrollingNode>::decode):
(WebKit::RemoteScrollingTreeTextStream::dump):
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::wheelEventHandlersChanged):
(WebKit::WebChromeClient::numWheelEventHandlersChanged): Deleted.
* WebProcess/WebCoreSupport/WebChromeClient.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::WebPage):
(WebKit::WebPage::wheelEventHandlersChanged):
(WebKit::WebPage::recomputeShortCircuitHorizontalWheelEventsState):
(WebKit::WebPage::numWheelEventHandlersChanged): Deleted.
* WebProcess/WebPage/WebPage.h:

LayoutTests:
Make it possible to compute a region for elements on the page that have wheel event handlers
https://bugs.webkit.org/show_bug.cgi?id=142807

Reviewed by Beth Dakin.

Update results, since any handler with position:fixed now causes the region to cover the document.

* platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-fixed-child-expected.txt:
* platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-inside-fixed-expected.txt:
* platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-on-fixed-expected.txt:

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

29 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-fixed-child-expected.txt
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-inside-fixed-expected.txt
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-on-fixed-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/loader/EmptyClients.h
Source/WebCore/page/ChromeClient.h
Source/WebCore/page/DebugPageOverlays.cpp
Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp
Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h
Source/WebCore/page/scrolling/ScrollingCoordinator.cpp
Source/WebCore/page/scrolling/ScrollingCoordinator.h
Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.cpp
Source/WebCore/page/scrolling/ScrollingStateFrameScrollingNode.h
Source/WebCore/page/scrolling/ScrollingTree.cpp
Source/WebCore/page/scrolling/ScrollingTree.h
Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm
Source/WebCore/testing/Internals.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebCoreSupport/WebChromeClient.h
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.h
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h

index 716b569..3af250f 100644 (file)
@@ -1,3 +1,16 @@
+2015-03-31  Simon Fraser  <simon.fraser@apple.com>
+
+        Make it possible to compute a region for elements on the page that have wheel event handlers
+        https://bugs.webkit.org/show_bug.cgi?id=142807
+
+        Reviewed by Beth Dakin.
+        
+        Update results, since any handler with position:fixed now causes the region to cover the document.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-fixed-child-expected.txt:
+        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-inside-fixed-expected.txt:
+        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/wheel-handler-on-fixed-expected.txt:
+
 2015-04-01  David Hyatt  <hyatt@apple.com>
 
         [New Block-Inside-Inline Model] Floats need to be allowed to intrude into anonymous inline-blocks.
index 4aff0d2..cab5edc 100644 (file)
@@ -1,3 +1,64 @@
+2015-03-31  Simon Fraser  <simon.fraser@apple.com>
+
+        Remove scrolling tree dependency on wheel event handler counts, and use fast scrolling even when there are wheel handlers
+        https://bugs.webkit.org/show_bug.cgi?id=143288
+        rdar://problem/16049624
+
+        Reviewed by Beth Dakin.
+
+        Remove the wheel event counting that Document does, and passes into the scrolling tree.
+        The ScrollingTree now just uses the non-fast scrollable region to determine when to
+        fast scroll on pages with wheel event handlers.
+        
+        If a handler includes position:fixed renderers, we just cover the whole document
+        with the slow-scrolling region currently. This could be improved.
+        
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::didBecomeCurrentDocumentInFrame):
+        (WebCore::Document::wheelEventHandlersChanged):
+        (WebCore::Document::didAddWheelEventHandler):
+        (WebCore::Document::didRemoveWheelEventHandler):
+        (WebCore::Document::wheelEventHandlerCount):
+        (WebCore::Document::touchEventHandlerCount):
+        (WebCore::Document::absoluteRegionForEventTargets): Changed to return a pair<Region, bool>
+        where the bool indicates whether any handler includes position:fixed content.
+        (WebCore::pageWheelEventHandlerCountChanged): Deleted.
+        (WebCore::wheelEventHandlerCountChanged): Deleted.
+        * dom/Document.h:
+        (WebCore::Document::wheelEventHandlerCount): Deleted.
+        * loader/EmptyClients.h:
+        * page/ChromeClient.h:
+        * page/DebugPageOverlays.cpp:
+        (WebCore::MouseWheelRegionOverlay::updateRegion):
+        * page/scrolling/AsyncScrollingCoordinator.cpp:
+        (WebCore::AsyncScrollingCoordinator::frameViewNonFastScrollableRegionChanged):
+        (WebCore::AsyncScrollingCoordinator::recomputeWheelEventHandlerCountForFrameView): Deleted.
+        * page/scrolling/AsyncScrollingCoordinator.h:
+        * page/scrolling/ScrollingCoordinator.cpp:
+        (WebCore::ScrollingCoordinator::computeNonFastScrollableRegion):
+        (WebCore::ScrollingCoordinator::frameViewRootLayerDidChange):
+        (WebCore::ScrollingCoordinator::computeCurrentWheelEventHandlerCount): Deleted.
+        (WebCore::ScrollingCoordinator::frameViewWheelEventHandlerCountChanged): Deleted.
+        * page/scrolling/ScrollingCoordinator.h:
+        (WebCore::ScrollingCoordinator::recomputeWheelEventHandlerCountForFrameView): Deleted.
+        * page/scrolling/ScrollingStateFrameScrollingNode.cpp:
+        (WebCore::ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode):
+        (WebCore::ScrollingStateFrameScrollingNode::setWheelEventHandlerCount): Deleted.
+        * page/scrolling/ScrollingStateFrameScrollingNode.h:
+        * page/scrolling/ScrollingTree.cpp:
+        (WebCore::ScrollingTree::ScrollingTree):
+        (WebCore::ScrollingTree::shouldHandleWheelEventSynchronously):
+        (WebCore::ScrollingTree::commitNewTreeState):
+        * page/scrolling/ScrollingTree.h:
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::updateTiledScrollingIndicator):
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeFrameScrollingNodeMac::updateBeforeChildren):
+        (WebCore::logWheelEventHandlerCountChanged): Deleted.
+        * testing/Internals.cpp:
+        (WebCore::Internals::touchEventHandlerCount):
+
 2015-04-01  David Hyatt  <hyatt@apple.com>
 
         [New Block-Inside-Inline Model] Floats need to be allowed to intrude into anonymous inline-blocks.
index c779d2e..b2be2b1 100644 (file)
@@ -490,7 +490,6 @@ Document::Document(Frame* frame, const URL& url, unsigned documentClasses, unsig
     , m_writingModeSetOnDocumentElement(false)
     , m_writeRecursionIsTooDeep(false)
     , m_writeRecursionDepth(0)
-    , m_wheelEventHandlerCount(0)
     , m_lastHandledUserGestureTimestamp(0)
 #if PLATFORM(IOS)
 #if ENABLE(DEVICE_ORIENTATION)
@@ -2098,16 +2097,6 @@ void Document::createRenderTree()
     recalcStyle(Style::Force);
 }
 
-static void pageWheelEventHandlerCountChanged(Page& page)
-{
-    unsigned count = 0;
-    for (const Frame* frame = &page.mainFrame(); frame; frame = frame->tree().traverseNext()) {
-        if (Document* document = frame->document())
-            count += document->wheelEventHandlerCount();
-    }
-    page.chrome().client().numWheelEventHandlersChanged(count);
-}
-
 void Document::didBecomeCurrentDocumentInFrame()
 {
     // FIXME: Are there cases where the document can be dislodged from the frame during the event handling below?
@@ -2128,7 +2117,7 @@ void Document::didBecomeCurrentDocumentInFrame()
     // subframes' documents have no wheel event handlers, then the count did not change,
     // unless the documents they are replacing had wheel event handlers.
     if (page() && m_frame->isMainFrame())
-        pageWheelEventHandlerCountChanged(*page());
+        wheelEventHandlersChanged();
 
 #if ENABLE(TOUCH_EVENTS)
     // FIXME: Doing this only for the main frame is insufficient.
@@ -5982,30 +5971,23 @@ RefPtr<Touch> Document::createTouch(DOMWindow* window, EventTarget* target, int
 #endif
 #endif // !PLATFORM(IOS)
 
-static void wheelEventHandlerCountChanged(Document* document)
+void Document::wheelEventHandlersChanged()
 {
-    Page* page = document->page();
+    Page* page = this->page();
     if (!page)
         return;
 
-    pageWheelEventHandlerCountChanged(*page);
-
-    ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator();
-    if (!scrollingCoordinator)
-        return;
-
-    FrameView* frameView = document->view();
-    if (!frameView)
-        return;
+    if (FrameView* frameView = view()) {
+        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
+            scrollingCoordinator->frameViewNonFastScrollableRegionChanged(*frameView);
+    }
 
-    // FIXME: Why doesn't this need to be called in didBecomeCurrentDocumentInFrame?
-    scrollingCoordinator->frameViewWheelEventHandlerCountChanged(*frameView);
+    bool haveHandlers = m_wheelEventTargets && !m_wheelEventTargets->isEmpty();
+    page->chrome().client().wheelEventHandlersChanged(haveHandlers);
 }
 
 void Document::didAddWheelEventHandler(Node& node)
 {
-    ++m_wheelEventHandlerCount;
-
     if (!m_wheelEventTargets)
         m_wheelEventTargets = std::make_unique<EventTargetSet>();
 
@@ -6016,7 +5998,7 @@ void Document::didAddWheelEventHandler(Node& node)
         return;
     }
 
-    wheelEventHandlerCountChanged(this);
+    wheelEventHandlersChanged();
 
     if (Frame* frame = this->frame())
         DebugPageOverlays::didChangeEventHandlers(*frame);
@@ -6024,9 +6006,6 @@ void Document::didAddWheelEventHandler(Node& node)
 
 void Document::didRemoveWheelEventHandler(Node& node)
 {
-    ASSERT(m_wheelEventHandlerCount > 0);
-    --m_wheelEventHandlerCount;
-
     if (!m_wheelEventTargets)
         return;
 
@@ -6038,12 +6017,24 @@ void Document::didRemoveWheelEventHandler(Node& node)
         return;
     }
 
-    wheelEventHandlerCountChanged(this);
+    wheelEventHandlersChanged();
 
     if (Frame* frame = this->frame())
         DebugPageOverlays::didChangeEventHandlers(*frame);
 }
 
+unsigned Document::wheelEventHandlerCount() const
+{
+    if (!m_wheelEventTargets)
+        return 0;
+
+    unsigned count = 0;
+    for (auto& handler : *m_wheelEventTargets)
+        count += handler.value;
+
+    return count;
+}
+
 void Document::didAddTouchEventHandler(Node& handler)
 {
 #if ENABLE(TOUCH_EVENTS)
@@ -6114,6 +6105,22 @@ void Document::didRemoveEventTargetNode(Node& handler)
     }
 }
 
+unsigned Document::touchEventHandlerCount() const
+{
+#if ENABLE(TOUCH_EVENTS)
+    if (!m_touchEventTargets)
+        return 0;
+
+    unsigned count = 0;
+    for (auto& handler : *m_touchEventTargets)
+        count += handler.value;
+
+    return count;
+#else
+    return 0;
+#endif
+}
+
 LayoutRect Document::absoluteEventHandlerBounds(bool& includesFixedPositionElements)
 {
     includesFixedPositionElements = false;
@@ -6123,10 +6130,10 @@ LayoutRect Document::absoluteEventHandlerBounds(bool& includesFixedPositionEleme
     return LayoutRect();
 }
 
-Region Document::absoluteRegionForEventTargets(const EventTargetSet* targets)
+Document::RegionFixedPair Document::absoluteRegionForEventTargets(const EventTargetSet* targets)
 {
     if (!targets)
-        return Region();
+        return RegionFixedPair(Region(), false);
 
     Region targetRegion;
     bool insideFixedPosition = false;
@@ -6149,7 +6156,7 @@ Region Document::absoluteRegionForEventTargets(const EventTargetSet* targets)
             targetRegion.unite(Region(enclosingIntRect(rootRelativeBounds)));
     }
 
-    return targetRegion;
+    return RegionFixedPair(targetRegion, insideFixedPosition);
 }
 
 void Document::updateLastHandledUserGestureTimestamp()
index 8a612ad..8fc98bb 100644 (file)
@@ -1122,7 +1122,6 @@ public:
 
     void initDNSPrefetch();
 
-    unsigned wheelEventHandlerCount() const { return m_wheelEventHandlerCount; }
     void didAddWheelEventHandler(Node&);
     void didRemoveWheelEventHandler(Node&);
 
@@ -1135,6 +1134,10 @@ public:
     bool hasTouchEventHandlers() const { return false; }
 #endif
 
+    // Used for testing. Count handlers in the main document, and one per frame which contains handlers.
+    WEBCORE_EXPORT unsigned wheelEventHandlerCount() const;
+    WEBCORE_EXPORT unsigned touchEventHandlerCount() const;
+
     void didAddTouchEventHandler(Node&);
     void didRemoveTouchEventHandler(Node&);
 
@@ -1151,7 +1154,8 @@ public:
 
     const EventTargetSet* wheelEventTargets() const { return m_wheelEventTargets.get(); }
 
-    Region absoluteRegionForEventTargets(const EventTargetSet*);
+    typedef std::pair<Region, bool> RegionFixedPair;
+    RegionFixedPair absoluteRegionForEventTargets(const EventTargetSet*);
 
     LayoutRect absoluteEventHandlerBounds(bool&) override final;
 
@@ -1320,6 +1324,8 @@ private:
 
     void didAssociateFormControlsTimerFired();
 
+    void wheelEventHandlersChanged();
+
     // DOM Cookies caching.
     const String& cachedDOMCookies() const { return m_cachedDOMCookies; }
     void setCachedDOMCookies(const String&);
@@ -1564,7 +1570,6 @@ private:
     bool m_writeRecursionIsTooDeep;
     unsigned m_writeRecursionDepth;
     
-    unsigned m_wheelEventHandlerCount;
 #if ENABLE(TOUCH_EVENTS)
     std::unique_ptr<EventTargetSet> m_touchEventTargets;
 #endif
index f903785..2694267 100644 (file)
@@ -227,7 +227,7 @@ public:
     virtual void needTouchEvents(bool) override { }
 #endif
     
-    virtual void numWheelEventHandlersChanged(unsigned) override { }
+    virtual void wheelEventHandlersChanged(bool) override { }
     
     virtual bool isEmptyChromeClient() const override { return true; }
 
index 99da950..bdc913c 100644 (file)
@@ -396,7 +396,7 @@ public:
     };
     virtual bool shouldRunModalDialogDuringPageDismissal(const DialogType&, const String& dialogMessage, FrameLoader::PageDismissalType) const { UNUSED_PARAM(dialogMessage); return true; }
 
-    virtual void numWheelEventHandlersChanged(unsigned) = 0;
+    virtual void wheelEventHandlersChanged(bool hasHandlers) = 0;
         
     virtual bool isSVGImageChromeClient() const { return false; }
 
index b2621ce..21371dc 100644 (file)
@@ -90,7 +90,7 @@ private:
 
 bool MouseWheelRegionOverlay::updateRegion()
 {
-    std::unique_ptr<Region> region = std::make_unique<Region>(m_frame.document()->absoluteRegionForEventTargets(m_frame.document()->wheelEventTargets()));
+    std::unique_ptr<Region> region = std::make_unique<Region>(m_frame.document()->absoluteRegionForEventTargets(m_frame.document()->wheelEventTargets()).first);
 
     bool regionChanged = !m_region || !(*m_region == *region);
     m_region = WTF::move(region);
index 9ed58aa..6c75fb2 100644 (file)
@@ -135,6 +135,7 @@ void AsyncScrollingCoordinator::frameViewNonFastScrollableRegionChanged(FrameVie
     if (!m_scrollingStateTree->rootStateNode())
         return;
 
+    // FIXME: computeNonFastScrollableRegion lazily.
     m_scrollingStateTree->rootStateNode()->setNonFastScrollableRegion(computeNonFastScrollableRegion(m_page->mainFrame(), IntPoint()));
 }
 
@@ -487,14 +488,6 @@ void AsyncScrollingCoordinator::updateMainFrameScrollLayerPosition()
         scrollLayer->setPosition(-frameView->scrollPosition());
 }
 
-void AsyncScrollingCoordinator::recomputeWheelEventHandlerCountForFrameView(FrameView& frameView)
-{
-    ScrollingStateFrameScrollingNode* node = downcast<ScrollingStateFrameScrollingNode>(m_scrollingStateTree->stateNodeForID(frameView.scrollLayerID()));
-    if (!node)
-        return;
-    node->setWheelEventHandlerCount(computeCurrentWheelEventHandlerCount());
-}
-
 bool AsyncScrollingCoordinator::isRubberBandInProgress() const
 {
     return scrollingTree()->isRubberBandInProgress();
index f3ebaa4..0226c5e 100644 (file)
@@ -96,7 +96,6 @@ private:
     WEBCORE_EXPORT virtual void syncChildPositions(const LayoutRect& viewportRect) override;
     WEBCORE_EXPORT virtual void scrollableAreaScrollbarLayerDidChange(ScrollableArea&, ScrollbarOrientation) override;
 
-    WEBCORE_EXPORT virtual void recomputeWheelEventHandlerCountForFrameView(FrameView&) override;
     WEBCORE_EXPORT virtual void setSynchronousScrollingReasons(SynchronousScrollingReasons) override;
 
     virtual void scheduleTreeStateCommit() = 0;
index 69fdc2b..4b84b70 100644 (file)
@@ -154,34 +154,22 @@ Region ScrollingCoordinator::computeNonFastScrollableRegion(const Frame& frame,
         nonFastScrollableRegion.unite(computeNonFastScrollableRegion(*subframe, offset));
 
     // Include wheel event handler region for the main frame.
-    Region wheelHandlerRegion = frame.document()->absoluteRegionForEventTargets(frame.document()->wheelEventTargets());
-    wheelHandlerRegion.translate(toIntSize(offset));
-    nonFastScrollableRegion.unite(wheelHandlerRegion);
+    Document::RegionFixedPair wheelHandlerRegion = frame.document()->absoluteRegionForEventTargets(frame.document()->wheelEventTargets());
+    bool wheelHandlerInFixedContent = wheelHandlerRegion.second;
+    if (wheelHandlerInFixedContent) {
+        // FIXME: if a fixed element has a wheel event handler, for now just cover the entire document
+        // with the slow-scrolling region. This could be improved.
+        // FIXME: need to handle position:sticky here too.
+        bool inFixed;
+        wheelHandlerRegion.first.unite(enclosingIntRect(frame.document()->absoluteEventHandlerBounds(inFixed)));
+    }
+    wheelHandlerRegion.first.translate(toIntSize(offset));
+    nonFastScrollableRegion.unite(wheelHandlerRegion.first);
 
     return nonFastScrollableRegion;
 #endif
 }
 
-unsigned ScrollingCoordinator::computeCurrentWheelEventHandlerCount()
-{
-    unsigned wheelEventHandlerCount = 0;
-
-    for (Frame* frame = &m_page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
-        if (frame->document())
-            wheelEventHandlerCount += frame->document()->wheelEventHandlerCount();
-    }
-
-    return wheelEventHandlerCount;
-}
-
-void ScrollingCoordinator::frameViewWheelEventHandlerCountChanged(FrameView& frameView)
-{
-    ASSERT(isMainThread());
-    ASSERT(m_page);
-
-    recomputeWheelEventHandlerCountForFrameView(frameView);
-}
-
 void ScrollingCoordinator::frameViewHasSlowRepaintObjectsDidChange(FrameView& frameView)
 {
     ASSERT(isMainThread());
@@ -283,7 +271,6 @@ void ScrollingCoordinator::frameViewRootLayerDidChange(FrameView& frameView)
         return;
 
     frameViewLayoutUpdated(frameView);
-    recomputeWheelEventHandlerCountForFrameView(frameView);
     updateSynchronousScrollingReasons(frameView);
 }
 
index 977f962..5148d5e 100644 (file)
@@ -120,10 +120,6 @@ public:
     // Should be called whenever the given frame view has been laid out.
     virtual void frameViewLayoutUpdated(FrameView&) { }
 
-    // Should be called whenever a wheel event handler is added or removed in the 
-    // frame view's underlying document.
-    void frameViewWheelEventHandlerCountChanged(FrameView&);
-
     // Should be called whenever the slow repaint objects counter changes between zero and one.
     void frameViewHasSlowRepaintObjectsDidChange(FrameView&);
 
@@ -210,7 +206,6 @@ protected:
 
     static GraphicsLayer* scrollLayerForScrollableArea(ScrollableArea&);
 
-    unsigned computeCurrentWheelEventHandlerCount();
     GraphicsLayer* scrollLayerForFrameView(FrameView&);
     GraphicsLayer* counterScrollingLayerForFrameView(FrameView&);
     GraphicsLayer* insetClipLayerForFrameView(FrameView&);
@@ -222,7 +217,6 @@ protected:
     Page* m_page; // FIXME: ideally this would be a reference but it gets nulled on async teardown.
 
 private:
-    virtual void recomputeWheelEventHandlerCountForFrameView(FrameView&) { }
     virtual void setSynchronousScrollingReasons(SynchronousScrollingReasons) { }
 
     virtual bool hasVisibleSlowRepaintViewportConstrainedObjects(const FrameView&) const;
index d80c1be..350c796 100644 (file)
@@ -45,7 +45,6 @@ ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode(ScrollingStat
     , m_horizontalScrollbarPainter(0)
 #endif
     , m_frameScaleFactor(1)
-    , m_wheelEventHandlerCount(0)
     , m_synchronousScrollingReasons(0)
     , m_behaviorForFixed(StickToDocumentBounds)
     , m_headerHeight(0)
@@ -63,7 +62,6 @@ ScrollingStateFrameScrollingNode::ScrollingStateFrameScrollingNode(const Scrolli
 #endif
     , m_nonFastScrollableRegion(stateNode.nonFastScrollableRegion())
     , m_frameScaleFactor(stateNode.frameScaleFactor())
-    , m_wheelEventHandlerCount(stateNode.wheelEventHandlerCount())
     , m_synchronousScrollingReasons(stateNode.synchronousScrollingReasons())
     , m_behaviorForFixed(stateNode.scrollBehaviorForFixedElements())
     , m_headerHeight(stateNode.headerHeight())
@@ -119,15 +117,6 @@ void ScrollingStateFrameScrollingNode::setNonFastScrollableRegion(const Region&
     setPropertyChanged(NonFastScrollableRegion);
 }
 
-void ScrollingStateFrameScrollingNode::setWheelEventHandlerCount(unsigned wheelEventHandlerCount)
-{
-    if (m_wheelEventHandlerCount == wheelEventHandlerCount)
-        return;
-
-    m_wheelEventHandlerCount = wheelEventHandlerCount;
-    setPropertyChanged(WheelEventHandlerCount);
-}
-
 void ScrollingStateFrameScrollingNode::setSynchronousScrollingReasons(SynchronousScrollingReasons reasons)
 {
     if (m_synchronousScrollingReasons == reasons)
index f70870c..b13b438 100644 (file)
@@ -49,7 +49,6 @@ public:
     enum ChangedProperty {
         FrameScaleFactor = NumScrollingStateNodeBits,
         NonFastScrollableRegion,
-        WheelEventHandlerCount,
         ReasonsForSynchronousScrolling,
         ScrolledContentsLayer,
         CounterScrollingLayer,
@@ -70,9 +69,6 @@ public:
     const Region& nonFastScrollableRegion() const { return m_nonFastScrollableRegion; }
     WEBCORE_EXPORT void setNonFastScrollableRegion(const Region&);
 
-    unsigned wheelEventHandlerCount() const { return m_wheelEventHandlerCount; }
-    WEBCORE_EXPORT void setWheelEventHandlerCount(unsigned);
-
     SynchronousScrollingReasons synchronousScrollingReasons() const { return m_synchronousScrollingReasons; }
     WEBCORE_EXPORT void setSynchronousScrollingReasons(SynchronousScrollingReasons);
 
@@ -139,7 +135,6 @@ private:
 
     Region m_nonFastScrollableRegion;
     float m_frameScaleFactor;
-    unsigned m_wheelEventHandlerCount;
     SynchronousScrollingReasons m_synchronousScrollingReasons;
     ScrollBehaviorForFixedElements m_behaviorForFixed;
     int m_headerHeight;
index 5b30df7..c260fe4 100644 (file)
@@ -38,8 +38,7 @@
 namespace WebCore {
 
 ScrollingTree::ScrollingTree()
-    : m_hasWheelEventHandlers(false)
-    , m_rubberBandsAtLeft(true)
+    : m_rubberBandsAtLeft(true)
     , m_rubberBandsAtRight(true)
     , m_rubberBandsAtTop(true)
     , m_rubberBandsAtBottom(true)
@@ -65,9 +64,6 @@ bool ScrollingTree::shouldHandleWheelEventSynchronously(const PlatformWheelEvent
     // This method is invoked by the event handling thread
     MutexLocker lock(m_mutex);
 
-    if (m_hasWheelEventHandlers)
-        return true;
-
     bool shouldSetLatch = wheelEvent.shouldConsiderLatching();
     
     if (hasLatchedNode() && !shouldSetLatch)
@@ -129,15 +125,12 @@ void ScrollingTree::commitNewTreeState(std::unique_ptr<ScrollingStateTree> scrol
     ScrollingStateScrollingNode* rootNode = scrollingStateTree->rootStateNode();
     if (rootNode
         && (rootStateNodeChanged
-            || rootNode->hasChangedProperty(ScrollingStateFrameScrollingNode::WheelEventHandlerCount)
             || rootNode->hasChangedProperty(ScrollingStateFrameScrollingNode::NonFastScrollableRegion)
             || rootNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))) {
         MutexLocker lock(m_mutex);
 
         if (rootStateNodeChanged || rootNode->hasChangedProperty(ScrollingStateNode::ScrollLayer))
             m_mainFrameScrollPosition = FloatPoint();
-        if (rootStateNodeChanged || rootNode->hasChangedProperty(ScrollingStateFrameScrollingNode::WheelEventHandlerCount))
-            m_hasWheelEventHandlers = scrollingStateTree->rootStateNode()->wheelEventHandlerCount();
         if (rootStateNodeChanged || rootNode->hasChangedProperty(ScrollingStateFrameScrollingNode::NonFastScrollableRegion))
             m_nonFastScrollableRegion = scrollingStateTree->rootStateNode()->nonFastScrollableRegion();
     }
index e1208f1..578caf0 100644 (file)
@@ -156,7 +156,6 @@ private:
     Mutex m_mutex;
     Region m_nonFastScrollableRegion;
     FloatPoint m_mainFrameScrollPosition;
-    bool m_hasWheelEventHandlers;
 
     Mutex m_swipeStateMutex;
     bool m_rubberBandsAtLeft;
index ea13927..b4b20db 100644 (file)
@@ -144,8 +144,6 @@ void ScrollingCoordinatorMac::updateTiledScrollingIndicator()
     ScrollingModeIndication indicatorMode;
     if (shouldUpdateScrollLayerPositionSynchronously())
         indicatorMode = SynchronousScrollingBecauseOfStyleIndication;
-    else if (scrollingStateTree()->rootStateNode() && scrollingStateTree()->rootStateNode()->wheelEventHandlerCount())
-        indicatorMode =  SynchronousScrollingBecauseOfEventHandlersIndication;
     else
         indicatorMode = AsyncScrollingIndication;
     
index b7429b8..f90d2be 100644 (file)
@@ -47,8 +47,6 @@
 namespace WebCore {
 
 static void logThreadedScrollingMode(unsigned synchronousScrollingReasons);
-static void logWheelEventHandlerCountChanged(unsigned);
-
 
 PassRefPtr<ScrollingTreeFrameScrollingNode> ScrollingTreeFrameScrollingNodeMac::create(ScrollingTree& scrollingTree, ScrollingNodeID nodeID)
 {
@@ -133,11 +131,6 @@ void ScrollingTreeFrameScrollingNodeMac::updateBeforeChildren(const ScrollingSta
             logThreadedScrollingMode(synchronousScrollingReasons());
     }
 
-    if (scrollingStateNode.hasChangedProperty(ScrollingStateFrameScrollingNode::WheelEventHandlerCount)) {
-        if (scrollingTree().scrollingPerformanceLoggingEnabled())
-            logWheelEventHandlerCountChanged(scrollingStateNode.wheelEventHandlerCount());
-    }
-
 #if ENABLE(CSS_SCROLL_SNAP)
     if (scrollingStateNode.hasChangedProperty(ScrollingStateFrameScrollingNode::HorizontalSnapOffsets))
         m_scrollController.updateScrollSnapPoints(ScrollEventAxis::Horizontal, convertToLayoutUnits(scrollingStateNode.horizontalSnapOffsets()));
@@ -543,11 +536,6 @@ static void logThreadedScrollingMode(unsigned synchronousScrollingReasons)
         WTFLogAlways("SCROLLING: Switching to threaded scrolling mode. Time: %f\n", WTF::monotonicallyIncreasingTime());
 }
 
-void logWheelEventHandlerCountChanged(unsigned count)
-{
-    WTFLogAlways("SCROLLING: Wheel event handler count changed. Time: %f Count: %u\n", WTF::monotonicallyIncreasingTime(), count);
-}
-
 #if ENABLE(CSS_SCROLL_SNAP) && PLATFORM(MAC)
 LayoutUnit ScrollingTreeFrameScrollingNodeMac::scrollOffsetOnAxis(ScrollEventAxis axis) const
 {
index 99401f6..34b6845 100644 (file)
@@ -1254,15 +1254,7 @@ unsigned Internals::touchEventHandlerCount(ExceptionCode& ec)
         return 0;
     }
 
-    auto touchHandlers = document->touchEventTargets();
-    if (!touchHandlers)
-        return 0;
-
-    unsigned count = 0;
-    for (auto& handler : *touchHandlers)
-        count += handler.value;
-
-    return count;
+    return document->touchEventHandlerCount();
 }
 
 // FIXME: Remove the document argument. It is almost always the same as
index dd6ef91..5c335a9 100644 (file)
@@ -1,3 +1,16 @@
+2015-03-31  Simon Fraser  <simon.fraser@apple.com>
+
+        Remove scrolling tree dependency on wheel event handler counts, and use fast scrolling even when there are wheel handlers
+        https://bugs.webkit.org/show_bug.cgi?id=143288
+
+        Reviewed by Beth Dakin.
+
+        Remove the wheel event counting that Document does, and passes into the scrolling tree.
+        The ScrollingTree now just uses the non-fast scrollable region to determine when to
+        fast scroll on pages with wheel event handlers.
+
+        * WebCoreSupport/WebChromeClient.h:
+
 2015-03-31  Timothy Horton  <timothy_horton@apple.com>
 
         TextIndicator for <span> inside an <a> only highlights the <span>, should highlight the whole <a>
index e9546e2..a1e4b88 100644 (file)
@@ -195,7 +195,7 @@ public:
     virtual PassRefPtr<WebCore::PopupMenu> createPopupMenu(WebCore::PopupMenuClient*) const override;
     virtual PassRefPtr<WebCore::SearchPopupMenu> createSearchPopupMenu(WebCore::PopupMenuClient*) const override;
 
-    virtual void numWheelEventHandlersChanged(unsigned) override { }
+    virtual void wheelEventHandlersChanged(bool) override { }
 
 #if ENABLE(SUBTLE_CRYPTO)
     virtual bool wrapCryptoKey(const Vector<uint8_t>&, Vector<uint8_t>&) const override;
index f4d47b4..617649e 100644 (file)
@@ -1,3 +1,33 @@
+2015-03-31  Simon Fraser  <simon.fraser@apple.com>
+
+        Remove scrolling tree dependency on wheel event handler counts, and use fast scrolling even when there are wheel handlers
+        https://bugs.webkit.org/show_bug.cgi?id=143288
+        rdar://problem/16049624
+
+        Reviewed by Beth Dakin.
+
+        Remove the wheel event counting that Document does, and passes into the scrolling tree.
+        The ScrollingTree now just uses the non-fast scrollable region to determine when to
+        fast scroll on pages with wheel event handlers.
+        
+        If a handler includes position:fixed renderers, we just cover the whole document
+        with the slow-scrolling region currently. This could be improved.
+
+        * Shared/Scrolling/RemoteScrollingCoordinatorTransaction.cpp:
+        (ArgumentCoder<ScrollingStateFrameScrollingNode>::encode):
+        (ArgumentCoder<ScrollingStateFrameScrollingNode>::decode):
+        (WebKit::RemoteScrollingTreeTextStream::dump):
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::wheelEventHandlersChanged):
+        (WebKit::WebChromeClient::numWheelEventHandlersChanged): Deleted.
+        * WebProcess/WebCoreSupport/WebChromeClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::WebPage):
+        (WebKit::WebPage::wheelEventHandlersChanged):
+        (WebKit::WebPage::recomputeShortCircuitHorizontalWheelEventsState):
+        (WebKit::WebPage::numWheelEventHandlersChanged): Deleted.
+        * WebProcess/WebPage/WebPage.h:
+
 2015-03-31  Dan Bernstein  <mitz@apple.com>
 
         <rdar://problem/20365675> [iOS] Include Add to Reading List link action only where supported
index b36bfcc..f00cc66 100644 (file)
@@ -143,7 +143,6 @@ void ArgumentCoder<ScrollingStateFrameScrollingNode>::encode(ArgumentEncoder& en
     
     SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::FrameScaleFactor, frameScaleFactor)
     SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::NonFastScrollableRegion, nonFastScrollableRegion)
-    SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::WheelEventHandlerCount, wheelEventHandlerCount)
     SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::ReasonsForSynchronousScrolling, synchronousScrollingReasons)
     SCROLLING_NODE_ENCODE_ENUM(ScrollingStateFrameScrollingNode::BehaviorForFixedElements, scrollBehaviorForFixedElements)
     SCROLLING_NODE_ENCODE(ScrollingStateFrameScrollingNode::HeaderHeight, headerHeight)
@@ -225,7 +224,6 @@ bool ArgumentCoder<ScrollingStateFrameScrollingNode>::decode(ArgumentDecoder& de
 
     SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::FrameScaleFactor, float, setFrameScaleFactor);
     SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::NonFastScrollableRegion, Region, setNonFastScrollableRegion);
-    SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::WheelEventHandlerCount, int, setWheelEventHandlerCount);
     SCROLLING_NODE_DECODE(ScrollingStateFrameScrollingNode::ReasonsForSynchronousScrolling, SynchronousScrollingReasons, setSynchronousScrollingReasons);
     SCROLLING_NODE_DECODE_ENUM(ScrollingStateFrameScrollingNode::BehaviorForFixedElements, ScrollBehaviorForFixedElements, setScrollBehaviorForFixedElements);
 
@@ -620,7 +618,6 @@ void RemoteScrollingTreeTextStream::dump(const ScrollingStateFrameScrollingNode&
         ts.decreaseIndent();
     }
 
-    // FIXME: dump wheelEventHandlerCount
     // FIXME: dump synchronousScrollingReasons
     // FIXME: dump scrollableAreaParameters
     // FIXME: dump scrollBehaviorForFixedElements
index 944fe6d..c7d2d09 100644 (file)
@@ -990,9 +990,9 @@ void WebChromeClient::pageExtendedBackgroundColorDidChange(Color backgroundColor
 #endif
 }
 
-void WebChromeClient::numWheelEventHandlersChanged(unsigned count)
+void WebChromeClient::wheelEventHandlersChanged(bool hasHandlers)
 {
-    m_page->numWheelEventHandlersChanged(count);
+    m_page->wheelEventHandlersChanged(hasHandlers);
 }
 
 String WebChromeClient::plugInStartLabelTitle(const String& mimeType) const
index fff49b3..cef2cec 100644 (file)
@@ -280,7 +280,7 @@ private:
 
     virtual void pageExtendedBackgroundColorDidChange(WebCore::Color) const override;
     
-    virtual void numWheelEventHandlersChanged(unsigned) override;
+    virtual void wheelEventHandlersChanged(bool) override;
 
     virtual String plugInStartLabelTitle(const String& mimeType) const override;
     virtual String plugInStartLabelSubtitle(const String& mimeType) const override;
index 669756f..b5f2c4d 100644 (file)
@@ -304,7 +304,7 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
     , m_cachedMainFrameIsPinnedToTopSide(true)
     , m_cachedMainFrameIsPinnedToBottomSide(true)
     , m_canShortCircuitHorizontalWheelEvents(false)
-    , m_numWheelEventHandlers(0)
+    , m_hasWheelEventHandlers(false)
     , m_cachedPageCount(0)
     , m_autoSizingShouldExpandToViewHeight(false)
 #if ENABLE(CONTEXT_MENUS)
@@ -4055,12 +4055,12 @@ void WebPage::confirmCompositionForTesting(const String& compositionString)
     frame.editor().confirmComposition(compositionString);
 }
 
-void WebPage::numWheelEventHandlersChanged(unsigned numWheelEventHandlers)
+void WebPage::wheelEventHandlersChanged(bool hasHandlers)
 {
-    if (m_numWheelEventHandlers == numWheelEventHandlers)
+    if (m_hasWheelEventHandlers == hasHandlers)
         return;
 
-    m_numWheelEventHandlers = numWheelEventHandlers;
+    m_hasWheelEventHandlers = hasHandlers;
     recomputeShortCircuitHorizontalWheelEventsState();
 }
 
@@ -4103,7 +4103,7 @@ static bool pageContainsAnyHorizontalScrollbars(Frame* mainFrame)
 
 void WebPage::recomputeShortCircuitHorizontalWheelEventsState()
 {
-    bool canShortCircuitHorizontalWheelEvents = !m_numWheelEventHandlers;
+    bool canShortCircuitHorizontalWheelEvents = !m_hasWheelEventHandlers;
 
     if (canShortCircuitHorizontalWheelEvents) {
         // Check if we have any horizontal scroll bars on the page.
index 5795059..1aea803 100644 (file)
@@ -760,7 +760,7 @@ public:
 
     void wheelEvent(const WebWheelEvent&);
 
-    void numWheelEventHandlersChanged(unsigned);
+    void wheelEventHandlersChanged(bool);
     void recomputeShortCircuitHorizontalWheelEventsState();
 
     void updateVisibilityState(bool isInitialState = false);
@@ -1263,7 +1263,7 @@ private:
     bool m_cachedMainFrameIsPinnedToTopSide;
     bool m_cachedMainFrameIsPinnedToBottomSide;
     bool m_canShortCircuitHorizontalWheelEvents;
-    unsigned m_numWheelEventHandlers;
+    bool m_hasWheelEventHandlers;
 
     unsigned m_cachedPageCount;