Bidi-Isolate inlines break layout with collapsed whitespace
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Jul 2015 20:39:16 +0000 (20:39 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Jul 2015 20:39:16 +0000 (20:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=109624
<rdar://problem/21752834>

Reviewed by David Hyatt.

Source/WebCore:

This patch changes the logic in constructBidiRunsForSegment() when it encounters an
isolate. It already has logic to create a BidiResolver for the isolated text;
however, that logic doesn't handle setting up the MidpointState at all.
Specifically, we can set the MidpointState's cursor to point to the context which
we can remember from addPlaceholderRunForIsolatedInline(). This information is
remembered in a HashMap in BidiResolver.

This patch is a partial port of Blink patch
https://src.chromium.org/viewvc/blink?view=rev&revision=159203

Here is some explanatory text regarding how we collapse spaces:

Collapsing whitespace happens in a series of phases. The first phase occurs when
we perform line breaking. Here, we keep track of sequences of whitespace which
should be collapsed, in the form of a vector of pairs of InlineIterators. We put
this knowledge into a MidpointState object.

Then, once we have a line, we run the bidi algorithm on the line (including the
whitespace). As output, the bidi algorithm calls the BidiResolver::appendRun()
callback with two InlineIterators each time it wants to create a run. Because
each renderer that we create has to be owned by exactly one DOM node,
BidiResolver::appendRun() iterates between its two InlineIterator arguments,
calling RenderBlockFlow::appendRunsForObject() on each interstitial DOM node.

This is the function where whitespace collapsing happens. The MidpointState object
keeps a cursor into its remembered whitespace sequences. Here, we simply make a
bidi run for each region in between adjacent whitespace pairs in the MidpointState
object. These bidi runs eventually get turned into leaf InlineBoxes.

The problem is that the BidiResolver::appendRun() callbacks don't occur in
string-order, but the Midpoint InlineIterator pairs are in string-order. In
particular, within a particular isolate, appendRun() gets called in string
order, but callbacks that occur for inner isolates are deferred. This means that
RenderBlockFlow::appendRunsForObject() gets confused when it looks for relevant
whitespace to skip.

Test: fast/text/bidi-isolate-whitespace-collapse.html

* platform/text/BidiResolver.h:
(WebCore::MidpointState::numMidpoints): Returning a const unsigned& is silly.
(WebCore::MidpointState::currentMidpoint): Ditto.
(WebCore::MidpointState::setCurrentMidpoint): The isolated MidpointState object
needs to be able to set its current midpoint to point to the first one inside
the isolate.
(WebCore::MidpointState::decrementNumMidpoints): Renamed from "decrease"
(WebCore::MidpointState::betweenMidpoints): This function is true iff
currentMidpoint() % 2. Instead of keeping a member variable, we can just compute
that.
(WebCore::MidpointState::reset): Deleted.
(WebCore::MidpointState::decreaseNumMidpoints): Deleted.
(WebCore::MidpointState::setBetweenMidpoints): Deleted.
* rendering/InlineIterator.h:
(WebCore::IsolateTracker::addFakeRunIfNecessary): Call
RenderBlockFlow::appendRunsForObject() to keep our MidpointState object in sync
when we pop out of the isolated object. However, we pass in a null run list,
because we don't want to append just yet (that happens when we process the
isolate).
(WebCore::InlineBidiResolver::appendRun): Update for new signature of
appendRunsForObject().
* rendering/RenderBlock.h:
(WebCore::RenderBlock::shouldSkipCreatingRunsForObject): Take a reference instead
of a pointer.
* rendering/RenderBlockFlow.h:
* rendering/RenderBlockLineLayout.cpp:
(WebCore::createRun): Ditto.
(WebCore::RenderBlockFlow::appendRunsForObject): Allow someone passing us a null
BidiRunList. In this case, we will keep the resolver's midpointState() up to date,
but won't actually emit any runs.
(WebCore::notifyResolverToResumeInIsolate): Renamed from setUp.
(WebCore::isolatedResolversMidpointState): Calculate the midpoint state for the
isolated resolver.
(WebCore::setUpResolverToResumeInIsolate): Call isolatedResolversMidpointState().
(WebCore::constructBidiRunsForSegment): Pass in the topResolver, which is
necessary for isolatedResolversMidpointState().
* rendering/line/BreakingContext.h:
(WebCore::checkMidpoints):

LayoutTests:

* fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt:
* fast/text/bidi-isolate-whitespace-collapse-expected.html: Added.
* fast/text/bidi-isolate-whitespace-collapse.html: Added.
* fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt:
* fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt
LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html [new file with mode: 0644]
LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html [new file with mode: 0644]
LayoutTests/fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt
LayoutTests/fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/platform/text/BidiResolver.h
Source/WebCore/rendering/InlineIterator.h
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderBlockFlow.h
Source/WebCore/rendering/RenderBlockLineLayout.cpp
Source/WebCore/rendering/line/BreakingContext.h

index 5149924..a9d1d3a 100644 (file)
@@ -1,3 +1,17 @@
+2015-07-10  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Bidi-Isolate inlines break layout with collapsed whitespace
+        https://bugs.webkit.org/show_bug.cgi?id=109624
+        <rdar://problem/21752834>
+
+        Reviewed by David Hyatt.
+
+        * fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt:
+        * fast/text/bidi-isolate-whitespace-collapse-expected.html: Added.
+        * fast/text/bidi-isolate-whitespace-collapse.html: Added.
+        * fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt:
+        * fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt:
+
 2015-07-10  Javier Fernandez  <jfernandez@igalia.com>
 
         [CSS Grid Layout] Grid item's auto-margins are not applied correctly
diff --git a/LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html b/LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html
new file mode 100644 (file)
index 0000000..7339a1f
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<style>
+body > div {
+       display: inline-block;
+       border: 1px solid black;
+       width: 400px;
+        font-size: 30px;
+}
+</style>
+<body>
+<p>This test makes sure that whitespace collapsing occurs correctly in the presence of bidi isolates.</p>
+<div id="control">
+<p>123
+ <span>456</span> 789</p>
+<p>123 <span>456</span> 789</p>
+<p>123
+ <span>4       56
+</span> 789</p>
+<p>123
+ <span>
+ 456
+ </span>
+ 789</p>
+<p>1  2  3
+ <span>
+ 4  5  6
+ </span>
+ 7    8    9    </p>
+<p>123
+ <span></span> 789</p>
+<p>123
+ <span>  </span> 789</p>
+</div>
+</body></html>
diff --git a/LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html b/LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html
new file mode 100644 (file)
index 0000000..a928af6
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<style>
+body > div {
+       display: inline-block;
+       border: 1px solid black;
+       width: 400px;
+        font-size: 30px;
+}
+</style>
+<body>
+<p>This test makes sure that whitespace collapsing occurs correctly in the presence of bidi isolates.</p>
+<div id="test">
+<p>123
+ <span style="unicode-bidi: -webkit-isolate">456</span> 789</p>
+<p>123 <span style="unicode-bidi: -webkit-isolate">456</span> 789</p>
+<p>123
+ <span style="unicode-bidi: -webkit-isolate">4       56
+</span> 789</p>
+<p>123
+ <span style="unicode-bidi: -webkit-isolate">
+ 456
+ </span>
+ 789</p>
+<p>1  2  3
+ <span style="unicode-bidi: -webkit-isolate">
+ 4  5  6
+ </span>
+ 7    8    9    </p>
+</p>
+<p>123
+ <span style="unicode-bidi: -webkit-isolate"></span> 789</p>
+<p>123
+ <span style="unicode-bidi: -webkit-isolate">  </span> 789</p>
+</div>
+</body></html>
index b9993a2..e3d147c 100644 (file)
@@ -1,3 +1,88 @@
+2015-07-10  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Bidi-Isolate inlines break layout with collapsed whitespace
+        https://bugs.webkit.org/show_bug.cgi?id=109624
+        <rdar://problem/21752834>
+
+        Reviewed by David Hyatt.
+
+        This patch changes the logic in constructBidiRunsForSegment() when it encounters an
+        isolate. It already has logic to create a BidiResolver for the isolated text;
+        however, that logic doesn't handle setting up the MidpointState at all.
+        Specifically, we can set the MidpointState's cursor to point to the context which
+        we can remember from addPlaceholderRunForIsolatedInline(). This information is
+        remembered in a HashMap in BidiResolver.
+
+        This patch is a partial port of Blink patch
+        https://src.chromium.org/viewvc/blink?view=rev&revision=159203
+
+        Here is some explanatory text regarding how we collapse spaces:
+
+        Collapsing whitespace happens in a series of phases. The first phase occurs when
+        we perform line breaking. Here, we keep track of sequences of whitespace which
+        should be collapsed, in the form of a vector of pairs of InlineIterators. We put
+        this knowledge into a MidpointState object.
+
+        Then, once we have a line, we run the bidi algorithm on the line (including the
+        whitespace). As output, the bidi algorithm calls the BidiResolver::appendRun()
+        callback with two InlineIterators each time it wants to create a run. Because
+        each renderer that we create has to be owned by exactly one DOM node,
+        BidiResolver::appendRun() iterates between its two InlineIterator arguments,
+        calling RenderBlockFlow::appendRunsForObject() on each interstitial DOM node.
+
+        This is the function where whitespace collapsing happens. The MidpointState object
+        keeps a cursor into its remembered whitespace sequences. Here, we simply make a
+        bidi run for each region in between adjacent whitespace pairs in the MidpointState
+        object. These bidi runs eventually get turned into leaf InlineBoxes.
+
+        The problem is that the BidiResolver::appendRun() callbacks don't occur in
+        string-order, but the Midpoint InlineIterator pairs are in string-order. In
+        particular, within a particular isolate, appendRun() gets called in string
+        order, but callbacks that occur for inner isolates are deferred. This means that
+        RenderBlockFlow::appendRunsForObject() gets confused when it looks for relevant
+        whitespace to skip.
+
+        Test: fast/text/bidi-isolate-whitespace-collapse.html
+
+        * platform/text/BidiResolver.h:
+        (WebCore::MidpointState::numMidpoints): Returning a const unsigned& is silly.
+        (WebCore::MidpointState::currentMidpoint): Ditto.
+        (WebCore::MidpointState::setCurrentMidpoint): The isolated MidpointState object
+        needs to be able to set its current midpoint to point to the first one inside
+        the isolate.
+        (WebCore::MidpointState::decrementNumMidpoints): Renamed from "decrease"
+        (WebCore::MidpointState::betweenMidpoints): This function is true iff
+        currentMidpoint() % 2. Instead of keeping a member variable, we can just compute
+        that.
+        (WebCore::MidpointState::reset): Deleted.
+        (WebCore::MidpointState::decreaseNumMidpoints): Deleted.
+        (WebCore::MidpointState::setBetweenMidpoints): Deleted.
+        * rendering/InlineIterator.h:
+        (WebCore::IsolateTracker::addFakeRunIfNecessary): Call
+        RenderBlockFlow::appendRunsForObject() to keep our MidpointState object in sync
+        when we pop out of the isolated object. However, we pass in a null run list,
+        because we don't want to append just yet (that happens when we process the
+        isolate).
+        (WebCore::InlineBidiResolver::appendRun): Update for new signature of
+        appendRunsForObject().
+        * rendering/RenderBlock.h:
+        (WebCore::RenderBlock::shouldSkipCreatingRunsForObject): Take a reference instead
+        of a pointer.
+        * rendering/RenderBlockFlow.h:
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::createRun): Ditto.
+        (WebCore::RenderBlockFlow::appendRunsForObject): Allow someone passing us a null
+        BidiRunList. In this case, we will keep the resolver's midpointState() up to date,
+        but won't actually emit any runs.
+        (WebCore::notifyResolverToResumeInIsolate): Renamed from setUp.
+        (WebCore::isolatedResolversMidpointState): Calculate the midpoint state for the
+        isolated resolver.
+        (WebCore::setUpResolverToResumeInIsolate): Call isolatedResolversMidpointState().
+        (WebCore::constructBidiRunsForSegment): Pass in the topResolver, which is
+        necessary for isolatedResolversMidpointState().
+        * rendering/line/BreakingContext.h:
+        (WebCore::checkMidpoints):
+
 2015-07-10  Daniel Bates  <dabates@apple.com>
 
         Cleanup: WebCore::Pair class should use RefPtr&& instead of PassRefPtr
