Title: [207219] trunk/Source/WebCore
Revision
207219
Author
za...@apple.com
Date
2016-10-12 09:45:55 -0700 (Wed, 12 Oct 2016)

Log Message

Refactor LineLayoutState's float box handling.
https://bugs.webkit.org/show_bug.cgi?id=163286

Reviewed by David Hyatt.

We keep track of float boxes both per line (RootInlineBox::m_floats) and
per flow block (LineLayoutState::m_floats) during layout.
As we lay out the lines and iterate through RootInlineBox::m_floats, we
increment LineLayoutState::m_floatIndex. This LineLayoutState::m_floatIndex is
later used to find the matching float box in the per-block-flow float list.
This logic works fine as long as the lists and the index manipulation are tightly coded.
However due to the complexity of the line/float layout code, this is no longer the case.

This patch makes float box handling more secure by changing this index based setup
to a list iterator. It helps to eliminate potential vector overflow issues.

LineLayoutState::FloatList (new class) keeps track of all the floats for the block flow.
It groups the float box related functions/members and provides an iterator interface to ensure safer
syncing between this and the line based floats.

No change in functionality.

* rendering/RenderBlockFlow.h:
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlockFlow::appendFloatingObjectToLastLine):
(WebCore::repaintDirtyFloats):
(WebCore::RenderBlockFlow::layoutRunsAndFloats):
(WebCore::RenderBlockFlow::layoutRunsAndFloatsInRange):
(WebCore::RenderBlockFlow::linkToEndLineIfNeeded):
(WebCore::RenderBlockFlow::layoutLineBoxes):
(WebCore::RenderBlockFlow::checkFloatInCleanLine):
(WebCore::RenderBlockFlow::determineStartPosition):
(WebCore::RenderBlockFlow::determineEndPosition):
(WebCore::RenderBlockFlow::repaintDirtyFloats): Deleted.
(WebCore::RenderBlockFlow::checkFloatsInCleanLine): Deleted.
* rendering/line/LineLayoutState.h:
(WebCore::FloatWithRect::create):
(WebCore::FloatWithRect::renderer):
(WebCore::FloatWithRect::rect):
(WebCore::FloatWithRect::everHadLayout):
(WebCore::FloatWithRect::adjustRect):
(WebCore::FloatWithRect::FloatWithRect):
(WebCore::LineLayoutState::FloatList::append):
(WebCore::LineLayoutState::FloatList::setLastFloat):
(WebCore::LineLayoutState::FloatList::lastFloat):
(WebCore::LineLayoutState::FloatList::setLastCleanFloat):
(WebCore::LineLayoutState::FloatList::lastCleanFloat):
(WebCore::LineLayoutState::FloatList::floatWithRect):
(WebCore::LineLayoutState::FloatList::begin):
(WebCore::LineLayoutState::FloatList::end):
(WebCore::LineLayoutState::FloatList::find):
(WebCore::LineLayoutState::FloatList::isEmpty):
(WebCore::LineLayoutState::LineLayoutState):
(WebCore::LineLayoutState::floatList):
(WebCore::LineLayoutState::lastFloat): Deleted.
(WebCore::LineLayoutState::setLastFloat): Deleted.
(WebCore::LineLayoutState::floats): Deleted.
(WebCore::LineLayoutState::floatIndex): Deleted.
(WebCore::LineLayoutState::setFloatIndex): Deleted.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207218 => 207219)


