Refcount simple line layout
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Sep 2019 15:54:58 +0000 (15:54 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Sep 2019 15:54:58 +0000 (15:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=202104

Reviewed by Zalan Bujtas.

Make SimpleLineLayout::Layout refcounted for safety and ease of use.

* dom/Position.cpp:
(WebCore::Position::upstream const):
(WebCore::Position::downstream const):
* editing/TextIterator.cpp:
(WebCore::TextIterator::handleTextNode):
(WebCore::TextIterator::handleTextBox):
(WebCore::TextIterator::handleTextNodeFirstLetter):
* editing/TextIterator.h:
* rendering/RenderBlockFlow.h:
* rendering/RenderTreeAsText.cpp:
(WebCore::RenderTreeAsText::writeRenderObject):
(WebCore::write):
* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::create):
(WebCore::SimpleLineLayout::Layout::create):
* rendering/SimpleLineLayout.h:
* rendering/SimpleLineLayoutFunctions.cpp:
(WebCore::SimpleLineLayout::outputLineLayoutForFlow):
* rendering/SimpleLineLayoutResolver.cpp:
(WebCore::SimpleLineLayout::RunResolver::Run::rect const):
(WebCore::SimpleLineLayout::RunResolver::Iterator::Iterator):

Iterator now refs the layout. Since the resolver is owned by the layout, it is guaranteed to stay alive too.

(WebCore::SimpleLineLayout::RunResolver::Iterator::advanceLines):
* rendering/SimpleLineLayoutResolver.h:
(WebCore::SimpleLineLayout::RunResolver::Iterator::layout const):
(WebCore::SimpleLineLayout::RunResolver::Run::computeBaselinePosition const):
(WebCore::SimpleLineLayout::RunResolver::Iterator::simpleRun const):
(WebCore::SimpleLineLayout::RunResolver::Iterator::inQuirksMode const): Deleted.
(WebCore::SimpleLineLayout::runResolver): Deleted.

Always use the cached resolver owned by SimpleLineLayout::Layout.

* rendering/line/LineLayoutInterfaceTextBoxes.cpp:
(WebCore::LineLayoutInterface::firstTextBoxInVisualOrderFor):
(WebCore::LineLayoutInterface::firstTextBoxInTextOrderFor):
(WebCore::LineLayoutInterface::textBoxRangeFor):
(WebCore::LineLayoutInterface::Provider::firstTextBoxInVisualOrderFor): Deleted.
(WebCore::LineLayoutInterface::Provider::firstTextBoxInTextOrderFor): Deleted.
(WebCore::LineLayoutInterface::Provider::textBoxRangeFor): Deleted.

There is no need for a separate Provider class anymore as the iterator keeps SimpleLineLayout::Layout
and Resolver instances alive itself.

* rendering/line/LineLayoutInterfaceTextBoxes.h:
(WebCore::LineLayoutInterface::hasTextBoxes):
(WebCore::LineLayoutInterface::Provider::firstTextBoxFor): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250234 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/Position.cpp
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/TextIterator.h
Source/WebCore/rendering/RenderBlockFlow.h
Source/WebCore/rendering/RenderTreeAsText.cpp
Source/WebCore/rendering/SimpleLineLayout.cpp
Source/WebCore/rendering/SimpleLineLayout.h
Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp
Source/WebCore/rendering/SimpleLineLayoutResolver.cpp
Source/WebCore/rendering/SimpleLineLayoutResolver.h
Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.cpp
Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.h

index 9bb3605..38eda37 100644 (file)
@@ -1,3 +1,61 @@
+2019-09-23  Antti Koivisto  <antti@apple.com>
+
+        Refcount simple line layout
+        https://bugs.webkit.org/show_bug.cgi?id=202104
+
+        Reviewed by Zalan Bujtas.
+
+        Make SimpleLineLayout::Layout refcounted for safety and ease of use.
+
+        * dom/Position.cpp:
+        (WebCore::Position::upstream const):
+        (WebCore::Position::downstream const):
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::handleTextNode):
+        (WebCore::TextIterator::handleTextBox):
+        (WebCore::TextIterator::handleTextNodeFirstLetter):
+        * editing/TextIterator.h:
+        * rendering/RenderBlockFlow.h:
+        * rendering/RenderTreeAsText.cpp:
+        (WebCore::RenderTreeAsText::writeRenderObject):
+        (WebCore::write):
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::create):
+        (WebCore::SimpleLineLayout::Layout::create):
+        * rendering/SimpleLineLayout.h:
+        * rendering/SimpleLineLayoutFunctions.cpp:
+        (WebCore::SimpleLineLayout::outputLineLayoutForFlow):
+        * rendering/SimpleLineLayoutResolver.cpp:
+        (WebCore::SimpleLineLayout::RunResolver::Run::rect const):
+        (WebCore::SimpleLineLayout::RunResolver::Iterator::Iterator):
+
+        Iterator now refs the layout. Since the resolver is owned by the layout, it is guaranteed to stay alive too.
+
+        (WebCore::SimpleLineLayout::RunResolver::Iterator::advanceLines):
+        * rendering/SimpleLineLayoutResolver.h:
+        (WebCore::SimpleLineLayout::RunResolver::Iterator::layout const):
+        (WebCore::SimpleLineLayout::RunResolver::Run::computeBaselinePosition const):
+        (WebCore::SimpleLineLayout::RunResolver::Iterator::simpleRun const):
+        (WebCore::SimpleLineLayout::RunResolver::Iterator::inQuirksMode const): Deleted.
+        (WebCore::SimpleLineLayout::runResolver): Deleted.
+
+        Always use the cached resolver owned by SimpleLineLayout::Layout.
+
+        * rendering/line/LineLayoutInterfaceTextBoxes.cpp:
+        (WebCore::LineLayoutInterface::firstTextBoxInVisualOrderFor):
+        (WebCore::LineLayoutInterface::firstTextBoxInTextOrderFor):
+        (WebCore::LineLayoutInterface::textBoxRangeFor):
+        (WebCore::LineLayoutInterface::Provider::firstTextBoxInVisualOrderFor): Deleted.
+        (WebCore::LineLayoutInterface::Provider::firstTextBoxInTextOrderFor): Deleted.
+        (WebCore::LineLayoutInterface::Provider::textBoxRangeFor): Deleted.
+
+        There is no need for a separate Provider class anymore as the iterator keeps SimpleLineLayout::Layout
+        and Resolver instances alive itself.
+
+        * rendering/line/LineLayoutInterfaceTextBoxes.h:
+        (WebCore::LineLayoutInterface::hasTextBoxes):
+        (WebCore::LineLayoutInterface::Provider::firstTextBoxFor): Deleted.
+
 2019-09-23  Youenn Fablet  <youenn@apple.com>
 
         Simplify UserMediaPermissionRequestManager management of UserMediaRequest
