Title: [250234] trunk/Source/WebCore
Revision
250234
Author
an...@apple.com
Date
2019-09-23 08:54:58 -0700 (Mon, 23 Sep 2019)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (250233 => 250234)


--- trunk/Source/WebCore/ChangeLog	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/ChangeLog	2019-09-23 15:54:58 UTC (rev 250234)
@@ -1,3 +1,61 @@
+2019-09-23  Antti Koivisto  <an...@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  <you...@apple.com>
 
         Simplify UserMediaPermissionRequestManager management of UserMediaRequest

Modified: trunk/Source/WebCore/dom/Position.cpp (250233 => 250234)


--- trunk/Source/WebCore/dom/Position.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/dom/Position.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -738,8 +738,7 @@
         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 @@
         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;
 

Modified: trunk/Source/WebCore/editing/TextIterator.cpp (250233 => 250234)


--- trunk/Source/WebCore/editing/TextIterator.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/editing/TextIterator.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -615,7 +615,7 @@
         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 @@
         return;
     }
 
-    auto firstTextBox = m_lineLayoutProvider.firstTextBoxInTextOrderFor(renderer);
+    auto firstTextBox = LineLayoutInterface::firstTextBoxInTextOrderFor(renderer);
 
     String rendererText = renderer.text();
     unsigned start = m_offset;
@@ -736,7 +736,7 @@
         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;
         }
     }

Modified: trunk/Source/WebCore/editing/TextIterator.h (250233 => 250234)


--- trunk/Source/WebCore/editing/TextIterator.h	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/editing/TextIterator.h	2019-09-23 15:54:58 UTC (rev 250234)
@@ -181,8 +181,6 @@
     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 };
 

Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (250233 => 250234)


--- trunk/Source/WebCore/rendering/RenderBlockFlow.h	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h	2019-09-23 15:54:58 UTC (rev 250234)
@@ -567,7 +567,7 @@
 
     // 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

Modified: trunk/Source/WebCore/rendering/RenderTreeAsText.cpp (250233 => 250234)


--- trunk/Source/WebCore/rendering/RenderTreeAsText.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/RenderTreeAsText.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -202,8 +202,7 @@
         // 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 @@
     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);
         }

Modified: trunk/Source/WebCore/rendering/SimpleLineLayout.cpp (250233 => 250234)


--- trunk/Source/WebCore/rendering/SimpleLineLayout.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/SimpleLineLayout.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -972,7 +972,7 @@
     } while (!isEndOfContent);
 }
 
-std::unique_ptr<Layout> create(RenderBlockFlow& flow)
+Ref<Layout> create(RenderBlockFlow& flow)
 {
     unsigned lineCount = 0;
     Layout::RunVector runs;
@@ -980,10 +980,10 @@
     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)

Modified: trunk/Source/WebCore/rendering/SimpleLineLayout.h (250233 => 250234)


--- trunk/Source/WebCore/rendering/SimpleLineLayout.h	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/SimpleLineLayout.h	2019-09-23 15:54:58 UTC (rev 250234)
@@ -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 @@
     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 @@
     Run m_runs[0];
 };
 
-std::unique_ptr<Layout> create(RenderBlockFlow&);
+Ref<Layout> create(RenderBlockFlow&);
 
 }
 }

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (250233 => 250234)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -392,7 +392,7 @@
         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 @@
     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()) {

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp (250233 => 250234)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -68,7 +68,7 @@
     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 @@
 }
 
 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::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;

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.h (250233 => 250234)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.h	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.h	2019-09-23 15:54:58 UTC (rev 250234)
@@ -95,8 +95,9 @@
         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 @@
 {
     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 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 @@
     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);

Modified: trunk/Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.cpp (250233 => 250234)


--- trunk/Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.cpp	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.cpp	2019-09-23 15:54:58 UTC (rev 250234)
@@ -272,18 +272,10 @@
     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() };
     }
 
@@ -290,7 +282,7 @@
     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 @@
     return firstTextBoxInVisualOrderFor(text);
 }
 
-TextBoxRange Provider::textBoxRangeFor(const RenderText& text)
+TextBoxRange textBoxRangeFor(const RenderText& text)
 {
     return { firstTextBoxInVisualOrderFor(text) };
 }

Modified: trunk/Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.h (250233 => 250234)


--- trunk/Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.h	2019-09-23 14:16:44 UTC (rev 250233)
+++ trunk/Source/WebCore/rendering/line/LineLayoutInterfaceTextBoxes.h	2019-09-23 15:54:58 UTC (rev 250234)
@@ -35,7 +35,6 @@
 namespace WebCore {
 
 class InlineTextBox;
-class Provider;
 class RenderText;
 
 namespace LineLayoutInterface {
@@ -131,19 +130,10 @@
     TextBoxIterator m_begin;
 };
 
-class Provider {
-public:
-    Provider();
-    ~Provider();
+TextBoxIterator firstTextBoxInVisualOrderFor(const RenderText&);
+TextBoxIterator firstTextBoxInTextOrderFor(const RenderText&);
+TextBoxRange textBoxRangeFor(const RenderText&);
+inline bool hasTextBoxes(const RenderText& text) { return !firstTextBoxInVisualOrderFor(text).atEnd(); }
 
-    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;
-};
-
 }
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to