--- trunk/Source/WebCore/ChangeLog	2016-10-12 16:36:11 UTC (rev 207218)
+++ trunk/Source/WebCore/ChangeLog	2016-10-12 16:45:55 UTC (rev 207219)
@@ -1,3 +1,65 @@
+2016-10-12  Zalan Bujtas  <za...@apple.com>
+
+        Refactor LineLayoutState's float box handling.
+        https://bugs.webkit.org/show_bug.cgi?id=163286
+
+        Reviewed by David Hyatt.
+        
+        We keep track of float boxes both per line (RootInlineBox::m_floats) and
+        per flow block (LineLayoutState::m_floats) during layout.
+        As we lay out the lines and iterate through RootInlineBox::m_floats, we
+        increment LineLayoutState::m_floatIndex. This LineLayoutState::m_floatIndex is
+        later used to find the matching float box in the per-block-flow float list.
+        This logic works fine as long as the lists and the index manipulation are tightly coded.
+        However due to the complexity of the line/float layout code, this is no longer the case.
+
+        This patch makes float box handling more secure by changing this index based setup
+        to a list iterator. It helps to eliminate potential vector overflow issues.
+
+        LineLayoutState::FloatList (new class) keeps track of all the floats for the block flow.
+        It groups the float box related functions/members and provides an iterator interface to ensure safer
+        syncing between this and the line based floats.
+
+        No change in functionality.
+
+        * rendering/RenderBlockFlow.h:
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlockFlow::appendFloatingObjectToLastLine):
+        (WebCore::repaintDirtyFloats):
+        (WebCore::RenderBlockFlow::layoutRunsAndFloats):
+        (WebCore::RenderBlockFlow::layoutRunsAndFloatsInRange):
+        (WebCore::RenderBlockFlow::linkToEndLineIfNeeded):
+        (WebCore::RenderBlockFlow::layoutLineBoxes):
+        (WebCore::RenderBlockFlow::checkFloatInCleanLine):
+        (WebCore::RenderBlockFlow::determineStartPosition):
+        (WebCore::RenderBlockFlow::determineEndPosition):
+        (WebCore::RenderBlockFlow::repaintDirtyFloats): Deleted.
+        (WebCore::RenderBlockFlow::checkFloatsInCleanLine): Deleted.
+        * rendering/line/LineLayoutState.h:
+        (WebCore::FloatWithRect::create):
+        (WebCore::FloatWithRect::renderer):
+        (WebCore::FloatWithRect::rect):
+        (WebCore::FloatWithRect::everHadLayout):
+        (WebCore::FloatWithRect::adjustRect):
+        (WebCore::FloatWithRect::FloatWithRect):
+        (WebCore::LineLayoutState::FloatList::append):
+        (WebCore::LineLayoutState::FloatList::setLastFloat):
+        (WebCore::LineLayoutState::FloatList::lastFloat):
+        (WebCore::LineLayoutState::FloatList::setLastCleanFloat):
+        (WebCore::LineLayoutState::FloatList::lastCleanFloat):
+        (WebCore::LineLayoutState::FloatList::floatWithRect):
+        (WebCore::LineLayoutState::FloatList::begin):
+        (WebCore::LineLayoutState::FloatList::end):
+        (WebCore::LineLayoutState::FloatList::find):
+        (WebCore::LineLayoutState::FloatList::isEmpty):
+        (WebCore::LineLayoutState::LineLayoutState):
+        (WebCore::LineLayoutState::floatList):
+        (WebCore::LineLayoutState::lastFloat): Deleted.
+        (WebCore::LineLayoutState::setLastFloat): Deleted.
+        (WebCore::LineLayoutState::floats): Deleted.
+        (WebCore::LineLayoutState::floatIndex): Deleted.
+        (WebCore::LineLayoutState::setFloatIndex): Deleted.
+
 2016-10-12  Said Abou-Hallawa  <sabouhall...@apple.com>
 
         Unreviewed, fix Windows build break after r207182.

Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (207218 => 207219)


--- trunk/Source/WebCore/rendering/RenderBlockFlow.h	2016-10-12 16:36:11 UTC (rev 207218)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h	2016-10-12 16:45:55 UTC (rev 207219)
@@ -33,6 +33,7 @@
 
 namespace WebCore {
 
+class FloatWithRect;
 class LayoutStateMaintainer;
 class LineBreaker;
 class LineInfo;
@@ -41,7 +42,6 @@
 class RenderNamedFlowFragment;
 class RenderRubyRun;
 
-struct FloatWithRect;
 struct WordMeasurement;
 
 template <class Run> class BidiRunList;
@@ -566,7 +566,7 @@
         float& availableLogicalWidth, BidiRun* firstRun, BidiRun* trailingSpaceRun, GlyphOverflowAndFallbackFontsMap& textBoxDataMap, VerticalPositionCache&, WordMeasurements&);
     void computeBlockDirectionPositionsForLine(RootInlineBox*, BidiRun*, GlyphOverflowAndFallbackFontsMap&, VerticalPositionCache&);
     BidiRun* handleTrailingSpaces(BidiRunList<BidiRun>&, BidiContext*);
-    void appendFloatingObjectToLastLine(FloatingObject*);
+    void appendFloatingObjectToLastLine(FloatingObject&);
     // Helper function for layoutInlineChildren()
     RootInlineBox* createLineBoxesFromBidiRuns(unsigned bidiLevel, BidiRunList<BidiRun>&, const InlineIterator& end, LineInfo&, VerticalPositionCache&, BidiRun* trailingSpaceRun, WordMeasurements&);
     void layoutRunsAndFloats(LineLayoutState&, bool hasInlineChild);
@@ -574,8 +574,8 @@
     void layoutRunsAndFloatsInRange(LineLayoutState&, InlineBidiResolver&, const InlineIterator& cleanLineStart, const BidiStatus& cleanLineBidiStatus, unsigned consecutiveHyphenatedLines);
     void reattachCleanLineFloats(RootInlineBox& cleanLine, LayoutUnit delta, bool isFirstCleanLine);
     void linkToEndLineIfNeeded(LineLayoutState&);
-    static void repaintDirtyFloats(Vector<FloatWithRect>& floats);
-    void checkFloatsInCleanLine(RootInlineBox*, Vector<FloatWithRect>&, size_t& floatIndex, bool& encounteredNewFloat, bool& dirtiedByFloat);
+    void checkFloatInCleanLine(RootInlineBox& cleanLine, RenderBox& floatBoxOnCleanLine, FloatWithRect& matchingFloatWithRect,
+        bool& encounteredNewFloat, bool& dirtiedByFloat);
     RootInlineBox* determineStartPosition(LineLayoutState&, InlineBidiResolver&);
     void determineEndPosition(LineLayoutState&, RootInlineBox* startBox, InlineIterator& cleanLineStart, BidiStatus& cleanLineBidiStatus);
     bool checkPaginationAndFloatsAtEndLine(LineLayoutState&);

Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (207218 => 207219)