index 4ae7e57..d2c9f73 100644 (file)
@@ -738,8 +738,7 @@ Position Position::upstream(EditingBoundaryCrossingRule rule) const
         if (is<RenderText>(*renderer)) {
             auto& textRenderer = downcast<RenderText>(*renderer);
 
-            LineLayoutInterface::Provider lineLayoutProvider;
-            auto firstTextBox = lineLayoutProvider.firstTextBoxInTextOrderFor(textRenderer);
+            auto firstTextBox = LineLayoutInterface::firstTextBoxInTextOrderFor(textRenderer);
             if (!firstTextBox)
                 continue;
 
@@ -849,8 +848,7 @@ Position Position::downstream(EditingBoundaryCrossingRule rule) const
         if (is<RenderText>(*renderer)) {
             auto& textRenderer = downcast<RenderText>(*renderer);
 
-            LineLayoutInterface::Provider lineLayoutProvider;
-            auto firstTextBox = lineLayoutProvider.firstTextBoxInTextOrderFor(textRenderer);
+            auto firstTextBox = LineLayoutInterface::firstTextBoxInTextOrderFor(textRenderer);
             if (!firstTextBox)
                 continue;
 
index bf41c81..ce3745e 100644 (file)
@@ -615,7 +615,7 @@ bool TextIterator::handleTextNode()
         return true;
     }
 
-    m_textBox = m_lineLayoutProvider.firstTextBoxInTextOrderFor(renderer);
+    m_textBox = LineLayoutInterface::firstTextBoxInTextOrderFor(renderer);
 
     bool shouldHandleFirstLetter = !m_handledFirstLetter && is<RenderTextFragment>(renderer) && !m_offset;
     if (shouldHandleFirstLetter)
@@ -642,7 +642,7 @@ void TextIterator::handleTextBox()
         return;
     }
 
-    auto firstTextBox = m_lineLayoutProvider.firstTextBoxInTextOrderFor(renderer);
+    auto firstTextBox = LineLayoutInterface::firstTextBoxInTextOrderFor(renderer);
 
     String rendererText = renderer.text();
     unsigned start = m_offset;
