Take continuation into consideration when calculating absoluteClippedOverflowRect.

Source/core/rendering/RenderInline.cpp

Index: Source/core/rendering/RenderInline.cpp
diff --git a/Source/core/rendering/RenderInline.cpp b/Source/core/rendering/RenderInline.cpp
index 120a6f50569de2a8edefeec4c7be73f66e4a40a4..53c072903849f099e8e35e6c8e9153c4bf64e26e 100644
--- a/Source/core/rendering/RenderInline.cpp
+++ b/Source/core/rendering/RenderInline.cpp
@@ -1017,6 +1017,29 @@ LayoutRect RenderInline::linesVisualOverflowBoundingBox() const
     return rect;
 }
 
+LayoutRect RenderInline::absoluteClippedOverflowRect() const
+{
+    if (!continuation())
+        return clippedOverflowRectForPaintInvalidation(view());

[pdr., 2014/12/02 21:56:21]

It looks like clippedOverflowRectForPaintInvalidation already has code for
continuations. Does it not work?

[c.shu, 2014/12/02 22:16:31]

The continuation code in clippedOverflowRectForPaintInvalidation only does some
additional calculation for outlineSize.

[pdr., 2014/12/03 22:11:17]

Can you help me understand why we wouldn't want to include outlines here too?
This patch is for hit testing and we can hit test outlines.

Which caller of absoluteClippedOverflowRect() matters for your test? Having a
better change description would make this much easier to review.

[c.shu, 2014/12/03 22:27:27]

It's not that we wouldn't want to include outlines. The continuation-related
code in clippedOverflowRectForPaintInvalidation is for something else. When it
calculates the rect, it doesn't take care the whole continuation flow. It is Ok
for clippedOverflowRectForPaintInvalidation since it will traverse through the
tree anyway. But absoluteClippedOverflowRect() has to do more.
The caller is hasOffscreenRect() in page/SpatialNavigation.cpp.
I will add details to the description.

+
+    FloatRect floatResult;
+    LinesBoundingBoxGeneratorContext context(floatResult);
+
+    RenderInline* endContinuation = inlineElementContinuation();
+    while (endContinuation->inlineElementContinuation()) {

[leviw_travelin_and_unemployed, 2014/12/09 20:39:56]

Nit: no braces needed.

[c.shu, 2014/12/10 02:40:51]

Acknowledged.

+        endContinuation = endContinuation->inlineElementContinuation();
+    }
+    for (RenderBlock* currBlock = containingBlock(); currBlock && currBlock->isAnonymousBlock(); currBlock = toRenderBlock(currBlock->nextSibling())) {
+        for (RenderObject* curr = currBlock->firstChild(); curr; curr = curr->nextSibling()) {
+            LayoutRect rect = curr->clippedOverflowRectForPaintInvalidation(view());

[leviw_travelin_and_unemployed, 2014/12/09 20:39:56]

This is N^2 since we'll walk up to the view for every renderer :(

Could we at least get a comment about that fact and about possible
optimizations? What comes to mind is potentially mapping all these rects into a
common ancestor's address space then mapping the result into the view.

[c.shu, 2014/12/10 02:40:51]

thanks for pointing out this performance issue. I will add the comments. btw, I
thought it walks up to the containinigBlock(), which is an anonymous block, and
then goes through its siblings. Did my code do something wrong? Is it correct to
pass view() for the clippedOverflowRectForPaintInvalidation() function?

+            context(FloatRect(enclosingIntRect(rect)));
+            if (curr == endContinuation)
+                return enclosingIntRect(floatResult);
+        }
+    }
+    return LayoutRect();
+}
+
 LayoutRect RenderInline::clippedOverflowRectForPaintInvalidation(const RenderLayerModelObject* paintInvalidationContainer, const PaintInvalidationState* paintInvalidationState) const
 {
     if ((!firstLineBoxIncludingCulling() && !continuation()) || style()->visibility() != VISIBLE)