--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2016-10-12 16:36:11 UTC (rev 207218)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp	2016-10-12 16:45:55 UTC (rev 207219)
@@ -1100,11 +1100,11 @@
     return trailingSpaceRun;
 }
 
-void RenderBlockFlow::appendFloatingObjectToLastLine(FloatingObject* floatingObject)
+void RenderBlockFlow::appendFloatingObjectToLastLine(FloatingObject& floatingObject)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(!floatingObject->originatingLine());
-    floatingObject->setOriginatingLine(lastRootBox());
-    lastRootBox()->appendFloat(floatingObject->renderer());
+    ASSERT_WITH_SECURITY_IMPLICATION(!floatingObject.originatingLine());
+    floatingObject.setOriginatingLine(lastRootBox());
+    lastRootBox()->appendFloat(floatingObject.renderer());
 }
 
 static inline void notifyResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
@@ -1249,6 +1249,20 @@
     }
 }
 
+static void repaintDirtyFloats(LineLayoutState::FloatList& floats)
+{
+    // Floats that did not have layout did not repaint when we laid them out. They would have
+    // painted by now if they had moved, but if they stayed at (0, 0), they still need to be
+    // painted.
+    for (auto& floatBox : floats) {
+        if (floatBox->everHadLayout())
+            continue;
+        auto& box = floatBox->renderer();
+        if (!box.x() && !box.y() && box.checkForRepaintDuringLayout())
+            box.repaint();
+    }
+}
+
 void RenderBlockFlow::layoutRunsAndFloats(LineLayoutState& layoutState, bool hasInlineChild)
 {
     // We want to skip ahead to the first dirty line
@@ -1279,7 +1293,7 @@
     }
 
     if (containsFloats())
-        layoutState.setLastFloat(m_floatingObjects->set().last().get());
+        layoutState.floatList().setLastFloat(m_floatingObjects->set().last().get());
 
     // We also find the first clean line and extract these lines.  We will add them back
     // if we determine that we're able to synchronize after handling all our dirty lines.
@@ -1312,7 +1326,7 @@
 
     layoutRunsAndFloatsInRange(layoutState, resolver, cleanLineStart, cleanLineBidiStatus, consecutiveHyphenatedLines);
     linkToEndLineIfNeeded(layoutState);
-    repaintDirtyFloats(layoutState.floats());
+    repaintDirtyFloats(layoutState.floatList());
 }
 
 // Before restarting the layout loop with a new logicalHeight, remove all floats that were added and reset the resolver.
@@ -1479,22 +1493,21 @@
             const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
             auto it = floatingObjectSet.begin();
             auto end = floatingObjectSet.end();
-            if (layoutState.lastFloat()) {
-                auto lastFloatIterator = floatingObjectSet.find<FloatingObject&, FloatingObjectHashTranslator>(*layoutState.lastFloat());
+            if (auto* lastFloat = layoutState.floatList().lastFloat()) {
+                auto lastFloatIterator = floatingObjectSet.find<FloatingObject&, FloatingObjectHashTranslator>(*lastFloat);
                 ASSERT(lastFloatIterator != end);
                 ++lastFloatIterator;
                 it = lastFloatIterator;
             }
             for (; it != end; ++it) {
-                FloatingObject* f = it->get();
-                appendFloatingObjectToLastLine(f);
-                ASSERT(&f->renderer() == &layoutState.floats()[layoutState.floatIndex()].object);
+                auto& floatingObject = *it;
+                appendFloatingObjectToLastLine(*floatingObject);
                 // If a float's geometry has changed, give up on syncing with clean lines.
-                if (layoutState.floats()[layoutState.floatIndex()].rect != f->frameRect())
+                auto* floatWithRect = layoutState.floatList().floatWithRect(floatingObject->renderer());
+                if (!floatWithRect || floatWithRect->rect() != floatingObject->frameRect())
                     checkForEndLineMatch = false;
-                layoutState.setFloatIndex(layoutState.floatIndex() + 1);
             }
-            layoutState.setLastFloat(!floatingObjectSet.isEmpty() ? floatingObjectSet.last().get() : nullptr);
+            layoutState.floatList().setLastFloat(!floatingObjectSet.isEmpty() ? floatingObjectSet.last().get() : nullptr);
         }
 
         lineWhitespaceCollapsingState.reset();
@@ -1641,33 +1654,18 @@
         const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
         auto it = floatingObjectSet.begin();
         auto end = floatingObjectSet.end();
