Move selection invalidation to the invalidation phase

Source/core/rendering/RenderObject.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  *           (C) 2004 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Google Inc. All rights reserved.
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 112 matching lines @@
     void* pointers[5];
 #if ENABLE(ASSERT)
     unsigned m_debugBitfields : 2;
 #if ENABLE(OILPAN)
     unsigned m_oilpanBitfields : 1;
 #endif
 #endif
     unsigned m_bitfields;
     unsigned m_bitfields2;
     LayoutRect rect; // Stores the previous paint invalidation rect.
+    LayoutRect rect2; // Stores the previous selection paint invalidation rect.

[dsinclair, 2014/10/31 14:03:15]

Would something like the following be better?

LayoutRect rect[2]; // Stores the previous paint and selection invalidation
rects.

     LayoutPoint position; // Stores the previous position from the paint invalidation container.
 };
 
 COMPILE_ASSERT(sizeof(RenderObject) == sizeof(SameSizeAsRenderObject), RenderObject_should_stay_small);
 
 bool RenderObject::s_affectsParentBlock = false;
 
 #if !ENABLE(OILPAN)
 void* RenderObject::operator new(size_t sz)
 {
@@ skipping 1096 matching lines @@
     addJsonObjectForRect(value.get(), "newRect", newRect);
     addJsonObjectForPoint(value.get(), "newLocation", newLocation);
     return value;
 }
 
 void RenderObject::invalidateSelectionIfNeeded(const RenderLayerModelObject& paintInvalidationContainer)
 {
     if (!shouldInvalidateSelection())
         return;
 
-    LayoutRect selection = selectionRectForPaintInvalidation(&paintInvalidationContainer);
-    // FIXME: groupedMapping() leaks the squashing abstraction. See RenderBlockSelectionInfo for more details.
+    LayoutRect oldSelectionRect = m_previousSelectionRectForPaintInvalidation;
+    m_previousSelectionRectForPaintInvalidation = selectionRectForPaintInvalidation(&paintInvalidationContainer);
+    // FIXME: groupedMapping() leaks the squashing abstraction.
     if (paintInvalidationContainer.layer()->groupedMapping())
-        RenderLayer::mapRectToPaintBackingCoordinates(&paintInvalidationContainer, selection);
-    invalidatePaintUsingContainer(&paintInvalidationContainer, selection, PaintInvalidationSelection);
+        RenderLayer::mapRectToPaintBackingCoordinates(&paintInvalidationContainer, m_previousSelectionRectForPaintInvalidation);
+
+    if (view()->doingFullPaintInvalidation())
+        return;
+
+    fullyInvalidatePaint(paintInvalidationContainer, PaintInvalidationSelection, oldSelectionRect, m_previousSelectionRectForPaintInvalidation);
 }
 
 PaintInvalidationReason RenderObject::invalidatePaintIfNeeded(const PaintInvalidationState& paintInvalidationState, const RenderLayerModelObject& paintInvalidationContainer)
 {
     RenderView* v = view();
     if (v->document().printing())
         return PaintInvalidationNone; // Don't invalidate paints if we're printing.
 
     const LayoutRect oldBounds = previousPaintInvalidationRect();
     const LayoutPoint oldLocation = previousPositionFromPaintInvalidationBacking();
     const LayoutRect newBounds = boundsRectForPaintInvalidation(&paintInvalidationContainer, &paintInvalidationState);
     const LayoutPoint newLocation = RenderLayer::positionFromPaintInvalidationBacking(this, &paintInvalidationContainer, &paintInvalidationState);
     setPreviousPaintInvalidationRect(newBounds);
     setPreviousPositionFromPaintInvalidationBacking(newLocation);
 
+    // We need to invalidate the selection before checking for whether we are doing a full invalidation.
+    // This is because we need to update the old rect regardless.
+    invalidateSelectionIfNeeded(paintInvalidationContainer);

[dsinclair, 2014/10/31 14:03:16]

We need to update the rect regardless, but we don't have to issue the
invalidation if the PaintInvalidatinoReason is a full invalidation right?

Could we do some magic here to make sure we don't issue extra invalidations (or
is it just not worth the effort?)

[Julien - ping for review, 2014/10/31 22:31:03]

It was easy to do, added it.

+
     PaintInvalidationReason invalidationReason = paintInvalidationReason(paintInvalidationContainer, oldBounds, oldLocation, newBounds, newLocation);
 
     // If we are set to do a full paint invalidation that means the RenderView will issue
     // paint invalidations. We can then skip issuing of paint invalidations for the child
     // renderers as they'll be covered by the RenderView.
     if (view()->doingFullPaintInvalidation())
         return invalidationReason;
 
     TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("blink.invalidation"), "RenderObject::invalidatePaintIfNeeded()",
         "object", this->debugName().ascii(),
         "info", jsonObjectForOldAndNewRects(oldBounds, oldLocation, newBounds, newLocation));
 
-    invalidateSelectionIfNeeded(paintInvalidationContainer);
-
     if (invalidationReason == PaintInvalidationNone)
         return invalidationReason;
 
     if (invalidationReason == PaintInvalidationIncremental) {
         incrementallyInvalidatePaint(paintInvalidationContainer, oldBounds, newBounds, newLocation);
         return invalidationReason;
     }
 
     fullyInvalidatePaint(paintInvalidationContainer, invalidationReason, oldBounds, newBounds);
     return invalidationReason;
@@ skipping 1852 matching lines @@
 {
     if (object1) {
         const blink::RenderObject* root = object1;
         while (root->parent())
             root = root->parent();
         root->showRenderTreeAndMark(object1, "*", object2, "-", 0);
     }
 }
 
 #endif