@@ -736,7 +736,7 @@ void TextIterator::handleTextNodeFirstLetter(RenderTextFragment& renderer)
         if (auto* firstLetterText = firstRenderTextInFirstLetter(firstLetter)) {
             m_handledFirstLetter = true;
             m_remainingTextBox = m_textBox;
-            m_textBox = m_lineLayoutProvider.firstTextBoxInTextOrderFor(*firstLetterText);
+            m_textBox = LineLayoutInterface::firstTextBoxInTextOrderFor(*firstLetterText);
             m_firstLetterText = firstLetterText;
         }
     }
index 5a0f98d..4462f3d 100644 (file)
@@ -181,8 +181,6 @@ private:
     bool m_lastTextNodeEndedWithCollapsedSpace { false };
     UChar m_lastCharacter { 0 };
 
-    LineLayoutInterface::Provider m_lineLayoutProvider;
-
     // Used when deciding whether to emit a "positioning" (e.g. newline) before any other content
     bool m_hasEmitted { false };
 
index 6e4294a..dab5817 100644 (file)
@@ -567,7 +567,7 @@ protected:
 
     // FIXME: Only one of these should be needed at any given time.
     std::unique_ptr<ComplexLineLayout> m_complexLineLayout;
-    std::unique_ptr<SimpleLineLayout::Layout> m_simpleLineLayout;
+    RefPtr<SimpleLineLayout::Layout> m_simpleLineLayout;
 
     friend class LineBreaker;
     friend class LineWidth; // Needs to know FloatingObject
index 8d4ed92..e75de0c 100644 (file)
@@ -202,8 +202,7 @@ void RenderTreeAsText::writeRenderObject(TextStream& ts, const RenderObject& o,
         // many test results.
         const RenderText& text = downcast<RenderText>(o);
         r = IntRect(text.firstRunLocation(), text.linesBoundingBox().size());
-        LineLayoutInterface::Provider lineLayoutProvider;
-        if (!lineLayoutProvider.firstTextBoxFor(text))
+        if (!LineLayoutInterface::hasTextBoxes(text))
             adjustForTableCells = false;
     } else if (o.isBR()) {
         const RenderLineBreak& br = downcast<RenderLineBreak>(o);
@@ -549,9 +548,8 @@ void write(TextStream& ts, const RenderObject& o, OptionSet<RenderAsTextFlag> be
     TextStream::IndentScope indentScope(ts);
 
     if (is<RenderText>(o)) {
-        LineLayoutInterface::Provider lineLayoutProvider;
         auto& text = downcast<RenderText>(o);
-        for (auto& textBox : lineLayoutProvider.textBoxRangeFor(text)) {
+        for (auto& textBox : LineLayoutInterface::textBoxRangeFor(text)) {
             ts << indent;
             writeTextBox(ts, text, textBox);
         }
index c362f4e..537c917 100644 (file)
@@ -972,7 +972,7 @@ static void createTextRuns(Layout::RunVector& runs, RenderBlockFlow& flow, unsig
     } while (!isEndOfContent);
 }
 
-std::unique_ptr<Layout> create(RenderBlockFlow& flow)
+Ref<Layout> create(RenderBlockFlow& flow)
 {
     unsigned lineCount = 0;
     Layout::RunVector runs;
@@ -980,10 +980,10 @@ std::unique_ptr<Layout> create(RenderBlockFlow& flow)
     return Layout::create(runs, lineCount, flow);
 }
 
-std::unique_ptr<Layout> Layout::create(const RunVector& runVector, unsigned lineCount, const RenderBlockFlow& blockFlow)
+Ref<Layout> Layout::create(const RunVector& runVector, unsigned lineCount, const RenderBlockFlow& blockFlow)
 {
     void* slot = WTF::fastMalloc(sizeof(Layout) + sizeof(Run) * runVector.size());
-    return std::unique_ptr<Layout>(new (NotNull, slot) Layout(runVector, lineCount, blockFlow));
+    return adoptRef(*new (NotNull, slot) Layout(runVector, lineCount, blockFlow));
 }
 
 Layout::Layout(const RunVector& runVector, unsigned lineCount, const RenderBlockFlow& blockFlow)
index 6e36549..4beb98c 100644 (file)
@@ -27,6 +27,7 @@
 
 #include "SimpleLineLayoutCoverage.h"
 #include "TextFlags.h"
+#include <wtf/RefCounted.h>
 #include <wtf/Vector.h>
 #include <wtf/text/WTFString.h>
 
@@ -75,12 +76,12 @@ struct SimpleLineStrut {
     float offset;
 };
 
-class Layout {
+class Layout : public RefCounted<Layout> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     using RunVector = Vector<Run, 10>;
     using SimpleLineStruts = Vector<SimpleLineStrut, 4>;
-    static std::unique_ptr<Layout> create(const RunVector&, unsigned lineCount, const RenderBlockFlow&);
+    static Ref<Layout> create(const RunVector&, unsigned lineCount, const RenderBlockFlow&);
 
     ~Layout();
 
@@ -108,7 +109,7 @@ private:
     Run m_runs[0];
 };
 
-std::unique_ptr<Layout> create(RenderBlockFlow&);
+Ref<Layout> create(RenderBlockFlow&);
 
 }
 }
index 0625b16..9df03c3 100644 (file)
@@ -392,7 +392,7 @@ static void printPrefix(TextStream& stream, int& printedCharacters, int depth)
         stream << " ";
 }
 