-        if (layoutState.lastFloat()) {
-            auto lastFloatIterator = floatingObjectSet.find<FloatingObject&, FloatingObjectHashTranslator>(*layoutState.lastFloat());
+        if (auto* lastFloat = layoutState.floatList().lastFloat()) {
+            auto lastFloatIterator = floatingObjectSet.find<FloatingObject&, FloatingObjectHashTranslator>(*lastFloat);
             ASSERT(lastFloatIterator != end);
             ++lastFloatIterator;
             it = lastFloatIterator;
         }
         for (; it != end; ++it)
-            appendFloatingObjectToLastLine(it->get());
-        layoutState.setLastFloat(!floatingObjectSet.isEmpty() ? floatingObjectSet.last().get() : nullptr);
+            appendFloatingObjectToLastLine(**it);
+        layoutState.floatList().setLastFloat(!floatingObjectSet.isEmpty() ? floatingObjectSet.last().get() : nullptr);
     }
 }
 
-void RenderBlockFlow::repaintDirtyFloats(Vector<FloatWithRect>& floats)
-{
-    size_t floatCount = floats.size();
-    // Floats that did not have layout did not repaint when we laid them out. They would have
-    // painted by now if they had moved, but if they stayed at (0, 0), they still need to be
-    // painted.
-    for (size_t i = 0; i < floatCount; ++i) {
-        if (!floats[i].everHadLayout) {
-            RenderBox& box = floats[i].object;
-            if (!box.x() && !box.y() && box.checkForRepaintDuringLayout())
-                box.repaint();
-        }
-    }
-}
-
 void RenderBlockFlow::layoutLineBoxes(bool relayoutChildren, LayoutUnit& repaintLogicalTop, LayoutUnit& repaintLogicalBottom)
 {
     ASSERT(!m_simpleLineLayout);
@@ -1729,7 +1727,7 @@
                 if (box.isOutOfFlowPositioned())
                     box.containingBlock()->insertPositionedObject(box);
                 else if (box.isFloating())
-                    layoutState.floats().append(FloatWithRect(box));
+                    layoutState.floatList().append(FloatWithRect::create(box));
                 else if (isFullLayout || box.needsLayout()) {
                     // Replaced element.
                     box.dirtyLineBoxes(isFullLayout);
@@ -1785,129 +1783,131 @@
         checkLinesForTextOverflow();
 }
 
-void RenderBlockFlow::checkFloatsInCleanLine(RootInlineBox* line, Vector<FloatWithRect>& floats, size_t& floatIndex, bool& encounteredNewFloat, bool& dirtiedByFloat)
+void RenderBlockFlow::checkFloatInCleanLine(RootInlineBox& cleanLine, RenderBox& floatBoxOnCleanLine, FloatWithRect& matchingFloatWithRect,
+    bool& encounteredNewFloat, bool& dirtiedByFloat)
 {
-    Vector<RenderBox*>* cleanLineFloats = line->floatsPtr();
-    if (!cleanLineFloats)
+    ASSERT_WITH_SECURITY_IMPLICATION(!floatBoxOnCleanLine.style().deletionHasBegun());
+    if (&matchingFloatWithRect.renderer() != &floatBoxOnCleanLine) {
+        encounteredNewFloat = true;
         return;
+    }
+    floatBoxOnCleanLine.layoutIfNeeded();
+    LayoutRect originalFloatRect = matchingFloatWithRect.rect();
+    LayoutSize newSize(
+        floatBoxOnCleanLine.width() + floatBoxOnCleanLine.horizontalMarginExtent(),
+        floatBoxOnCleanLine.height() + floatBoxOnCleanLine.verticalMarginExtent());
     
-    if (!floats.size()) {
-        encounteredNewFloat = true;
+    // We have to reset the cap-height alignment done by the first-letter floats when initial-letter is set, so just always treat first-letter floats as dirty.
+    if (originalFloatRect.size() == newSize && (floatBoxOnCleanLine.style().styleType() != FIRST_LETTER || !floatBoxOnCleanLine.style().initialLetterDrop()))
         return;
-    }
 
-    for (auto it = cleanLineFloats->begin(), end = cleanLineFloats->end(); it != end; ++it) {
-        RenderBox* floatingBox = *it;
-        ASSERT_WITH_SECURITY_IMPLICATION(!floatingBox->style().deletionHasBegun());
-        floatingBox->layoutIfNeeded();
-        LayoutSize newSize(floatingBox->width() + floatingBox->horizontalMarginExtent(), floatingBox->height() + floatingBox->verticalMarginExtent());
-        ASSERT_WITH_SECURITY_IMPLICATION(floatIndex < floats.size());
-        if (&floats[floatIndex].object != floatingBox) {
-            encounteredNewFloat = true;
-            return;
-        }
-    
-        // We have to reset the cap-height alignment done by the first-letter floats when initial-letter is set, so just always treat first-letter floats
-        // as dirty.
-        if (floats[floatIndex].rect.size() != newSize || (floatingBox->style().styleType() == FIRST_LETTER && floatingBox->style().initialLetterDrop() > 0)) {
-            LayoutUnit floatTop = isHorizontalWritingMode() ? floats[floatIndex].rect.y() : floats[floatIndex].rect.x();
-            LayoutUnit floatHeight = isHorizontalWritingMode() ? std::max(floats[floatIndex].rect.height(), newSize.height()) : std::max(floats[floatIndex].rect.width(), newSize.width());
-            floatHeight = std::min(floatHeight, LayoutUnit::max() - floatTop);
-            line->markDirty();
-            markLinesDirtyInBlockRange(line->lineBottomWithLeading(), floatTop + floatHeight, line);
-            floats[floatIndex].rect.setSize(newSize);
-            dirtiedByFloat = true;
-        }
-        floatIndex++;
-    }
+    LayoutUnit floatTop = isHorizontalWritingMode() ? originalFloatRect.y() : originalFloatRect.x();
+    LayoutUnit floatHeight = isHorizontalWritingMode() ? std::max(originalFloatRect.height(), newSize.height())
+        : std::max(originalFloatRect.width(), newSize.width());
+    floatHeight = std::min(floatHeight, LayoutUnit::max() - floatTop);
+    cleanLine.markDirty();
+    markLinesDirtyInBlockRange(cleanLine.lineBottomWithLeading(), floatTop + floatHeight, &cleanLine);
+    LayoutRect newFloatRect = originalFloatRect;
+    newFloatRect.setSize(newSize);
+    matchingFloatWithRect.adjustRect(newFloatRect);
+    dirtiedByFloat = true;
 }
 
 RootInlineBox* RenderBlockFlow::determineStartPosition(LineLayoutState& layoutState, InlineBidiResolver& resolver)
 {
-    RootInlineBox* curr = 0;
-    RootInlineBox* last = 0;
+    RootInlineBox* currentLine = nullptr;
+    RootInlineBox* lastLine = nullptr;
 
     // FIXME: This entire float-checking block needs to be broken into a new function.
+    auto& floats = layoutState.floatList();
     bool dirtiedByFloat = false;
     if (!layoutState.isFullLayout()) {
         // Paginate all of the clean lines.
         bool paginated = view().layoutState() && view().layoutState()->isPaginated();
         LayoutUnit paginationDelta = 0;
-        size_t floatIndex = 0;
-        for (curr = firstRootBox(); curr && !curr->isDirty(); curr = curr->nextRootBox()) {
+        auto floatsIterator = floats.begin();
+        auto end = floats.end();
+        for (currentLine = firstRootBox(); currentLine && !currentLine->isDirty(); currentLine = currentLine->nextRootBox()) {
             if (paginated) {
-                if (lineWidthForPaginatedLineChanged(curr, 0, layoutState.flowThread())) {
-                    curr->markDirty();
+                if (lineWidthForPaginatedLineChanged(currentLine, 0, layoutState.flowThread())) {
+                    currentLine->markDirty();
                     break;
                 }
-                paginationDelta -= curr->paginationStrut();
+                paginationDelta -= currentLine->paginationStrut();
                 bool overflowsRegion;
-                adjustLinePositionForPagination(curr, paginationDelta, overflowsRegion, layoutState.flowThread());
+                adjustLinePositionForPagination(currentLine, paginationDelta, overflowsRegion, layoutState.flowThread());
                 if (paginationDelta) {
-                    if (containsFloats() || !layoutState.floats().isEmpty()) {
+                    if (containsFloats() || !floats.isEmpty()) {
                         // FIXME: Do better eventually.  For now if we ever shift because of pagination and floats are present just go to a full layout.
                         layoutState.markForFullLayout();
                         break;
                     }
 
-                    layoutState.updateRepaintRangeFromBox(curr, paginationDelta);
-                    curr->adjustBlockDirectionPosition(paginationDelta);
+                    layoutState.updateRepaintRangeFromBox(currentLine, paginationDelta);
+                    currentLine->adjustBlockDirectionPosition(paginationDelta);
                 }
                 if (layoutState.flowThread())
-                    updateRegionForLine(curr);
+                    updateRegionForLine(currentLine);
             }
 
-            // If a new float has been inserted before this line or before its last known float, just do a full layout.
-            bool encounteredNewFloat = false;
-            checkFloatsInCleanLine(curr, layoutState.floats(), floatIndex, encounteredNewFloat, dirtiedByFloat);
-            if (encounteredNewFloat)
-                layoutState.markForFullLayout();
-
-            if (dirtiedByFloat || layoutState.isFullLayout())
-                break;
+            if (auto* cleanLineFloats = currentLine->floatsPtr()) {
+                // If a new float has been inserted before this line or before its last known float, just do a full layout.
+                bool encounteredNewFloat = false;
+                for (auto* floatBoxOnCleanLine : *cleanLineFloats) {
+                    ASSERT(floatsIterator != end);
+                    checkFloatInCleanLine(*currentLine, *floatBoxOnCleanLine, *floatsIterator, encounteredNewFloat, dirtiedByFloat);
+                    ++floatsIterator;
+                    if (floatsIterator == end || encounteredNewFloat) {
+                        layoutState.markForFullLayout();
+                        break;
+                    }
+                }
+                if (dirtiedByFloat || encounteredNewFloat)
+                    break;
+            }
         }
         // Check if a new float has been inserted after the last known float.
-        if (!curr && floatIndex < layoutState.floats().size())
+        if (!currentLine && floatsIterator != end)
             layoutState.markForFullLayout();
     }
 
     if (layoutState.isFullLayout()) {
         m_lineBoxes.deleteLineBoxTree();
-        curr = 0;
-
+        currentLine = nullptr;
         ASSERT(!firstRootBox() && !lastRootBox());
     } else {
-        if (curr) {
+        if (currentLine) {
             // We have a dirty line.
-            if (RootInlineBox* prevRootBox = curr->prevRootBox()) {
+            if (RootInlineBox* prevRootBox = currentLine->prevRootBox()) {
                 // We have a previous line.
-                if (!dirtiedByFloat && !curr->hasAnonymousInlineBlock() && (!prevRootBox->endsWithBreak() || !prevRootBox->lineBreakObj() || (is<RenderText>(*prevRootBox->lineBreakObj()) && prevRootBox->lineBreakPos() >= downcast<RenderText>(*prevRootBox->lineBreakObj()).textLength()))) {
+                if (!dirtiedByFloat && !currentLine->hasAnonymousInlineBlock() && (!prevRootBox->endsWithBreak()
+                    || !prevRootBox->lineBreakObj()
+                    || (is<RenderText>(*prevRootBox->lineBreakObj())
+                    && prevRootBox->lineBreakPos() >= downcast<RenderText>(*prevRootBox->lineBreakObj()).textLength()))) {
                     // The previous line didn't break cleanly or broke at a newline
                     // that has been deleted, so treat it as dirty too.
-                    curr = prevRootBox;
+                    currentLine = prevRootBox;
                 }
             }
         }
         // If we have no dirty lines, then last is just the last root box.
-        last = curr ? curr->prevRootBox() : lastRootBox();
+        lastLine = currentLine ? currentLine->prevRootBox() : lastRootBox();
     }
 
-    unsigned numCleanFloats = 0;
-    if (!layoutState.floats().isEmpty()) {
+    if (!floats.isEmpty()) {
         LayoutUnit savedLogicalHeight = logicalHeight();
         // Restore floats from clean lines.
         RootInlineBox* line = firstRootBox();
-        while (line != curr) {
+        while (line != currentLine) {
             if (Vector<RenderBox*>* cleanLineFloats = line->floatsPtr()) {
                 for (auto it = cleanLineFloats->begin(), end = cleanLineFloats->end(); it != end; ++it) {
-                    RenderBox* floatingBox = *it;
-                    FloatingObject* floatingObject = insertFloatingObject(*floatingBox);
+                    auto* floatingBox = *it;
+                    auto* floatingObject = insertFloatingObject(*floatingBox);
                     ASSERT_WITH_SECURITY_IMPLICATION(!floatingObject->originatingLine());
                     floatingObject->setOriginatingLine(line);
                     setLogicalHeight(logicalTopForChild(*floatingBox) - marginBeforeForChild(*floatingBox));
                     positionNewFloats();
-                    ASSERT(&layoutState.floats()[numCleanFloats].object == floatingBox);
-                    numCleanFloats++;
+                    floats.setLastCleanFloat(*floatingBox);
                 }
             }
             line = line->nextRootBox();
@@ -1914,16 +1914,15 @@
         }
         setLogicalHeight(savedLogicalHeight);
     }
-    layoutState.setFloatIndex(numCleanFloats);
 
-    layoutState.lineInfo().setFirstLine(!last);
-    layoutState.lineInfo().setPreviousLineBrokeCleanly(!last || last->endsWithBreak());
+    layoutState.lineInfo().setFirstLine(!lastLine);
+    layoutState.lineInfo().setPreviousLineBrokeCleanly(!lastLine || lastLine->endsWithBreak());
 
-    if (last) {
-        setLogicalHeight(last->lineBottomWithLeading());
-        InlineIterator iter = InlineIterator(this, last->lineBreakObj(), last->lineBreakPos());
+    if (lastLine) {
+        setLogicalHeight(lastLine->lineBottomWithLeading());
+        InlineIterator iter = InlineIterator(this, lastLine->lineBreakObj(), lastLine->lineBreakPos());
         resolver.setPosition(iter, numberOfIsolateAncestors(iter));
-        resolver.setStatus(last->lineBreakBidiStatus());
+        resolver.setStatus(lastLine->lineBreakBidiStatus());
     } else {
         TextDirection direction = style().direction();
         if (style().unicodeBidi() == Plaintext)
@@ -1932,44 +1931,59 @@
         InlineIterator iter = InlineIterator(this, bidiFirstSkippingEmptyInlines(*this, &resolver), 0);
         resolver.setPosition(iter, numberOfIsolateAncestors(iter));
     }
-    return curr;
+    return currentLine;
 }
 
 void RenderBlockFlow::determineEndPosition(LineLayoutState& layoutState, RootInlineBox* startLine, InlineIterator& cleanLineStart, BidiStatus& cleanLineBidiStatus)
 {
+    auto iteratorForFirstDirtyFloat = [](LineLayoutState::FloatList& floats) {
+        auto lastCleanFloat = floats.lastCleanFloat();
+        if (!lastCleanFloat)
+            return floats.begin();
+        auto* lastCleanFloatWithRect = floats.floatWithRect(*lastCleanFloat);
+        ASSERT(lastCleanFloatWithRect);
+        return ++floats.find(*lastCleanFloatWithRect);
+    };
+
     ASSERT(!layoutState.endLine());
-    size_t floatIndex = layoutState.floatIndex();
-    RootInlineBox* last = 0;
-    for (RootInlineBox* curr = startLine->nextRootBox(); curr; curr = curr->nextRootBox()) {
-        if (!curr->isDirty()) {
-            bool encounteredNewFloat = false;
-            bool dirtiedByFloat = false;
-            checkFloatsInCleanLine(curr, layoutState.floats(), floatIndex, encounteredNewFloat, dirtiedByFloat);
-            if (encounteredNewFloat)
-                return;
+    auto floatsIterator = iteratorForFirstDirtyFloat(layoutState.floatList());
+    auto end = layoutState.floatList().end();
+    RootInlineBox* lastLine = nullptr;
+    for (RootInlineBox* currentLine = startLine->nextRootBox(); currentLine; currentLine = currentLine->nextRootBox()) {
+        if (!currentLine->isDirty()) {
+            if (auto* cleanLineFloats = currentLine->floatsPtr()) {
+                bool encounteredNewFloat = false;
+                bool dirtiedByFloat = false;
+                for (auto* floatBoxOnCleanLine : *cleanLineFloats) {
+                    ASSERT(floatsIterator != end);
+                    checkFloatInCleanLine(*currentLine, *floatBoxOnCleanLine, *floatsIterator, encounteredNewFloat, dirtiedByFloat);
+                    ++floatsIterator;
+                    if (floatsIterator == end || encounteredNewFloat)
+                        return;
+                }
+            }
         }
-        if (curr->isDirty())
-            last = 0;
-        else if (!last)
-            last = curr;
+        if (currentLine->isDirty())
+            lastLine = nullptr;
+        else if (!lastLine)
+            lastLine = currentLine;
     }
 
-    if (!last)
+    if (!lastLine)
         return;
 
     // At this point, |last| is the first line in a run of clean lines that ends with the last line
     // in the block.
+    RootInlineBox* previousLine = lastLine->prevRootBox();
+    cleanLineStart = InlineIterator(this, previousLine->lineBreakObj(), previousLine->lineBreakPos());
+    cleanLineBidiStatus = previousLine->lineBreakBidiStatus();
+    layoutState.setEndLineLogicalTop(previousLine->lineBottomWithLeading());
 
-    RootInlineBox* prev = last->prevRootBox();
-    cleanLineStart = InlineIterator(this, prev->lineBreakObj(), prev->lineBreakPos());
-    cleanLineBidiStatus = prev->lineBreakBidiStatus();
-    layoutState.setEndLineLogicalTop(prev->lineBottomWithLeading());
-
-    for (RootInlineBox* line = last; line; line = line->nextRootBox())
-        line->extractLine(); // Disconnect all line boxes from their render objects while preserving
-                             // their connections to one another.
-
-    layoutState.setEndLine(last);
+    for (RootInlineBox* line = lastLine; line; line = line->nextRootBox()) {
+        // Disconnect all line boxes from their render objects while preserving their connections to one another.
+        line->extractLine();
+    }
+    layoutState.setEndLine(lastLine);
 }
 
 bool RenderBlockFlow::checkPaginationAndFloatsAtEndLine(LineLayoutState& layoutState)

Modified: trunk/Source/WebCore/rendering/line/LineLayoutState.h (207218 => 207219)


--- trunk/Source/WebCore/rendering/line/LineLayoutState.h	2016-10-12 16:36:11 UTC (rev 207218)
+++ trunk/Source/WebCore/rendering/line/LineLayoutState.h	2016-10-12 16:45:55 UTC (rev 207219)
@@ -36,20 +36,37 @@
 
 #include "LayoutRect.h"
 #include "RenderBlockFlow.h"
+#include <wtf/RefCounted.h>
 
 namespace WebCore {
 
-struct FloatWithRect {
-    FloatWithRect(RenderBox& f)
-        : object(f)
-        , rect(LayoutRect(f.x() - f.marginLeft(), f.y() - f.marginTop(), f.width() + f.horizontalMarginExtent(), f.height() + f.verticalMarginExtent()))
-        , everHadLayout(f.everHadLayout())
+class FloatWithRect : public RefCounted<FloatWithRect> {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
+    static Ref<FloatWithRect> create(RenderBox& renderer)
     {
+        return adoptRef(*new FloatWithRect(renderer));
     }
+    
+    RenderBox& renderer() const { return m_renderer; }
+    LayoutRect rect() const { return m_rect; }
+    bool everHadLayout() const { return m_everHadLayout; }
 
-    RenderBox& object;
-    LayoutRect rect;
-    bool everHadLayout;
+    void adjustRect(const LayoutRect& rect) { m_rect = rect; }
+
+private:
+    FloatWithRect() = default;
+    
+    FloatWithRect(RenderBox& renderer)
+        : m_renderer(renderer)
+        , m_rect(LayoutRect(renderer.x() - renderer.marginLeft(), renderer.y() - renderer.marginTop(), renderer.width() + renderer.horizontalMarginExtent(), renderer.height() + renderer.verticalMarginExtent()))
+        , m_everHadLayout(renderer.everHadLayout())
+    {
+    }
+    
+    RenderBox& m_renderer;
+    LayoutRect m_rect;
+    bool m_everHadLayout { false };
 };
 
 // Like LayoutState for layout(), LineLayoutState keeps track of global information
@@ -56,13 +73,36 @@
 // during an entire linebox tree layout pass (aka layoutInlineChildren).
 class LineLayoutState {
 public:
+    class FloatList {
+    public:
+        void append(Ref<FloatWithRect>&& floatWithRect)
+        {
+            m_floats.add(floatWithRect.copyRef());
+            m_floatWithRectMap.add(&floatWithRect->renderer(), WTFMove(floatWithRect));
+        }
+        void setLastFloat(FloatingObject* lastFloat) { m_lastFloat = lastFloat; }
+        FloatingObject* lastFloat() const { return m_lastFloat; }
+
+        void setLastCleanFloat(RenderBox& floatBox) { m_lastCleanFloat = &floatBox; }
+        RenderBox* lastCleanFloat() const { return m_lastCleanFloat; }
+
+        FloatWithRect* floatWithRect(RenderBox& floatBox) const { return m_floatWithRectMap.get(&floatBox); }
+
+        using Iterator = ListHashSet<Ref<FloatWithRect>>::iterator;
+        Iterator begin() { return m_floats.begin(); }
+        Iterator end() { return m_floats.end(); }
+        Iterator find(FloatWithRect& floatBoxWithRect) { return m_floats.find(floatBoxWithRect); }
+        bool isEmpty() const { return m_floats.isEmpty(); }
+
+    private:
+        ListHashSet<Ref<FloatWithRect>> m_floats;
+        HashMap<RenderBox*, Ref<FloatWithRect>> m_floatWithRectMap;
+        FloatingObject* m_lastFloat { nullptr };
+        RenderBox* m_lastCleanFloat { nullptr };
+    };
+
     LineLayoutState(const RenderBlockFlow& blockFlow, bool fullLayout, LayoutUnit& repaintLogicalTop, LayoutUnit& repaintLogicalBottom, RenderFlowThread* flowThread)
-        : m_endLineLogicalTop(0)
-        , m_endLine(0)
-        , m_lastFloat(0)
-        , m_floatIndex(0)
-        , m_adjustedLogicalLineTop(0)
-        , m_flowThread(flowThread)
+        : m_flowThread(flowThread)
         , m_repaintLogicalTop(repaintLogicalTop)
         , m_repaintLogicalBottom(repaintLogicalBottom)
         , m_marginInfo(blockFlow, blockFlow.borderAndPaddingBefore(), blockFlow.borderAndPaddingAfter() + blockFlow.scrollbarLogicalHeight())
@@ -82,14 +122,6 @@
     RootInlineBox* endLine() const { return m_endLine; }
     void setEndLine(RootInlineBox* line) { m_endLine = line; }
 
-    FloatingObject* lastFloat() const { return m_lastFloat; }
-    void setLastFloat(FloatingObject* lastFloat) { m_lastFloat = lastFloat; }
-
-    Vector<FloatWithRect>& floats() { return m_floats; }
-
-    unsigned floatIndex() const { return m_floatIndex; }
-    void setFloatIndex(unsigned floatIndex) { m_floatIndex = floatIndex; }
-
     LayoutUnit adjustedLogicalLineTop() const { return m_adjustedLogicalLineTop; }
     void setAdjustedLogicalLineTop(LayoutUnit value) { m_adjustedLogicalLineTop = value; }
 
@@ -124,19 +156,18 @@
     LayoutUnit& prevFloatBottomFromAnonymousInlineBlock() { return m_prevFloatBottomFromAnonymousInlineBlock; }
     LayoutUnit& maxFloatBottomFromAnonymousInlineBlock() { return m_maxFloatBottomFromAnonymousInlineBlock; }
 
+    FloatList& floatList() { return m_floatList; }
+
 private:
     LineInfo m_lineInfo;
     LayoutUnit m_endLineLogicalTop;
-    RootInlineBox* m_endLine;
+    RootInlineBox* m_endLine { nullptr };
 
-    FloatingObject* m_lastFloat;
-    Vector<FloatWithRect> m_floats;
-    unsigned m_floatIndex;
-
     LayoutUnit m_adjustedLogicalLineTop;
 
-    RenderFlowThread* m_flowThread;
+    RenderFlowThread* m_flowThread { nullptr };
 
+    FloatList m_floatList;
     // FIXME: Should this be a range object instead of two ints?
     LayoutUnit& m_repaintLogicalTop;
     LayoutUnit& m_repaintLogicalBottom;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to