index e3ce749..3735290 100644 (file)
@@ -25,6 +25,7 @@
 #include "BidiContext.h"
 #include "BidiRunList.h"
 #include "WritingMode.h"
+#include <wtf/HashMap.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/Vector.h>
@@ -44,7 +45,6 @@ public:
     {
         m_numMidpoints = 0;
         m_currentMidpoint = 0;
-        m_betweenMidpoints = false;
     }
     
     void startIgnoringSpaces(const Iterator& midpoint)
@@ -69,12 +69,12 @@ public:
     }
 
     Vector<Iterator>& midpoints() { return m_midpoints; }
-    const unsigned& numMidpoints() const { return m_numMidpoints; }
-    const unsigned& currentMidpoint() const { return m_currentMidpoint; }
+    unsigned numMidpoints() const { return m_numMidpoints; }
+    unsigned currentMidpoint() const { return m_currentMidpoint; }
+    void setCurrentMidpoint(unsigned currentMidpoint) { m_currentMidpoint = currentMidpoint; }
     void incrementCurrentMidpoint() { ++m_currentMidpoint; }
-    void decreaseNumMidpoints() { --m_numMidpoints; }
-    const bool& betweenMidpoints() const { return m_betweenMidpoints; }
-    void setBetweenMidpoints(bool betweenMidpoint) { m_betweenMidpoints = betweenMidpoint; }
+    void decrementNumMidpoints() { --m_numMidpoints; }
+    bool betweenMidpoints() const { return m_currentMidpoint % 2; }
 private:
     // The goal is to reuse the line state across multiple
     // lines so we just keep an array around for midpoints and never clear it across multiple
