Don't call LayoutPart::widgetPositionsUpdated unnecessarily.

third_party/WebKit/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 114 matching lines @@
     , m_isTransparent(false)
     , m_baseBackgroundColor(Color::white)
     , m_mediaType(MediaTypeNames::screen)
     , m_safeToPropagateScrollToParent(true)
     , m_isTrackingPaintInvalidations(false)
     , m_scrollCorner(nullptr)
     , m_inputEventsScaleFactorForEmulation(1)
     , m_layoutSizeFixedToFrameSize(true)
     , m_didScrollTimer(this, &FrameView::didScrollTimerFired)
     , m_topControlsViewportAdjustment(0)
-    , m_needsUpdateWidgetPositions(false)
+    , m_needsUpdateWidgetGeometries(false)
     , m_needsUpdateViewportIntersection(true)
     , m_needsUpdateViewportIntersectionInSubtree(true)
 #if ENABLE(ASSERT)
     , m_hasBeenDisposed(false)
 #endif
     , m_horizontalScrollbarMode(ScrollbarAuto)
     , m_verticalScrollbarMode(ScrollbarAuto)
     , m_horizontalScrollbarLock(false)
     , m_verticalScrollbarLock(false)
     , m_scrollbarsAvoidingResizer(0)
@@ skipping 990 matching lines @@
 void FrameView::addPart(LayoutPart* object)
 {
     m_parts.add(object);
 }
 
 void FrameView::removePart(LayoutPart* object)
 {
     m_parts.remove(object);
 }
 
-void FrameView::updateWidgetPositions()
+void FrameView::updateWidgetGeometries()
 {
+    if (!layoutView())
+        return;
+
     Vector<RefPtr<LayoutPart>> parts;
     copyToVector(m_parts, parts);
 
     // Script or plugins could detach the frame so abort processing if that happens.
+    Vector<bool> wasUpdated;
+    wasUpdated.resize(parts.size());

[wkorman, 2015/12/12 06:54:55]

Can we initialize it with the right size from the get-go with:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

 
-    for (size_t i = 0; i < parts.size() && layoutView(); ++i)
-        parts[i]->updateWidgetPosition();
+    for (size_t i = 0; i < parts.size(); ++i)
+        wasUpdated[i] = parts[i]->updateWidgetGeometry();
 
-    for (size_t i = 0; i < parts.size() && layoutView(); ++i)
-        parts[i]->widgetPositionsUpdated();
+    for (size_t i = 0; i < parts.size(); ++i) {
+        if (wasUpdated[i])

[chrishtr, 2015/12/12 05:08:54]

This is the money line in this patch. The rest is renames.

+            parts[i]->widgetGeometriesUpdated();

[wkorman, 2015/12/12 06:54:55]

Re: the comment at line 1154, just double-checking that this can't result in
losing our layoutView(), thus necessitating that we keep that check in the for
loop?

+    }
 }
 
 void FrameView::addPartToUpdate(LayoutEmbeddedObject& object)
 {
     ASSERT(isInPerformLayout());
     // Tell the DOM element that it needs a widget update.
     Node* node = object.node();
     ASSERT(node);
     if (isHTMLObjectElement(*node) || isHTMLEmbedElement(*node))
         toHTMLPlugInElement(node)->setNeedsWidgetUpdate(true);
@@ skipping 394 matching lines @@
     // Nothing to do after scrolling if there are no fixed position elements.
     if (!hasViewportConstrainedObjects())
         return;
 
     RefPtrWillBeRawPtr<FrameView> protect(this);
 
     // If there fixed position elements, scrolling may cause compositing layers to change.
     // Update widget and layer positions after scrolling, but only if we're not inside of
     // layout.
     if (!m_nestedLayoutCount) {
-        updateWidgetPositions();
+        updateWidgetGeometries();
         if (LayoutView* layoutView = this->layoutView())
             layoutView->layer()->setNeedsCompositingInputsUpdate();
     }
 }
 
 bool FrameView::computeCompositedSelection(LocalFrame& frame, CompositedSelection& selection)
 {
     const VisibleSelection& visibleSelection = frame.selection().selection();
     if (!visibleSelection.isCaretOrRange())
         return false;
@@ skipping 312 matching lines @@
         // but FrameView holds a manual ref, so it won't have been deleted.
         if (!element)
             continue;
 
         // No need to update if it's already crashed or known to be missing.
         if (object.showsUnavailablePluginIndicator())
             continue;
 
         if (element->needsWidgetUpdate())
             element->updateWidget();
-        object.updateWidgetPosition();
+        object.updateWidgetGeometry();
 
         // Prevent plugins from causing infinite updates of themselves.
         // FIXME: Do we really need to prevent this?
         m_partUpdateSet.remove(&object);
     }
 
     return m_partUpdateSet.isEmpty();
 }
 
 void FrameView::updateWidgetsTimerFired(Timer<FrameView>*)
