Refactor wheel event handler registration in ScrollingCoordinator

Source/core/page/scrolling/ScrollingCoordinator.cpp

 /*
  * Copyright (C) 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 634 matching lines @@
         clipParentWebLayer = toWebLayer(parent->compositedLayerMapping()->parentForSublayers());
 
     child->setClipParent(clipParentWebLayer);
 }
 
 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)
 {
     if (WebLayer* scrollLayer = toWebLayer(m_page->mainFrame()->view()->layerForScrolling())) {
         m_lastMainThreadScrollingReasons = reasons;
         scrollLayer->setShouldScrollOnMainThread(reasons);
     }
 }
 
@@ skipping 139 matching lines @@
     TRACE_EVENT0("input", "ScrollingCoordinator::computeTouchEventTargetRects");
     ASSERT(touchHitTestingEnabled());
 
     Document* document = m_page->mainFrame()->document();
     if (!document || !document->view())
         return;
 
     accumulateDocumentTouchEventTargetRects(rects, document);
 }
 
-unsigned ScrollingCoordinator::computeCurrentWheelEventHandlerCount()
-{
-    unsigned wheelEventHandlerCount = 0;
-
-    for (LocalFrame* frame = m_page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
-        if (frame->document())
-            wheelEventHandlerCount += WheelController::from(*frame->document())->wheelEventHandlerCount();
-    }
-
-    return wheelEventHandlerCount;
-}
-
-void ScrollingCoordinator::frameViewWheelEventHandlerCountChanged(FrameView* frameView)
+void ScrollingCoordinator::haveWheelEventHandlersChangedForPage()
 {
     ASSERT(isMainThread());
     ASSERT(m_page);
 
-    recomputeWheelEventHandlerCountForFrameView(frameView);
+    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::haveScrollEventHandlersChangedForPage()
+{
+    ASSERT(isMainThread());
+    ASSERT(m_page);
+
+    updateHaveScrollEventHandlers();
 }

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

ditto

 
 void ScrollingCoordinator::frameViewHasSlowRepaintObjectsDidChange(FrameView* frameView)
 {
     ASSERT(isMainThread());
     ASSERT(m_page);
 
     if (!coordinatesScrollingForFrameView(frameView))
         return;
 
     m_shouldScrollOnMainThreadDirty = true;
@@ skipping 30 matching lines @@
 
 void ScrollingCoordinator::frameViewRootLayerDidChange(FrameView* frameView)
 {
     ASSERT(isMainThread());
     ASSERT(m_page);
 
     if (!coordinatesScrollingForFrameView(frameView))
         return;
 
     notifyLayoutUpdated();
-    recomputeWheelEventHandlerCountForFrameView(frameView);
+    updateHaveWheelEventHandlers();
+    updateHaveScrollEventHandlers();
 }
 
 #if OS(MACOSX)
 void ScrollingCoordinator::handleWheelEventPhase(PlatformWheelEventPhase phase)
 {
     ASSERT(isMainThread());
 
     if (!m_page)
         return;
 
@@ skipping 75 matching lines @@
     bool frameIsScrollable = frameView && frameView->isScrollable();
     if (frameIsScrollable != m_wasFrameScrollable)
         return true;
 
     if (WebLayer* scrollLayer = frameView ? toWebLayer(frameView->layerForScrolling()) : 0)
         return blink::WebSize(frameView->contentsSize()) != scrollLayer->bounds();
     return false;
 }
 
 } // namespace WebCore