@@ -82,7 +82,6 @@ private:
     Vector<Iterator> m_midpoints;
     unsigned m_numMidpoints;
     unsigned m_currentMidpoint;
-    bool m_betweenMidpoints;
 
     void addMidpoint(const Iterator& midpoint)
     {
@@ -263,6 +262,8 @@ public:
     void markCurrentRunEmpty() { m_emptyRun = true; }
 
     Vector<Run*>& isolatedRuns() { return m_isolatedRuns; }
+    void setMidpointForIsolatedRun(Run*, unsigned);
+    unsigned midpointForIsolatedRun(Run*);
 
 protected:
     // FIXME: Instead of InlineBidiResolvers subclassing this method, we should
@@ -290,6 +291,7 @@ protected:
 
     unsigned m_nestedIsolateCount;
     Vector<Run*> m_isolatedRuns;
+    HashMap<Run*, unsigned> m_midpointForIsolatedRun;
 
 private:
     void raiseExplicitEmbeddingLevel(UCharDirection from, UCharDirection to);
@@ -954,6 +956,19 @@ void BidiResolver<Iterator, Run>::createBidiRunsForLine(const Iterator& end, Vis
     endOfLine = Iterator();
 }
 
+template <class Iterator, class Run>
+void BidiResolver<Iterator, Run>::setMidpointForIsolatedRun(Run* run, unsigned midpoint)
+{
+    ASSERT(!m_midpointForIsolatedRun.contains(run));
+    m_midpointForIsolatedRun.add(run, midpoint);
+}
+
+template<class Iterator, class Run>
+unsigned BidiResolver<Iterator, Run>::midpointForIsolatedRun(Run* run)
+{
+    return m_midpointForIsolatedRun.take(run);
+}
+
 } // namespace WebCore
 
 #endif // BidiResolver_h
