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

Source/core/rendering/InlineFlowBox.cpp

Index: Source/core/rendering/InlineFlowBox.cpp
diff --git a/Source/core/rendering/InlineFlowBox.cpp b/Source/core/rendering/InlineFlowBox.cpp
index 5a263d9ab993ecbce0e9aede687c746789aa5438..f93faca863ca56d0f38cc2b35841c640df37ce7c 100644
--- a/Source/core/rendering/InlineFlowBox.cpp
+++ b/Source/core/rendering/InlineFlowBox.cpp
@@ -1142,15 +1142,27 @@ void InlineFlowBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
         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);
+                }
+            }
         }
     }
 }