@@ skipping 41 matching lines @@
 
     ASSERT(m_frame->document());
 
     FontFaceSet::didLayout(*m_frame->document());
     // Cursor update scheduling is done by the local root, which is the main frame if there
     // are no RemoteFrame ancestors in the frame tree. Use of localFrameRoot() is
     // discouraged but will change when cursor update scheduling is moved from EventHandler
     // to PageEventHandler.
     frame().localFrameRoot()->eventHandler().scheduleCursorUpdate();
 
-    updateWidgetPositions();
+    updateWidgetGeometries();
 
-    // Plugins could have torn down the page inside updateWidgetPositions().
+    // Plugins could have torn down the page inside updateWidgetGeometries().
     if (!layoutView())
         return;
 
     scheduleUpdateWidgetsIfNecessary();
 
     if (ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator())
         scrollingCoordinator->notifyLayoutUpdated();
 
     scrollToAnchor();
 
@@ skipping 350 matching lines @@
 bool FrameView::isPainting() const
 {
     return m_isPainting;
 }
 
 void FrameView::setNodeToDraw(Node* node)
 {
     m_nodeToDraw = node;
 }
 
-void FrameView::updateWidgetPositionsIfNeeded()
+void FrameView::updateWidgetGeometriesIfNeeded()
 {
-    if (!m_needsUpdateWidgetPositions)
+    if (!m_needsUpdateWidgetGeometries)
         return;
 
-    m_needsUpdateWidgetPositions = false;
+    m_needsUpdateWidgetGeometries = false;
 
-    updateWidgetPositions();
+    updateWidgetGeometries();
 }
 
 void FrameView::updateAllLifecyclePhases()
 {
     frame().localFrameRoot()->view()->updateLifecyclePhasesInternal(AllPhases);
 }
 
 // TODO(chrishtr): add a scrolling update lifecycle phase.
 void FrameView::updateLifecycleToCompositingCleanPlusScrolling()
 {
@@ skipping 200 matching lines @@
             layout();
     }
 
     // These asserts ensure that parent frames are clean, when child frames finished updating layout and style.
     ASSERT(!needsLayout());
     ASSERT(!m_frame->document()->hasSVGFilterElementsRequiringLayerUpdate());
 #if ENABLE(ASSERT)
     m_frame->document()->layoutView()->assertLaidOut();
 #endif
 
-    updateWidgetPositionsIfNeeded();
+    updateWidgetGeometriesIfNeeded();
 
     if (lifecycle().state() < DocumentLifecycle::LayoutClean)
         lifecycle().advanceTo(DocumentLifecycle::LayoutClean);
 
     // Ensure that we become visually non-empty eventually.
     // TODO(esprehn): This should check isRenderingReady() instead.
     if (frame().document()->hasFinishedParsing() && frame().loader().stateMachine()->committedFirstRealDocumentLoad())
         m_isVisuallyNonEmpty = true;
 }
 
@@ skipping 1435 matching lines @@
     return m_hiddenForThrottling && m_crossOriginForThrottling;
 }
 
 LayoutBox& FrameView::boxForScrollControlPaintInvalidation() const
 {
     ASSERT(layoutView());
     return *layoutView();
 }
 
 } // namespace blink