index a9efe0d..b474b3e 100644 (file)
@@ -482,6 +482,7 @@ static inline void addPlaceholderRunForIsolatedInline(InlineBidiResolver& resolv
     // FIXME: isolatedRuns() could be a hash of object->run and then we could cheaply
     // ASSERT here that we didn't create multiple objects for the same inline.
     resolver.isolatedRuns().append(isolatedRun);
+    resolver.setMidpointForIsolatedRun(isolatedRun, resolver.midpointState().currentMidpoint());
 }
 
 class IsolateTracker {
@@ -506,26 +507,21 @@ public:
     void embed(UCharDirection, BidiEmbeddingSource) { }
     void commitExplicitEmbedding() { }
 
-    void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, InlineBidiResolver& resolver)
+    void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, unsigned end, InlineBidiResolver& resolver)
     {
         // We only need to add a fake run for a given isolated span once during each call to createBidiRunsForLine.
         // We'll be called for every span inside the isolated span so we just ignore subsequent calls.
         // We also avoid creating a fake run until we hit a child that warrants one, e.g. we skip floats.
-        if (m_haveAddedFakeRunForRootIsolate || RenderBlock::shouldSkipCreatingRunsForObject(&obj))
+        if (RenderBlock::shouldSkipCreatingRunsForObject(obj))
             return;
-        m_haveAddedFakeRunForRootIsolate = true;
-        // obj and pos together denote a single position in the inline, from which the parsing of the isolate will start.
-        // We don't need to mark the end of the run because this is implicit: it is either endOfLine or the end of the
-        // isolate, when we call createBidiRunsForLine it will stop at whichever comes first.
-        addPlaceholderRunForIsolatedInline(resolver, obj, pos);
-        // FIXME: Inline isolates don't work properly with collapsing whitespace, see webkit.org/b/109624
-        // For now, if we enter an isolate between midpoints, we increment our current midpoint or else
-        // we'll leave the isolate and ignore the content that follows.
-        MidpointState<InlineIterator>& midpointState = resolver.midpointState();
-        if (midpointState.betweenMidpoints() && midpointState.midpoints()[midpointState.currentMidpoint()].renderer() == &obj) {
-            midpointState.setBetweenMidpoints(false);
-            midpointState.incrementCurrentMidpoint();
+        if (!m_haveAddedFakeRunForRootIsolate) {
+            // obj and pos together denote a single position in the inline, from which the parsing of the isolate will start.
+            // We don't need to mark the end of the run because this is implicit: it is either endOfLine or the end of the
+            // isolate, when we call createBidiRunsForLine it will stop at whichever comes first.
+            addPlaceholderRunForIsolatedInline(resolver, obj, pos);
         }
+        m_haveAddedFakeRunForRootIsolate = true;
+        RenderBlockFlow::appendRunsForObject(nullptr, pos, end, obj, resolver);
     }
 
 private:
@@ -545,9 +541,9 @@ inline void InlineBidiResolver::appendRun()
         RenderObject* obj = m_sor.renderer();
         while (obj && obj != m_eor.renderer() && obj != endOfLine.renderer()) {
             if (isolateTracker.inIsolate())
-                isolateTracker.addFakeRunIfNecessary(*obj, start, *this);
+                isolateTracker.addFakeRunIfNecessary(*obj, start, obj->length(), *this);
             else
-                RenderBlockFlow::appendRunsForObject(m_runs, start, obj->length(), obj, *this);
+                RenderBlockFlow::appendRunsForObject(&m_runs, start, obj->length(), *obj, *this);
             // FIXME: start/obj should be an InlineIterator instead of two separate variables.
             start = 0;
             obj = bidiNextSkippingEmptyInlines(*m_sor.root(), obj, &isolateTracker);
@@ -561,9 +557,9 @@ inline void InlineBidiResolver::appendRun()
             // It's OK to add runs for zero-length RenderObjects, just don't make the run larger than it should be
             int end = obj->length() ? pos + 1 : 0;
             if (isolateTracker.inIsolate())
-                isolateTracker.addFakeRunIfNecessary(*obj, start, *this);
+                isolateTracker.addFakeRunIfNecessary(*obj, start, obj->length(), *this);
             else
-                RenderBlockFlow::appendRunsForObject(m_runs, start, end, obj, *this);
+                RenderBlockFlow::appendRunsForObject(&m_runs, start, end, *obj, *this);
         }
 
         m_eor.increment();
index 6794575..e2eec2e 100644 (file)
@@ -199,9 +199,9 @@ public:
 
     virtual RenderBox* createAnonymousBoxWithSameTypeAs(const RenderObject* parent) const override;
 
-    static bool shouldSkipCreatingRunsForObject(RenderObject* obj)
+    static bool shouldSkipCreatingRunsForObject(RenderObject& obj)
     {
-        return obj->isFloating() || (obj->isOutOfFlowPositioned() && !obj->style().isOriginalDisplayInlineType() && !obj->container()->isRenderInline());
+        return obj.isFloating() || (obj.isOutOfFlowPositioned() && !obj.style().isOriginalDisplayInlineType() && !obj.container()->isRenderInline());
     }
 
     static TextRun constructTextRun(RenderObject* context, const FontCascade&, StringView, const RenderStyle&,
index 04289b1..9edcf72 100644 (file)
@@ -537,7 +537,7 @@ private:
 // line layout code is separated from RenderBlock and RenderBlockFlow.
 // START METHODS DEFINED IN RenderBlockLineLayout
 public:
-    static void appendRunsForObject(BidiRunList<BidiRun>&, int start, int end, RenderObject*, InlineBidiResolver&);
+    static void appendRunsForObject(BidiRunList<BidiRun>*, int start, int end, RenderObject&, InlineBidiResolver&);
     RootInlineBox* createAndAppendRootInlineBox();
 
     LayoutUnit startAlignedOffsetForLine(LayoutUnit position, bool shouldIndentText);
index 5ec0470..6ffab49 100644 (file)
@@ -71,13 +71,12 @@ static void determineDirectionality(TextDirection& dir, InlineIterator iter)
     }
 }
 
