[Regression] Fix Draggable is float:left and container is overflow:hidden

Source/core/rendering/InlineFlowBox.cpp

 /*
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 1124 matching lines @@
         } else {
             // Paint our background, border and box-shadow.
             paintBoxDecorations(paintInfo, paintOffset);
         }
     }
 
     if (paintInfo.phase == PaintPhaseMask)
         return;
 
     PaintPhase paintPhase = paintInfo.phase == PaintPhaseChildOutlines ? PaintPhaseOutline : paintInfo.phase;
-    PaintInfo childInfo(paintInfo);
-    childInfo.phase = paintPhase;
-    childInfo.updatePaintingRootForChildren(&renderer());
 
     // Paint our children.
     if (paintPhase != PaintPhaseSelfOutline) {
+        PaintInfo childInfo(paintInfo);
+        childInfo.phase = paintPhase;
+        childInfo.updatePaintingRootForChildren(&renderer());

[Julien - ping for review, 2014/05/22 09:26:03]

You're setting childInfo.paintingRoot to a non-NULL renderer here so it seems
like |paintingRootCleared| will never be true.

[spartha, 2014/05/22 12:24:15]

updatePaintingRootForChildren does not set the paintingRoot. if the passed
renderer is same as the paintingRoot, it is cleared, otherwise untouched. So,
paintingRootCleared will be true in cases where renderer() is same as the
paintingRoot or if paintingRoot is already NULL.

[Julien - ping for review, 2014/06/05 22:01:35]

OK, this function is confusingly named for sure :-/

+        bool paintingRootCleared = !childInfo.paintingRoot;

[esprehn, 2014/05/20 05:56:15]

Why do you only ever clear the first one? That doesnt' make sense.

[spartha, 2014/05/20 09:24:10]

I am looking to paint just the InlineBox that holds the paintingRoot and not the
rest. If the paintingRoot is not set, then the chidren are painted anyway.
Otherwise we check if the any of the lineboxes have the paintingRoot as its
child and paint it.

+
         for (InlineBox* curr = firstChild(); curr; curr = curr->nextOnLine()) {
-            if (curr->renderer().isText() || !curr->boxModelObject()->hasSelfPaintingLayer())
-                curr->paint(childInfo, paintOffset, lineTop, lineBottom);
+            if (curr->renderer().isText() || !curr->boxModelObject()->hasSelfPaintingLayer()) {

[esprehn, 2014/05/20 05:56:15]

This needs a better test, you should be able to test this without dragging.

[spartha, 2014/05/20 09:24:10]

I agree, that this mode of testing is not effective. AFIK, drag feature is the
only one which sets the paintingRoot to draw an element sub-tree.

+                // crbug.com/101204. Clear the paintRoot to allow the paint to go through if the
+                // paintingRoot is a descendant of the InlineBox.
+                if (!paintingRootCleared && childInfo.paintingRoot->isDescendantOf(&curr->renderer())) {

[Julien - ping for review, 2014/05/22 09:26:03]

That potentially makes this loop go to the root all the time. It seems like a
bad trade-off for a rare case.

[spartha, 2014/05/22 12:24:15]

There are sufficient checks in place not to affect normal paint operations.
Firstly, paintingRoot is set only for the drag & drop feature (Correct me if I
am wrong). Secondly, RenderLineBoxList::paint filters off calls to
InflowBox::paint by checking against the dirtyRect. Would this still be a big
tradeoff?

+                    PaintInfo clearPaintRoot(childInfo);
+                    clearPaintRoot.paintingRoot = 0;
+                    curr->paint(clearPaintRoot, paintOffset, lineTop, lineBottom);
+                    paintingRootCleared = true;

[esprehn, 2014/05/20 05:56:15]

Nothing reads this, why assign it?

[spartha, 2014/05/20 09:24:10]

paintingRootCleared is being evaluated @1157. I added the check to avoid
additional calls isDescendantOf().

+                } else {
+                    curr->paint(childInfo, paintOffset, lineTop, lineBottom);
+                }
+            }
         }
     }
 }
 
 void InlineFlowBox::paintFillLayers(const PaintInfo& paintInfo, const Color& c, const FillLayer* fillLayer, const LayoutRect& rect, CompositeOperator op)
 {
     if (!fillLayer)
         return;
     paintFillLayers(paintInfo, c, fillLayer->next(), rect, op);
     paintFillLayer(paintInfo, c, fillLayer, rect, op);
@@ skipping 492 matching lines @@
         ASSERT(child->prevOnLine() == prev);
         prev = child;
     }
     ASSERT(prev == m_lastChild);
 #endif
 }
 
 #endif
 
 } // namespace WebCore