FrameView now notifies ScrollCoorinator of changes in its scrollable area set

Source/core/frame/FrameView.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
@@ skipping 275 matching lines @@
 
     if (ScrollAnimator* scrollAnimator = existingScrollAnimator())
         scrollAnimator->cancelAnimations();
     cancelProgrammaticScrollAnimation();
 
     detachCustomScrollbars();
     // When the view is no longer associated with a frame, it needs to be removed from the ax object cache
     // 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)?
     //
     // FIXME: Oilpan: is this safe to dispose() if there are FrameView protections on the stack?
     dispose();
 #endif
 }
 
 void FrameView::detachCustomScrollbars()
@@ skipping 133 matching lines @@
 Page* FrameView::page() const
 {
     return frame().page();
 }
 
 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;
 
     ScrollbarMode newHorizontalMode;
     ScrollbarMode newVerticalMode;
 
     scrollbarModes(newHorizontalMode, newVerticalMode);
 
     if (canHaveScrollbars && newVerticalMode == ScrollbarAlwaysOff)
@@ skipping 750 matching lines @@
 
 bool FrameView::contentsInCompositedLayer() const
 {
     RenderView* renderView = this->renderView();
     return renderView && renderView->compositingState() == PaintsIntoOwnBacking;
 }
 
 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);
     }
 }
 
 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);
     }
 }
 
 void FrameView::addViewportConstrainedObject(RenderObject* object)
 {
     if (!m_viewportConstrainedObjects)
         m_viewportConstrainedObjects = adoptPtr(new ViewportConstrainedObjectSet);
 
     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);
     }
 }
 
 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);
     }
 }
 
 LayoutRect FrameView::viewportConstrainedVisibleContentRect() const
 {
     LayoutRect viewportRect = visibleContentRect();
     // Ignore overhang. No-op when not using rubber banding.
     viewportRect.setLocation(clampScrollPosition(scrollPosition()));
     return viewportRect;
 }
@@ skipping 805 matching lines @@
     FontFaceSet::didLayout(*m_frame->document());
 
     updateWidgetPositions();
 
     // Plugins could have torn down the page inside updateWidgetPositions().
     if (!renderView())
         return;
 
     scheduleUpdateWidgetsIfNecessary();
 
-    if (Page* page = m_frame->page()) {
-        if (ScrollingCoordinator* scrollingCoordinator = page->scrollingCoordinator())
-            scrollingCoordinator->notifyLayoutUpdated();
-    }
+    if (scrollingCoordinator())
+        scrollingCoordinator()->notifyLayoutUpdated();
 
     scrollToAnchor();
 
     sendResizeEventIfNeeded();
 }
 
 bool FrameView::wasViewportResized()
 {
     ASSERT(m_frame);
     RenderView* renderView = this->renderView();
@@ skipping 451 matching lines @@
 
     RenderView* view = renderView();
     if (view) {
         TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "UpdateLayerTree", "data", InspectorUpdateLayerTreeEvent::data(m_frame.get()));
         // FIXME(361045): remove InspectorInstrumentation calls once DevTools Timeline migrates to tracing.
         InspectorInstrumentation::willUpdateLayerTree(m_frame.get());
 
         view->compositor()->updateIfNeededRecursive();
 
         if (view->compositor()->inCompositingMode() && m_frame->isLocalRoot())
-            m_frame->page()->scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
+            scrollingCoordinator()->updateAfterCompositingChangeIfNeeded();
 
         updateCompositedSelectionBoundsIfNeeded();
 
         InspectorInstrumentation::didUpdateLayerTree(m_frame.get());
 
         scrollContentsIfNeededRecursive();
 
         invalidateTreeIfNeededRecursive();
 
         ASSERT(!view->hasPendingSelection());
@@ skipping 324 matching lines @@
     if (it != m_resizerAreas->end())
         m_resizerAreas->remove(it);
 }
 
 void FrameView::addScrollableArea(ScrollableArea* scrollableArea)
 {
     ASSERT(scrollableArea);
     if (!m_scrollableAreas)
         m_scrollableAreas = adoptPtr(new ScrollableAreaSet);
     m_scrollableAreas->add(scrollableArea);
+
+    if (scrollingCoordinator())
+        scrollingCoordinator()->scrollAreaSetDidChange();
 }
 
 void FrameView::removeScrollableArea(ScrollableArea* scrollableArea)
 {
     if (!m_scrollableAreas)
         return;
     m_scrollableAreas->remove(scrollableArea);
+
+    if (scrollingCoordinator())
+        scrollingCoordinator()->scrollAreaSetDidChange();
 }
 
 void FrameView::addAnimatingScrollableArea(ScrollableArea* scrollableArea)
 {
     ASSERT(scrollableArea);
     if (!m_animatingScrollableAreas)
         m_animatingScrollableAreas = adoptPtr(new ScrollableAreaSet);
     m_animatingScrollableAreas->add(scrollableArea);
 }
 
@@ skipping 1068 matching lines @@
         return;
 
     ScrollableArea::setScrollOrigin(origin);
 
     // Update if the scroll origin changes, since our position will be different if the content size did not change.
     if (updatePositionAtAll && updatePositionSynchronously)
         updateScrollbars(scrollOffsetDouble());
 }
 
 } // namespace blink