Attachment 339885 Details for Bug 185411 – Patch

[patch] Patch

bug-185411-20180508145146.patch (text/plain), 12.75 KB, created by zalan on 2018-05-08 14:51:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-05-08 14:51:47 PDT

Size: 12.75 KB

patch

obsolete

>Subversion Revision: 231371
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0ba10b34f9106c347d262918c00eac5d257f085d..a555e07d61fce98c708fde14611cf1140542562a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2018-05-08  Zalan Bujtas  <zalan@apple.com>
>+
>+        [Simple line layout] Cache run resolver.
>+        https://bugs.webkit.org/show_bug.cgi?id=185411
>+
>+        Reviewed by Antti Koivisto.
>+
>+        This patch caches the run resolver on the [SimpleLine]Layout object. 
>+        In certain cases, when the block container has thousands of elements (foobar1<br>foobar2<br>.....foobar9999<br>),
>+        constructing the resolver (and its dependencies) in a repeating fashion could hang the WebProcess.
>+
>+        Covered by existing tests.
>+
>+        * rendering/SimpleLineLayout.cpp:
>+        (WebCore::SimpleLineLayout::create):
>+        (WebCore::SimpleLineLayout::Layout::create):
>+        (WebCore::SimpleLineLayout::Layout::Layout):
>+        * rendering/SimpleLineLayout.h:
>+        (WebCore::SimpleLineLayout::Layout::runResolver const):
>+        * rendering/SimpleLineLayoutFunctions.cpp:
>+        (WebCore::SimpleLineLayout::paintFlow):
>+        (WebCore::SimpleLineLayout::hitTestFlow):
>+        (WebCore::SimpleLineLayout::collectFlowOverflow):
>+        (WebCore::SimpleLineLayout::computeBoundingBox):
>+        (WebCore::SimpleLineLayout::computeFirstRunLocation):
>+        (WebCore::SimpleLineLayout::collectAbsoluteRects):
>+        (WebCore::SimpleLineLayout::collectAbsoluteQuads):
>+        (WebCore::SimpleLineLayout::textOffsetForPoint):
>+        (WebCore::SimpleLineLayout::collectAbsoluteQuadsForRange):
>+        (WebCore::SimpleLineLayout::generateLineBoxTree):
>+        * rendering/SimpleLineLayoutResolver.cpp:
>+        (WebCore::SimpleLineLayout::LineResolver::LineResolver):
>+        * rendering/SimpleLineLayoutResolver.h:
>+        (WebCore::SimpleLineLayout::lineResolver):
>+
> 2018-05-04  Chris Dumez  <cdumez@apple.com>
> 
>         Unreviewed, rolling out r231331.
>diff --git a/Source/WebCore/rendering/SimpleLineLayout.cpp b/Source/WebCore/rendering/SimpleLineLayout.cpp
>index 301300f6e37ded5672bc0d65a1af3ccf2e1c4336..2a3e67054b736de7cb949371beaeec54687eccdd 100644
>--- a/Source/WebCore/rendering/SimpleLineLayout.cpp
>+++ b/Source/WebCore/rendering/SimpleLineLayout.cpp
>@@ -50,6 +50,7 @@
> #include "Settings.h"
> #include "SimpleLineLayoutFlowContents.h"
> #include "SimpleLineLayoutFunctions.h"
>+#include "SimpleLineLayoutResolver.h"
> #include "SimpleLineLayoutTextFragmentIterator.h"
> #include "Text.h"
> #include "TextPaintStyle.h"
>@@ -962,22 +963,30 @@ std::unique_ptr<Layout> create(RenderBlockFlow& flow)
>     unsigned lineCount = 0;
>     Layout::RunVector runs;
>     createTextRuns(runs, flow, lineCount);
>-    return Layout::create(runs, lineCount);
>+    return Layout::create(runs, lineCount, flow);
> }
> 
>-std::unique_ptr<Layout> Layout::create(const RunVector& runVector, unsigned lineCount)
>+std::unique_ptr<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));
>+    return std::unique_ptr<Layout>(new (NotNull, slot) Layout(runVector, lineCount, blockFlow));
> }
> 
>-Layout::Layout(const RunVector& runVector, unsigned lineCount)
>+Layout::Layout(const RunVector& runVector, unsigned lineCount, const RenderBlockFlow& blockFlow)
>     : m_lineCount(lineCount)
>     , m_runCount(runVector.size())
>+    , m_blockFlowRenderer(blockFlow)
> {
>     memcpy(m_runs, runVector.data(), m_runCount * sizeof(Run));
> }
> 
>+const RunResolver& Layout::runResolver() const
>+{
>+    if (!m_runResolver)
>+        m_runResolver = std::make_unique<RunResolver>(m_blockFlowRenderer, *this);
>+    return *m_runResolver;
>+}
>+
> Layout::~Layout()
> {
>     simpleLineLayoutWillBeDeleted(*this);
>diff --git a/Source/WebCore/rendering/SimpleLineLayout.h b/Source/WebCore/rendering/SimpleLineLayout.h
>index 527447d56814d5f20e1b141cfc1c11b84477c8d2..6e365495367b12d6c9acd31d3bd1f909b7201c22 100644
>--- a/Source/WebCore/rendering/SimpleLineLayout.h
>+++ b/Source/WebCore/rendering/SimpleLineLayout.h
>@@ -41,10 +41,11 @@ class RenderBlockFlow;
> 
> namespace SimpleLineLayout {
> 
>+class RunResolver;
>+
> bool canUseFor(const RenderBlockFlow&);
> AvoidanceReasonFlags canUseForWithReason(const RenderBlockFlow&, IncludeReasons);
> 
>-
> struct Run {
> #if COMPILER(MSVC)
>     Run() { }
>@@ -79,7 +80,7 @@ class Layout {
> public:
>     using RunVector = Vector<Run, 10>;
>     using SimpleLineStruts = Vector<SimpleLineStrut, 4>;
>-    static std::unique_ptr<Layout> create(const RunVector&, unsigned lineCount);
>+    static std::unique_ptr<Layout> create(const RunVector&, unsigned lineCount, const RenderBlockFlow&);
> 
>     ~Layout();
> 
>@@ -93,13 +94,17 @@ public:
>     bool hasLineStruts() const { return !m_lineStruts.isEmpty(); }
>     void setLineStruts(SimpleLineStruts&& lineStruts) { m_lineStruts = lineStruts; }
>     const SimpleLineStruts& struts() const { return m_lineStruts; }
>+    const RunResolver& runResolver() const;
>+
> private:
>-    Layout(const RunVector&, unsigned lineCount);
>+    Layout(const RunVector&, unsigned lineCount, const RenderBlockFlow&);
> 
>     unsigned m_lineCount;
>     unsigned m_runCount;
>     bool m_isPaginated { false };
>     SimpleLineStruts m_lineStruts;
>+    const RenderBlockFlow& m_blockFlowRenderer;
>+    mutable std::unique_ptr<RunResolver> m_runResolver;
>     Run m_runs[0];
> };
> 
>diff --git a/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp b/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp
>index faf46ccb641e4180f53db1783e3f8ae6f8ad11c4..c6f05b53324bb36c9b1fdc271721c9ca520e5f10 100644
>--- a/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp
>+++ b/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp
>@@ -105,7 +105,7 @@ void paintFlow(const RenderBlockFlow& flow, const Layout& layout, PaintInfo& pai
>     LayoutRect paintRect = paintInfo.rect;
>     paintRect.moveBy(-paintOffset);
> 
>-    auto resolver = runResolver(flow, layout);
>+    auto& resolver = layout.runResolver();
>     float deviceScaleFactor = flow.document().deviceScaleFactor();
>     for (auto run : resolver.rangeForRect(paintRect)) {
>         if (run.start() == run.end())
>@@ -147,7 +147,7 @@ bool hitTestFlow(const RenderBlockFlow& flow, const Layout& layout, const HitTes
> 
>     LayoutRect rangeRect = locationInContainer.boundingBox();
>     rangeRect.moveBy(-accumulatedOffset);
>-    auto resolver = lineResolver(flow, layout);
>+    auto resolver = lineResolver(layout.runResolver());
>     auto range = resolver.rangeForRect(rangeRect);
>     for (auto it = range.begin(), end = range.end(); it != end; ++it) {
>         auto lineRect = *it;
>@@ -164,7 +164,7 @@ bool hitTestFlow(const RenderBlockFlow& flow, const Layout& layout, const HitTes
> 
> void collectFlowOverflow(RenderBlockFlow& flow, const Layout& layout)
> {
>-    for (auto lineRect : lineResolver(flow, layout)) {
>+    for (auto lineRect : lineResolver(layout.runResolver())) {
>         LayoutRect visualOverflowRect = LayoutRect(computeOverflow(flow, lineRect));
>         flow.addLayoutOverflow(LayoutRect(lineRect));
>         flow.addVisualOverflow(visualOverflowRect);
>@@ -173,7 +173,7 @@ void collectFlowOverflow(RenderBlockFlow& flow, const Layout& layout)
> 
> IntRect computeBoundingBox(const RenderObject& renderer, const Layout& layout)
> {
>-    auto resolver = runResolver(downcast<RenderBlockFlow>(*renderer.parent()), layout);
>+    auto& resolver = layout.runResolver();
>     FloatRect boundingBoxRect;
>     for (auto run : resolver.rangeForRenderer(renderer)) {
>         FloatRect rect = run.rect();
>@@ -187,7 +187,7 @@ IntRect computeBoundingBox(const RenderObject& renderer, const Layout& layout)
> 
> IntPoint computeFirstRunLocation(const RenderObject& renderer, const Layout& layout)
> {
>-    auto resolver = runResolver(downcast<RenderBlockFlow>(*renderer.parent()), layout);
>+    auto& resolver = layout.runResolver();
>     auto range = resolver.rangeForRenderer(renderer);
>     auto begin = range.begin();
>     if (begin == range.end())
>@@ -198,7 +198,7 @@ IntPoint computeFirstRunLocation(const RenderObject& renderer, const Layout& lay
> Vector<IntRect> collectAbsoluteRects(const RenderObject& renderer, const Layout& layout, const LayoutPoint& accumulatedOffset)
> {
>     Vector<IntRect> rects;
>-    auto resolver = runResolver(downcast<RenderBlockFlow>(*renderer.parent()), layout);
>+    auto& resolver = layout.runResolver();
>     for (auto run : resolver.rangeForRenderer(renderer)) {
>         FloatRect rect = run.rect();
>         rects.append(enclosingIntRect(FloatRect(accumulatedOffset + rect.location(), rect.size())));
>@@ -209,7 +209,7 @@ Vector<IntRect> collectAbsoluteRects(const RenderObject& renderer, const Layout&
> Vector<FloatQuad> collectAbsoluteQuads(const RenderObject& renderer, const Layout& layout, bool* wasFixed)
> {
>     Vector<FloatQuad> quads;
>-    auto resolver = runResolver(downcast<RenderBlockFlow>(*renderer.parent()), layout);
>+    auto& resolver = layout.runResolver();
>     for (auto run : resolver.rangeForRenderer(renderer))
>         quads.append(renderer.localToAbsoluteQuad(FloatQuad(run.rect()), UseTransforms, wasFixed));
>     return quads;
>@@ -219,7 +219,7 @@ unsigned textOffsetForPoint(const LayoutPoint& point, const RenderText& renderer
> {
>     auto& flow = downcast<RenderBlockFlow>(*renderer.parent());
>     ASSERT(flow.firstChild() == flow.lastChild());
>-    auto resolver = runResolver(flow, layout);
>+    auto& resolver = layout.runResolver();
>     auto it = resolver.runForPoint(point);
>     if (it == resolver.end())
>         return renderer.text().length();
>@@ -234,7 +234,7 @@ Vector<FloatQuad> collectAbsoluteQuadsForRange(const RenderObject& renderer, uns
> {
>     auto& style = downcast<RenderBlockFlow>(*renderer.parent()).style();
>     Vector<FloatQuad> quads;
>-    auto resolver = runResolver(downcast<RenderBlockFlow>(*renderer.parent()), layout);
>+    auto& resolver = layout.runResolver();
>     for (auto run : resolver.rangeForRendererWithOffsets(renderer, start, end)) {
>         // This run is fully contained.
>         if (start <= run.start() && end >= run.end()) {
>@@ -320,7 +320,7 @@ void generateLineBoxTree(RenderBlockFlow& flow, const Layout& layout)
>         return;
> 
>     Ref<BidiContext> bidiContext = BidiContext::create(0, U_LEFT_TO_RIGHT);
>-    auto resolver = runResolver(flow, layout);
>+    auto& resolver = layout.runResolver();
>     unsigned lineIndex = 0;
>     while (true) {
>         auto range = resolver.rangeForLine(lineIndex++);
>diff --git a/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp b/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp
>index 4764295c21faf006036a37ce572b0a2d2bac8cd5..d1f7752ea89027a5d20c98501354f0cb3e2253f7 100644
>--- a/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp
>+++ b/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp
>@@ -313,8 +313,8 @@ const RenderObject& LineResolver::Iterator::renderer() const
>     return m_runIterator.resolver().flowContents().segmentForRun(run.start(), run.end()).renderer;
> }
> 
>-LineResolver::LineResolver(const RenderBlockFlow& flow, const Layout& layout)
>-    : m_runResolver(flow, layout)
>+LineResolver::LineResolver(const RunResolver& runResolver)
>+    : m_runResolver(runResolver)
> {
> }
> 
>diff --git a/Source/WebCore/rendering/SimpleLineLayoutResolver.h b/Source/WebCore/rendering/SimpleLineLayoutResolver.h
>index 0e8f207a99a6f4ef42bb913dc5ee8b22431b377c..7eb6a4327f50bff734eefcd627b444d20c10924c 100644
>--- a/Source/WebCore/rendering/SimpleLineLayoutResolver.h
>+++ b/Source/WebCore/rendering/SimpleLineLayoutResolver.h
>@@ -27,6 +27,7 @@
> 
> #include "LayoutRect.h"
> #include "RenderBlockFlow.h"
>+#include "SimpleLineLayout.h"
> #include "SimpleLineLayoutFlowContents.h"
> #include <wtf/IteratorRange.h>
> #include <wtf/text/WTFString.h>
>@@ -143,7 +144,7 @@ public:
>         RunResolver::Iterator m_runIterator;
>     };
> 
>-    LineResolver(const RenderBlockFlow&, const Layout&);
>+    LineResolver(const RunResolver&);
> 
>     Iterator begin() const;
>     Iterator end() const;
>@@ -151,11 +152,11 @@ public:
>     WTF::IteratorRange<Iterator> rangeForRect(const LayoutRect&) const;
> 
> private:
>-    RunResolver m_runResolver;
>+    const RunResolver& m_runResolver;
> };
> 
> RunResolver runResolver(const RenderBlockFlow&, const Layout&);
>-LineResolver lineResolver(const RenderBlockFlow&, const Layout&);
>+LineResolver lineResolver(const RunResolver&);
> 
> inline unsigned RunResolver::Run::start() const
> {
>@@ -297,9 +298,9 @@ inline RunResolver runResolver(const RenderBlockFlow& flow, const Layout& layout
>     return RunResolver(flow, layout);
> }
> 
>-inline LineResolver lineResolver(const RenderBlockFlow& flow, const Layout& layout)
>+inline LineResolver lineResolver(const RunResolver& runResolver)
> {
>-    return LineResolver(flow, layout);
>+    return LineResolver(runResolver);
> }
> 
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 185411: 339794 | 339834 | 339885