-void outputLineLayoutForFlow(TextStream& stream, const RenderBlockFlow& flow, const Layout& layout, int depth)
+void outputLineLayoutForFlow(TextStream& stream, const RenderBlockFlow&, const Layout& layout, int depth)
 {
     int printedCharacters = 0;
     printPrefix(stream, printedCharacters, depth);
@@ -401,7 +401,7 @@ void outputLineLayoutForFlow(TextStream& stream, const RenderBlockFlow& flow, co
     stream.nextLine();
     ++depth;
 
-    for (auto run : runResolver(flow, layout)) {
+    for (auto run : layout.runResolver()) {
         FloatRect rect = run.rect();
         printPrefix(stream, printedCharacters, depth);
         if (run.start() < run.end()) {
index d21d962..2823463 100644 (file)
@@ -68,7 +68,7 @@ FloatRect RunResolver::Run::rect() const
     FloatPoint position = linePosition(run.logicalLeft, baseline - resolver.m_ascent);
     FloatSize size = lineSize(run.logicalLeft, run.logicalRight, resolver.m_ascent + resolver.m_descent + resolver.m_visualOverflowOffset);
     bool moveLineBreakToBaseline = false;
-    if (run.start == run.end && m_iterator != resolver.begin() && m_iterator.inQuirksMode()) {
+    if (run.start == run.end && m_iterator != resolver.begin() && resolver.m_inQuirksMode) {
         auto previousRun = m_iterator;
         --previousRun;
         moveLineBreakToBaseline = !previousRun.simpleRun().isEndOfLine;
@@ -113,10 +113,12 @@ unsigned RunResolver::Run::localEnd() const
 }
 
 RunResolver::Iterator::Iterator(const RunResolver& resolver, unsigned runIndex, unsigned lineIndex)
-    : m_resolver(&resolver)
+    : m_layout(&resolver.m_layout)
+    , m_resolver(&resolver)
     , m_runIndex(runIndex)
     , m_lineIndex(lineIndex)
 {
+    ASSERT(&resolver == &m_layout->runResolver());
 }
 
 RunResolver::Iterator& RunResolver::Iterator::advance()
@@ -129,8 +131,8 @@ RunResolver::Iterator& RunResolver::Iterator::advance()
 
 RunResolver::Iterator& RunResolver::Iterator::advanceLines(unsigned lineCount)
 {
-    unsigned runCount = resolver().m_layout.runCount();
-    if (runCount == resolver().m_layout.lineCount()) {
+    unsigned runCount = layout().runCount();
+    if (runCount == layout().lineCount()) {
         m_runIndex = std::min(runCount, m_runIndex + lineCount);
         m_lineIndex = m_runIndex;
         return *this;
index 6ab7031..ca13cbe 100644 (file)
@@ -95,8 +95,9 @@ public:
         Iterator& advance();
         Iterator& advanceLines(unsigned);
         const RunResolver& resolver() const { return *m_resolver; }
-        bool inQuirksMode() const { return resolver().m_inQuirksMode; }
+        const Layout& layout() const { return *m_layout; }
 
+        RefPtr<const Layout> m_layout;
         const RunResolver* m_resolver;
         unsigned m_runIndex;
         unsigned m_lineIndex;
@@ -218,7 +219,7 @@ inline float RunResolver::Run::computeBaselinePosition() const
 {
     auto& resolver = m_iterator.resolver();
     auto offset = resolver.m_borderAndPaddingBefore + resolver.m_lineHeight * lineIndex();
-    if (!resolver.m_layout.hasLineStruts())
+    if (!m_iterator.layout().hasLineStruts())
         return offset + resolver.m_baseline;
     for (auto& strutEntry : resolver.m_layout.struts()) {
         if (strutEntry.lineBreak > lineIndex())
@@ -254,7 +255,7 @@ inline RunResolver::Run RunResolver::Iterator::operator*() const
 
 inline const SimpleLineLayout::Run& RunResolver::Iterator::simpleRun() const
 {
-    return resolver().m_layout.runAt(m_runIndex);
+    return layout().runAt(m_runIndex);
 }
 
 inline RunResolver::Iterator RunResolver::begin() const
@@ -299,11 +300,6 @@ inline WTF::IteratorRange<LineResolver::Iterator> LineResolver::rangeForRect(con
     return { Iterator(runRange.begin()), Iterator(runRange.end()) };
 }
 
-inline RunResolver runResolver(const RenderBlockFlow& flow, const Layout& layout)
-{
-    return RunResolver(flow, layout);
-}
-
 inline LineResolver lineResolver(const RunResolver& runResolver)
 {
     return LineResolver(runResolver);
index 399945c..7e9e017 100644 (file)
@@ -272,25 +272,17 @@ bool TextBoxIterator::atEnd() const
     return WTF::switchOn(m_pathVariant, simple, complex);
 }
 
-Provider::Provider() = default;
-Provider::~Provider() = default;
-
-TextBoxIterator Provider::firstTextBoxInVisualOrderFor(const RenderText& text)
+TextBoxIterator firstTextBoxInVisualOrderFor(const RenderText& text)
 {
     if (auto* simpleLineLayout = text.simpleLineLayout()) {
-        auto& parent = downcast<const RenderBlockFlow>(*text.parent());
-        auto& resolver = m_simpleLineLayoutResolvers.ensure(&parent, [&] {
-            return makeUnique<SimpleLineLayout::RunResolver>(parent, *simpleLineLayout);
-        }).iterator->value;
-
-        auto range = resolver->rangeForRenderer(text);
+        auto range = simpleLineLayout->runResolver().rangeForRenderer(text);
         return { range.begin(), range.end() };
     }
 
     return TextBoxIterator { text.firstTextBox() };
 }
 
-TextBoxIterator Provider::firstTextBoxInTextOrderFor(const RenderText& text)
+TextBoxIterator firstTextBoxInTextOrderFor(const RenderText& text)
 {
     if (!text.simpleLineLayout() && text.containsReversedText()) {
         Vector<const InlineTextBox*> sortedTextBoxes;
@@ -303,7 +295,7 @@ TextBoxIterator Provider::firstTextBoxInTextOrderFor(const RenderText& text)
     return firstTextBoxInVisualOrderFor(text);
 }
 
-TextBoxRange Provider::textBoxRangeFor(const RenderText& text)
+TextBoxRange textBoxRangeFor(const RenderText& text)
 {
     return { firstTextBoxInVisualOrderFor(text) };
 }
index ce8f61d..634c4cb 100644 (file)
@@ -35,7 +35,6 @@
 namespace WebCore {
 
 class InlineTextBox;
-class Provider;
 class RenderText;
 
 namespace LineLayoutInterface {
@@ -131,19 +130,10 @@ private:
     TextBoxIterator m_begin;
 };
 
-class Provider {
-public:
-    Provider();
-    ~Provider();
-
-    TextBoxIterator firstTextBoxFor(const RenderText& text) { return firstTextBoxInVisualOrderFor(text); }
-    TextBoxIterator firstTextBoxInVisualOrderFor(const RenderText&);
-    TextBoxIterator firstTextBoxInTextOrderFor(const RenderText&);
-    TextBoxRange textBoxRangeFor(const RenderText&);
-
-private:
-    HashMap<const RenderBlockFlow*, std::unique_ptr<SimpleLineLayout::RunResolver>> m_simpleLineLayoutResolvers;
-};
+TextBoxIterator firstTextBoxInVisualOrderFor(const RenderText&);
+TextBoxIterator firstTextBoxInTextOrderFor(const RenderText&);
+TextBoxRange textBoxRangeFor(const RenderText&);
+inline bool hasTextBoxes(const RenderText& text) { return !firstTextBoxInVisualOrderFor(text).atEnd(); }
 
 }
 }