-inline BidiRun* createRun(int start, int end, RenderObject* obj, InlineBidiResolver& resolver)
+inline BidiRun* createRun(int start, int end, RenderObject& obj, InlineBidiResolver& resolver)
 {
-    ASSERT(obj);
-    return new BidiRun(start, end, *obj, resolver.context(), resolver.dir());
+    return new BidiRun(start, end, obj, resolver.context(), resolver.dir());
 }
 
-void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>& runs, int start, int end, RenderObject* obj, InlineBidiResolver& resolver)
+void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>* runs, int start, int end, RenderObject& obj, InlineBidiResolver& resolver)
 {
     if (start > end || shouldSkipCreatingRunsForObject(obj))
         return;
@@ -88,33 +87,34 @@ void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>& runs, int start,
     if (haveNextMidpoint)
         nextMidpoint = lineMidpointState.midpoints()[lineMidpointState.currentMidpoint()];
     if (lineMidpointState.betweenMidpoints()) {
-        if (!(haveNextMidpoint && nextMidpoint.renderer() == obj))
+        if (!haveNextMidpoint || (&obj != nextMidpoint.renderer()))
             return;
         // This is a new start point. Stop ignoring objects and
         // adjust our start.
-        lineMidpointState.setBetweenMidpoints(false);
         start = nextMidpoint.offset();
         lineMidpointState.incrementCurrentMidpoint();
-        if (start < end)
-            return appendRunsForObject(runs, start, end, obj, resolver);
+        if (start < end) {
+            appendRunsForObject(runs, start, end, obj, resolver);
+            return;
+        }
     } else {
-        if (!haveNextMidpoint || (obj != nextMidpoint.renderer())) {
-            runs.addRun(createRun(start, end, obj, resolver));
+        if (!haveNextMidpoint || (&obj != nextMidpoint.renderer())) {
+            if (runs)
+                runs->addRun(createRun(start, end, obj, resolver));
             return;
         }
 
         // An end midpoint has been encountered within our object. We need to append a run with our endpoint.
         if (static_cast<int>(nextMidpoint.offset() + 1) <= end) {
-            lineMidpointState.setBetweenMidpoints(true);
             lineMidpointState.incrementCurrentMidpoint();
             // The end of the line is before the object we're inspecting. Skip everything and return
             if (nextMidpoint.refersToEndOfPreviousNode())
                 return;
-            if (static_cast<int>(nextMidpoint.offset() + 1) > start)
-                runs.addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));
+            if (static_cast<int>(nextMidpoint.offset() + 1) > start && runs)
+                runs->addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));
             appendRunsForObject(runs, nextMidpoint.offset() + 1, end, obj, resolver);
