Include the whole outline into visual overflow

Source/core/layout/LayoutBlockFlowLine.cpp

Index: Source/core/layout/LayoutBlockFlowLine.cpp
diff --git a/Source/core/layout/LayoutBlockFlowLine.cpp b/Source/core/layout/LayoutBlockFlowLine.cpp
index ec91c14250785e1970d3db3c59bddc7b833fa4aa..80c56cef31e30ac4cf8a4032efc55ee696dadcd6 100644
--- a/Source/core/layout/LayoutBlockFlowLine.cpp
+++ b/Source/core/layout/LayoutBlockFlowLine.cpp
@@ -52,6 +52,9 @@ namespace blink {
 
 using namespace WTF::Unicode;
 
+typedef WTF::HashMap<LayoutBlockFlow*, HashSet<LayoutInline*>> InlinesWithOutlineAndContinuationMap;
+static InlinesWithOutlineAndContinuationMap* gInlinesWithOutlineAndContinuationMap = nullptr;
+
 static inline InlineBox* createInlineBoxForLayoutObject(LayoutObject* obj, bool isRootLineBox, bool isOnlyRun = false)
 {
     // Callers should handle text themselves.
@@ -1489,6 +1492,19 @@ void LayoutBlockFlow::computeInlinePreferredLogicalWidths(LayoutUnit& minLogical
     maxLogicalWidth = std::max(maxLogicalWidth, inlineMax);
 }
 
+void LayoutBlockFlow::removeInlinesWithOutlineAndContinuation()
+{
+    if (gInlinesWithOutlineAndContinuationMap)
+        gInlinesWithOutlineAndContinuationMap->remove(this);
+}
+
+void LayoutBlockFlow::addInlineWithOutlineAndContinuation(LayoutInline* layoutInline)
+{
+    if (!gInlinesWithOutlineAndContinuationMap)
+        gInlinesWithOutlineAndContinuationMap = new InlinesWithOutlineAndContinuationMap;
+    gInlinesWithOutlineAndContinuationMap->add(this, HashSet<LayoutInline*>()).storedValue->value.add(layoutInline);
+}
+
 void LayoutBlockFlow::layoutInlineChildren(bool relayoutChildren, LayoutUnit& paintInvalidationLogicalTop, LayoutUnit& paintInvalidationLogicalBottom, LayoutUnit afterEdge)
 {
     LayoutFlowThread* flowThread = flowThreadContainingBlock();
@@ -1522,6 +1538,8 @@ void LayoutBlockFlow::layoutInlineChildren(bool relayoutChildren, LayoutUnit& pa
     if (hasTextOverflow)
         deleteEllipsisLineBoxes();
 
+    removeInlinesWithOutlineAndContinuation();

[leviw_travelin_and_unemployed, 2015/08/10 20:13:14]

It's a bit unfortunate that we incur a hash lookup for every BlockFlow layout if
we have a single inline with outline and continuation. It'd be nice if we had
data for how common this really ends up being (% of pages, and number of inlines
per page) to make a more informed decision about the memory tradeoff. I'm
generally surprised by the prevalence of continuations on the web, but I don't
have a feel for how prevalent this combo is.

[Xianzhu, 2015/08/11 17:57:50]

Ran top-10k web sites, 7986 of them succeeded, and none of them contains inline
with outline and continuation.

I think the reasons might be:
- Outlines of inlines are very broken at least in WebKit/blink, web developers
are likely to avoid them when they see the breakage;
- The feature might be also implemented differently on different browsers;
- The static test doesn't cover focus ring cases.

Because there might be more focus ring cases, I think we still should implement
the feature correctly. Because normally there is at most one focus ring in a
document, the map may normally have only one entry or empty/null in most cases,
and lookup of such a map should be super fast.

In addition, the map is equivalent to the existing continuationOutlineTable() in
LayoutBlock which will be removed in the next CL. We can't just reuse that map
because continuationOutlineTable() is populated and used during painting only
which conflicts with the usage of this CL (during layout). So eventually this CL
doesn't add a new map, but fixes the tricky relationship between layout and
paint.

+
     if (firstChild()) {
         // In full layout mode, clear the line boxes of children upfront. Otherwise,
         // siblings can run into stale root lineboxes during layout. Then layout
@@ -1566,6 +1584,9 @@ void LayoutBlockFlow::layoutInlineChildren(bool relayoutChildren, LayoutUnit& pa
             }
             if (!o->isText() || !toLayoutText(o)->isAllCollapsibleWhitespace())
                 lastChild = o;
+
+            if (o->isLayoutInline() && o->styleRef().hasOutline() && !o->isElementContinuation() && toLayoutInline(o)->continuation())
+                addInlineWithOutlineAndContinuation(toLayoutInline(o));
         }
         // If there is a trailing float on the line that will possibly occur after a natural line break
         // then dirty its adjacent lineboxes to ensure it gets placed.
@@ -1863,6 +1884,23 @@ void LayoutBlockFlow::addOverflowFromInlineChildren()
         LayoutRect visualOverflow = curr->visualOverflowRect(curr->lineTop(), curr->lineBottom());
         addContentsVisualOverflow(visualOverflow);
     }
+
+    if (!gInlinesWithOutlineAndContinuationMap)
+        return;
+    InlinesWithOutlineAndContinuationMap::const_iterator it = gInlinesWithOutlineAndContinuationMap->find(this);
+    if (it == gInlinesWithOutlineAndContinuationMap->end() || it->value.isEmpty())
+        return;
+
+    // Add outline rects of continuations of descendant inlines into visual overflow of this block.
+    LayoutRect outlineBoundsOfAllContinuations;
+    for (LayoutInline* flow : it->value) {
+        Vector<LayoutRect> outlineRects;
+        flow->addOutlineRectsForContinuations(outlineRects, LayoutPoint());

[chrishtr, 2015/08/10 23:45:33]

Is this cache really so important for performance? I suggest you get rid of it
and use the code from 1588 inside of a loop over all children.

[Xianzhu, 2015/08/11 17:57:50]

They are in different document stages. The code from 1588 is during layout, here
is during overflow recalc. We may have overflow recalc without layout, so we
need cache the result from earlier layout.

[chrishtr, 2015/08/13 20:04:13]

You're saying that the layout data necessary to loop over children adn check
their styles may not be available or up-to-date here?

All I was saying about 1588 is to copy the code from there.

[Xianzhu, 2015/08/13 20:26:00]

I see. That would cause we always traverse the whole subtree during overflow
update even if there is no interesting inline descendant. However, I think I can
set a bit to indicate whether there is such descendant during layout, and
traverse here if the bit is set, to avoid the map. Will upload a new patch soon.

+        LayoutRect outlineBounds = unionRect(outlineRects);
+        outlineBounds.inflate(flow->styleRef().outlineOutsetExtent());
+        outlineBoundsOfAllContinuations.unite(outlineBounds);
+    }
+    addContentsVisualOverflow(outlineBoundsOfAllContinuations);
 }
 
 void LayoutBlockFlow::deleteEllipsisLineBoxes()