Refactor wheel event handler registration in ScrollingCoordinator

Source/core/page/scrolling/ScrollingCoordinator.cpp

Index: Source/core/page/scrolling/ScrollingCoordinator.cpp
diff --git a/Source/core/page/scrolling/ScrollingCoordinator.cpp b/Source/core/page/scrolling/ScrollingCoordinator.cpp
index 4ce8278a620aa7ff1285f938df99cad0b064bd12..22a3cb1f025a4ca9e1040a16d11093ff7e80f1b8 100644
--- a/Source/core/page/scrolling/ScrollingCoordinator.cpp
+++ b/Source/core/page/scrolling/ScrollingCoordinator.cpp
@@ -652,15 +652,30 @@ void ScrollingCoordinator::willDestroyRenderLayer(RenderLayer* layer)
     m_layersWithTouchRects.remove(layer);
 }
 
-void ScrollingCoordinator::setWheelEventHandlerCount(unsigned count)
+void ScrollingCoordinator::updateHaveWheelEventHandlers()
 {
-    if (WebLayer* scrollLayer = toWebLayer(m_page->mainFrame()->view()->layerForScrolling()))
-        scrollLayer->setHaveWheelEventHandlers(count > 0);
+    if (WebLayer* scrollLayer = toWebLayer(m_page->mainFrame()->view()->layerForScrolling())) {
+        unsigned wheelEventHandlerCount = 0;
+
+        for (LocalFrame* frame = m_page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
+            if (frame->document())

[abarth-chromium, 2014/04/11 18:53:38]

Can frame->document() actually be zero here?  That seems unlikely...

+                wheelEventHandlerCount += WheelController::from(*frame->document())->wheelEventHandlerCount();
+        }
+
+        scrollLayer->setHaveWheelEventHandlers(wheelEventHandlerCount);
+    }
 }
 
-void ScrollingCoordinator::recomputeWheelEventHandlerCountForFrameView(FrameView*)
+void ScrollingCoordinator::updateHaveScrollEventHandlers()
 {
-    setWheelEventHandlerCount(computeCurrentWheelEventHandlerCount());
+    // Currently the compositor only cares whether there are scroll handlers anywhere on the page
+    // instead on a per-layer basis. We therefore only update this information for the root
+    // scrolling layer.
+    if (WebLayer* scrollLayer = toWebLayer(m_page->mainFrame()->view()->layerForScrolling())) {
+        // TODO(skyostil): Hook this up.
+        bool haveHandlers = false;
+        scrollLayer->setHaveScrollEventHandlers(haveHandlers);
+    }
 }
 
 void ScrollingCoordinator::setShouldUpdateScrollLayerPositionOnMainThread(MainThreadScrollingReasons reasons)
@@ -820,24 +835,20 @@ void ScrollingCoordinator::computeTouchEventTargetRects(LayerHitTestRects& rects
     accumulateDocumentTouchEventTargetRects(rects, document);
 }
 
-unsigned ScrollingCoordinator::computeCurrentWheelEventHandlerCount()
+void ScrollingCoordinator::haveWheelEventHandlersChangedForPage()
 {
-    unsigned wheelEventHandlerCount = 0;
-
-    for (LocalFrame* frame = m_page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
-        if (frame->document())
-            wheelEventHandlerCount += WheelController::from(*frame->document())->wheelEventHandlerCount();
-    }
+    ASSERT(isMainThread());
+    ASSERT(m_page);
 
-    return wheelEventHandlerCount;
+    updateHaveWheelEventHandlers();
 }

[abarth-chromium, 2014/04/11 18:53:38]

Why not have the callers of this function just call
updateHaveWheelEventHandlers() directly?  It seems unnecessary to have this
intermediate function.

 
-void ScrollingCoordinator::frameViewWheelEventHandlerCountChanged(FrameView* frameView)
+void ScrollingCoordinator::haveScrollEventHandlersChangedForPage()
 {
     ASSERT(isMainThread());
     ASSERT(m_page);
 
-    recomputeWheelEventHandlerCountForFrameView(frameView);
+    updateHaveScrollEventHandlers();
 }

[abarth-chromium, 2014/04/11 18:53:38]

ditto

 
 void ScrollingCoordinator::frameViewHasSlowRepaintObjectsDidChange(FrameView* frameView)
@@ -889,7 +900,8 @@ void ScrollingCoordinator::frameViewRootLayerDidChange(FrameView* frameView)
         return;
 
     notifyLayoutUpdated();
-    recomputeWheelEventHandlerCountForFrameView(frameView);
+    updateHaveWheelEventHandlers();
+    updateHaveScrollEventHandlers();
 }
 
 #if OS(MACOSX)