-        } else
-           runs.addRun(createRun(start, end, obj, resolver));
+        } else if (runs)
+            runs->addRun(createRun(start, end, obj, resolver));
     }
 }
 
@@ -1011,15 +1011,25 @@ void RenderBlockFlow::appendFloatingObjectToLastLine(FloatingObject* floatingObj
     lastRootBox()->appendFloat(floatingObject->renderer());
 }
 
-static inline void setUpResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
+static inline void notifyResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
 {
     if (root != startObject) {
         RenderObject* parent = startObject->parent();
-        setUpResolverToResumeInIsolate(resolver, root, parent);
+        notifyResolverToResumeInIsolate(resolver, root, parent);
         notifyObserverEnteredObject(&resolver, startObject);
     }
 }
 
+static inline void setUpResolverToResumeInIsolate(InlineBidiResolver& resolver, InlineBidiResolver& topResolver, BidiRun* isolatedRun, RenderObject* root, RenderObject* startObject)
+{
+    // Set up m_midpointState
+    resolver.midpointState() = topResolver.midpointState();
+    resolver.midpointState().setCurrentMidpoint(topResolver.midpointForIsolatedRun(isolatedRun));
+
+    // Set up m_nestedIsolateCount
+    notifyResolverToResumeInIsolate(resolver, root, startObject);
+}
+
 // FIXME: BidiResolver should have this logic.
 static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver, BidiRunList<BidiRun>& bidiRuns, const InlineIterator& endOfRuns, VisualDirectionOverride override, bool previousLineBrokeCleanly)
 {
@@ -1056,7 +1066,7 @@ static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver,
         }
         isolatedResolver.setStatus(BidiStatus(direction, isOverride(unicodeBidi)));
 
