FrameView now notifies ScrollCoorinator of changes in its scrollable area set

Source/core/frame/FrameView.cpp

Index: Source/core/frame/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index 4e0a8b6c27a8aef1fca3ec27b151dace782b7ec1..b6a8b3fb011fe161a1628db6200713e001878426 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -293,10 +293,8 @@ void FrameView::prepareForDetach()
     // right now, otherwise it won't be able to reach the topDocument()'s axObject cache later.
     removeFromAXObjectCache();
 
-    if (m_frame->page()) {
-        if (ScrollingCoordinator* scrollingCoordinator = m_frame->page()->scrollingCoordinator())
-            scrollingCoordinator->willDestroyScrollableArea(this);
-    }
+    if (scrollingCoordinator())

[Rick Byers, 2015/01/07 17:20:16]

The preferred style in blink is generally something like this:

if (ScrollingCoordinator* scrollingCoordinator = scrollingCoordinator())
  scrollingCoordinator->willDestroyScrollableArea(this);

The extra overhead of asking for it again may be trivial, and it may be
impossible to change value from the first call to the second, but in general
it's hard to know that from the call site.

[majidvp, 2015/01/07 19:52:28]

I will update to match the convention.

[majidvp, 2015/01/12 16:28:27]

Done.

+        scrollingCoordinator()->willDestroyScrollableArea(this);
 
 #if ENABLE(OILPAN)
     // FIXME: once/if dust settles, do this always (non-Oilpan)?
@@ -450,6 +448,12 @@ RenderView* FrameView::renderView() const
     return frame().contentRenderer();
 }
 
+ScrollingCoordinator* FrameView::scrollingCoordinator()
+{
+    Page* p = page();
+    return p ? p->scrollingCoordinator() : nullptr;
+}
+
 void FrameView::setCanHaveScrollbars(bool canHaveScrollbars)
 {
     m_canHaveScrollbars = canHaveScrollbars;
@@ -1220,10 +1224,8 @@ bool FrameView::contentsInCompositedLayer() const
 void FrameView::addSlowRepaintObject()
 {
     if (!m_slowRepaintObjectCount++) {
-        if (Page* page = m_frame->page()) {
-            if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-                scrollingCoordinator->frameViewHasSlowRepaintObjectsDidChange(this);
-        }
+        if (scrollingCoordinator())
+            scrollingCoordinator()->frameViewHasSlowRepaintObjectsDidChange(this);
     }
 }
 
@@ -1232,10 +1234,8 @@ void FrameView::removeSlowRepaintObject()
     ASSERT(m_slowRepaintObjectCount > 0);
     m_slowRepaintObjectCount--;
     if (!m_slowRepaintObjectCount) {
-        if (Page* page = m_frame->page()) {
-            if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-                scrollingCoordinator->frameViewHasSlowRepaintObjectsDidChange(this);
-        }
+        if (scrollingCoordinator())
+            scrollingCoordinator()->frameViewHasSlowRepaintObjectsDidChange(this);
     }
 }
 
@@ -1247,10 +1247,8 @@ void FrameView::addViewportConstrainedObject(RenderObject* object)
     if (!m_viewportConstrainedObjects->contains(object)) {
         m_viewportConstrainedObjects->add(object);
 
-        if (Page* page = m_frame->page()) {
-            if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-                scrollingCoordinator->frameViewFixedObjectsDidChange(this);
-        }
+        if (scrollingCoordinator())
+            scrollingCoordinator()->frameViewFixedObjectsDidChange(this);
     }
 }
 
@@ -1259,10 +1257,8 @@ void FrameView::removeViewportConstrainedObject(RenderObject* object)
     if (m_viewportConstrainedObjects && m_viewportConstrainedObjects->contains(object)) {
         m_viewportConstrainedObjects->remove(object);
 
-        if (Page* page = m_frame->page()) {
-            if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-                scrollingCoordinator->frameViewFixedObjectsDidChange(this);
-        }
+        if (scrollingCoordinator())
+            scrollingCoordinator()->frameViewFixedObjectsDidChange(this);
     }
 }
 
@@ -2088,10 +2084,8 @@ void FrameView::performPostLayoutTasks()
 
     scheduleUpdateWidgetsIfNecessary();
 
-    if (Page* page = m_frame->page()) {
-        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-            scrollingCoordinator->notifyLayoutUpdated();
-    }
+    if (scrollingCoordinator())
+        scrollingCoordinator()->notifyLayoutUpdated();
 
     scrollToAnchor();
 
@@ -2563,7 +2557,7 @@ void FrameView::updateLayoutAndStyleForPainting()
         view->compositor()->updateIfNeededRecursive();
 
         if (view->compositor()->inCompositingMode() && m_frame->isLocalRoot())
-            m_frame->page()->scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
+            scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
 
         updateCompositedSelectionBoundsIfNeeded();
 
@@ -2908,6 +2902,9 @@ void FrameView::addScrollableArea(ScrollableArea* scrollableArea)
     if (!m_scrollableAreas)
         m_scrollableAreas = adoptPtr(new ScrollableAreaSet);
     m_scrollableAreas->add(scrollableArea);
+
+    if (scrollingCoordinator())
+        scrollingCoordinator()->scrollAreaSetDidChange();
 }
 
 void FrameView::removeScrollableArea(ScrollableArea* scrollableArea)
@@ -2915,6 +2912,9 @@ void FrameView::removeScrollableArea(ScrollableArea* scrollableArea)
     if (!m_scrollableAreas)
         return;
     m_scrollableAreas->remove(scrollableArea);
+
+    if (scrollingCoordinator())
+        scrollingCoordinator()->scrollAreaSetDidChange();
 }
 
 void FrameView::addAnimatingScrollableArea(ScrollableArea* scrollableArea)