-        setUpResolverToResumeInIsolate(isolatedResolver, isolatedInline, &startObject);
+        setUpResolverToResumeInIsolate(isolatedResolver, topResolver, isolatedRun, isolatedInline, &startObject);
 
         // The starting position is the beginning of the first run within the isolate that was identified
         // during the earlier call to createBidiRunsForLine. This can be but is not necessarily the
@@ -1079,6 +1089,8 @@ static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver,
         // to the top resolver's list for later processing.
         if (!isolatedResolver.isolatedRuns().isEmpty()) {
             topResolver.isolatedRuns().appendVector(isolatedResolver.isolatedRuns());
+            for (auto* run : isolatedResolver.isolatedRuns())
+                topResolver.setMidpointForIsolatedRun(run, isolatedResolver.midpointForIsolatedRun(run));
             isolatedResolver.isolatedRuns().clear();
             currentRoot = isolatedInline;
         }
index e55133c..802ee43 100644 (file)
@@ -971,7 +971,7 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool
             lastSpaceWordSpacing = applyWordSpacing ? wordSpacing : 0;
             wordSpacingForWordMeasurement = (applyWordSpacing && wordMeasurements.last().width) ? wordSpacing : 0;
             lastSpace = m_current.offset(); // e.g., "Foo    goo", don't add in any of the ignored spaces.
-            m_lineMidpointState.stopIgnoringSpaces(InlineIterator(0, m_current.renderer(), m_current.offset()));
+            m_lineMidpointState.stopIgnoringSpaces(InlineIterator(nullptr, m_current.renderer(), m_current.offset()));
         }
 
         if (isSVGText && m_current.offset()) {
@@ -1147,7 +1147,7 @@ inline TrailingObjects::CollapseFirstSpaceOrNot checkMidpoints(LineMidpointState
             currpoint.increment();
         if (currpoint == lBreak) {
             // We hit the line break before the start point. Shave off the start point.
-            lineMidpointState.decreaseNumMidpoints();
+            lineMidpointState.decrementNumMidpoints();
             if (endpoint.renderer()->style().collapseWhiteSpace() && endpoint.renderer()->isText()) {
                 endpoint.fastDecrement();
                 return TrailingObjects::DoNotCollapseFirstSpace;