<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[186686] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/186686">186686</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2015-07-10 13:39:16 -0700 (Fri, 10 Jul 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Bidi-Isolate inlines break layout with collapsed whitespace
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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastinlinecrashwhenchildrendererisremovedandlinestayscleanexpectedtxt">trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasttextinternationalembedbidistyleinisolatecrashexpectedtxt">trunk/LayoutTests/fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfasttextremovetextnodelineboxnotdirtycrashexpectedtxt">trunk/LayoutTests/fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformtextBidiResolverh">trunk/Source/WebCore/platform/text/BidiResolver.h</a></li>
<li><a href="#trunkSourceWebCorerenderingInlineIteratorh">trunk/Source/WebCore/rendering/InlineIterator.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBlockh">trunk/Source/WebCore/rendering/RenderBlock.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBlockFlowh">trunk/Source/WebCore/rendering/RenderBlockFlow.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderBlockLineLayoutcpp">trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderinglineBreakingContexth">trunk/Source/WebCore/rendering/line/BreakingContext.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttextbidiisolatewhitespacecollapseexpectedhtml">trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html</a></li>
<li><a href="#trunkLayoutTestsfasttextbidiisolatewhitespacecollapsehtml">trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/LayoutTests/ChangeLog 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+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:
+
</ins><span class="cx"> 2015-07-10 Javier Fernandez <jfernandez@igalia.com>
</span><span class="cx">
</span><span class="cx"> [CSS Grid Layout] Grid item's auto-margins are not applied correctly
</span></span></pre></div>
<a id="trunkLayoutTestsfastinlinecrashwhenchildrendererisremovedandlinestayscleanexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -1,4 +1,4 @@
</span><del>-Pass if no crash or assert in Debug. bar
</del><ins>+Pass if no crash or assert in Debug. bar
</ins><span class="cx">
</span><span class="cx">
</span><span class="cx">
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextbidiisolatewhitespacecollapseexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html (0 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse-expected.html 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -0,0 +1,35 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextbidiisolatewhitespacecollapsehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html (0 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html (rev 0)
+++ trunk/LayoutTests/fast/text/bidi-isolate-whitespace-collapse.html 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -0,0 +1,36 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextinternationalembedbidistyleinisolatecrashexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/LayoutTests/fast/text/international/embed-bidi-style-in-isolate-crash-expected.txt 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -1 +1 @@
</span><del>-PASS, if no exception or crash in debug
</del><ins>+PASS, if no exception or crash in debug
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextremovetextnodelineboxnotdirtycrashexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/LayoutTests/fast/text/remove-text-node-linebox-not-dirty-crash-expected.txt 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -1,3 +1,3 @@
</span><del>-This test passes if it does not crash. bar
</del><ins>+This test passes if it does not crash. bar
</ins><span class="cx">
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/ChangeLog 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -1,3 +1,88 @@
</span><ins>+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):
+
</ins><span class="cx"> 2015-07-10 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><span class="cx"> Cleanup: WebCore::Pair class should use RefPtr&& instead of PassRefPtr
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformtextBidiResolverh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/text/BidiResolver.h (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/text/BidiResolver.h 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/platform/text/BidiResolver.h 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -25,6 +25,7 @@
</span><span class="cx"> #include "BidiContext.h"
</span><span class="cx"> #include "BidiRunList.h"
</span><span class="cx"> #include "WritingMode.h"
</span><ins>+#include <wtf/HashMap.h>
</ins><span class="cx"> #include <wtf/Noncopyable.h>
</span><span class="cx"> #include <wtf/PassRefPtr.h>
</span><span class="cx"> #include <wtf/Vector.h>
</span><span class="lines">@@ -44,7 +45,6 @@
</span><span class="cx"> {
</span><span class="cx"> m_numMidpoints = 0;
</span><span class="cx"> m_currentMidpoint = 0;
</span><del>- m_betweenMidpoints = false;
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void startIgnoringSpaces(const Iterator& midpoint)
</span><span class="lines">@@ -69,12 +69,12 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> Vector<Iterator>& midpoints() { return m_midpoints; }
</span><del>- const unsigned& numMidpoints() const { return m_numMidpoints; }
- const unsigned& currentMidpoint() const { return m_currentMidpoint; }
</del><ins>+ unsigned numMidpoints() const { return m_numMidpoints; }
+ unsigned currentMidpoint() const { return m_currentMidpoint; }
+ void setCurrentMidpoint(unsigned currentMidpoint) { m_currentMidpoint = currentMidpoint; }
</ins><span class="cx"> void incrementCurrentMidpoint() { ++m_currentMidpoint; }
</span><del>- void decreaseNumMidpoints() { --m_numMidpoints; }
- const bool& betweenMidpoints() const { return m_betweenMidpoints; }
- void setBetweenMidpoints(bool betweenMidpoint) { m_betweenMidpoints = betweenMidpoint; }
</del><ins>+ void decrementNumMidpoints() { --m_numMidpoints; }
+ bool betweenMidpoints() const { return m_currentMidpoint % 2; }
</ins><span class="cx"> private:
</span><span class="cx"> // The goal is to reuse the line state across multiple
</span><span class="cx"> // lines so we just keep an array around for midpoints and never clear it across multiple
</span><span class="lines">@@ -82,7 +82,6 @@
</span><span class="cx"> Vector<Iterator> m_midpoints;
</span><span class="cx"> unsigned m_numMidpoints;
</span><span class="cx"> unsigned m_currentMidpoint;
</span><del>- bool m_betweenMidpoints;
</del><span class="cx">
</span><span class="cx"> void addMidpoint(const Iterator& midpoint)
</span><span class="cx"> {
</span><span class="lines">@@ -263,6 +262,8 @@
</span><span class="cx"> void markCurrentRunEmpty() { m_emptyRun = true; }
</span><span class="cx">
</span><span class="cx"> Vector<Run*>& isolatedRuns() { return m_isolatedRuns; }
</span><ins>+ void setMidpointForIsolatedRun(Run*, unsigned);
+ unsigned midpointForIsolatedRun(Run*);
</ins><span class="cx">
</span><span class="cx"> protected:
</span><span class="cx"> // FIXME: Instead of InlineBidiResolvers subclassing this method, we should
</span><span class="lines">@@ -290,6 +291,7 @@
</span><span class="cx">
</span><span class="cx"> unsigned m_nestedIsolateCount;
</span><span class="cx"> Vector<Run*> m_isolatedRuns;
</span><ins>+ HashMap<Run*, unsigned> m_midpointForIsolatedRun;
</ins><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> void raiseExplicitEmbeddingLevel(UCharDirection from, UCharDirection to);
</span><span class="lines">@@ -954,6 +956,19 @@
</span><span class="cx"> endOfLine = Iterator();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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);
+}
+
</ins><span class="cx"> } // namespace WebCore
</span><span class="cx">
</span><span class="cx"> #endif // BidiResolver_h
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingInlineIteratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/InlineIterator.h (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/InlineIterator.h 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/rendering/InlineIterator.h 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -482,6 +482,7 @@
</span><span class="cx"> // FIXME: isolatedRuns() could be a hash of object->run and then we could cheaply
</span><span class="cx"> // ASSERT here that we didn't create multiple objects for the same inline.
</span><span class="cx"> resolver.isolatedRuns().append(isolatedRun);
</span><ins>+ resolver.setMidpointForIsolatedRun(isolatedRun, resolver.midpointState().currentMidpoint());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> class IsolateTracker {
</span><span class="lines">@@ -506,26 +507,21 @@
</span><span class="cx"> void embed(UCharDirection, BidiEmbeddingSource) { }
</span><span class="cx"> void commitExplicitEmbedding() { }
</span><span class="cx">
</span><del>- void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, InlineBidiResolver& resolver)
</del><ins>+ void addFakeRunIfNecessary(RenderObject& obj, unsigned pos, unsigned end, InlineBidiResolver& resolver)
</ins><span class="cx"> {
</span><span class="cx"> // We only need to add a fake run for a given isolated span once during each call to createBidiRunsForLine.
</span><span class="cx"> // We'll be called for every span inside the isolated span so we just ignore subsequent calls.
</span><span class="cx"> // We also avoid creating a fake run until we hit a child that warrants one, e.g. we skip floats.
</span><del>- if (m_haveAddedFakeRunForRootIsolate || RenderBlock::shouldSkipCreatingRunsForObject(&obj))
</del><ins>+ if (RenderBlock::shouldSkipCreatingRunsForObject(obj))
</ins><span class="cx"> return;
</span><del>- 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();
</del><ins>+ 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);
</ins><span class="cx"> }
</span><ins>+ m_haveAddedFakeRunForRootIsolate = true;
+ RenderBlockFlow::appendRunsForObject(nullptr, pos, end, obj, resolver);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="lines">@@ -545,9 +541,9 @@
</span><span class="cx"> RenderObject* obj = m_sor.renderer();
</span><span class="cx"> while (obj && obj != m_eor.renderer() && obj != endOfLine.renderer()) {
</span><span class="cx"> if (isolateTracker.inIsolate())
</span><del>- isolateTracker.addFakeRunIfNecessary(*obj, start, *this);
</del><ins>+ isolateTracker.addFakeRunIfNecessary(*obj, start, obj->length(), *this);
</ins><span class="cx"> else
</span><del>- RenderBlockFlow::appendRunsForObject(m_runs, start, obj->length(), obj, *this);
</del><ins>+ RenderBlockFlow::appendRunsForObject(&m_runs, start, obj->length(), *obj, *this);
</ins><span class="cx"> // FIXME: start/obj should be an InlineIterator instead of two separate variables.
</span><span class="cx"> start = 0;
</span><span class="cx"> obj = bidiNextSkippingEmptyInlines(*m_sor.root(), obj, &isolateTracker);
</span><span class="lines">@@ -561,9 +557,9 @@
</span><span class="cx"> // It's OK to add runs for zero-length RenderObjects, just don't make the run larger than it should be
</span><span class="cx"> int end = obj->length() ? pos + 1 : 0;
</span><span class="cx"> if (isolateTracker.inIsolate())
</span><del>- isolateTracker.addFakeRunIfNecessary(*obj, start, *this);
</del><ins>+ isolateTracker.addFakeRunIfNecessary(*obj, start, obj->length(), *this);
</ins><span class="cx"> else
</span><del>- RenderBlockFlow::appendRunsForObject(m_runs, start, end, obj, *this);
</del><ins>+ RenderBlockFlow::appendRunsForObject(&m_runs, start, end, *obj, *this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> m_eor.increment();
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBlock.h (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderBlock.h 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -199,9 +199,9 @@
</span><span class="cx">
</span><span class="cx"> virtual RenderBox* createAnonymousBoxWithSameTypeAs(const RenderObject* parent) const override;
</span><span class="cx">
</span><del>- static bool shouldSkipCreatingRunsForObject(RenderObject* obj)
</del><ins>+ static bool shouldSkipCreatingRunsForObject(RenderObject& obj)
</ins><span class="cx"> {
</span><del>- return obj->isFloating() || (obj->isOutOfFlowPositioned() && !obj->style().isOriginalDisplayInlineType() && !obj->container()->isRenderInline());
</del><ins>+ return obj.isFloating() || (obj.isOutOfFlowPositioned() && !obj.style().isOriginalDisplayInlineType() && !obj.container()->isRenderInline());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static TextRun constructTextRun(RenderObject* context, const FontCascade&, StringView, const RenderStyle&,
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBlockFlowh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderBlockFlow.h 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -537,7 +537,7 @@
</span><span class="cx"> // line layout code is separated from RenderBlock and RenderBlockFlow.
</span><span class="cx"> // START METHODS DEFINED IN RenderBlockLineLayout
</span><span class="cx"> public:
</span><del>- static void appendRunsForObject(BidiRunList<BidiRun>&, int start, int end, RenderObject*, InlineBidiResolver&);
</del><ins>+ static void appendRunsForObject(BidiRunList<BidiRun>*, int start, int end, RenderObject&, InlineBidiResolver&);
</ins><span class="cx"> RootInlineBox* createAndAppendRootInlineBox();
</span><span class="cx">
</span><span class="cx"> LayoutUnit startAlignedOffsetForLine(LayoutUnit position, bool shouldIndentText);
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderBlockLineLayoutcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -71,13 +71,12 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline BidiRun* createRun(int start, int end, RenderObject* obj, InlineBidiResolver& resolver)
</del><ins>+inline BidiRun* createRun(int start, int end, RenderObject& obj, InlineBidiResolver& resolver)
</ins><span class="cx"> {
</span><del>- ASSERT(obj);
- return new BidiRun(start, end, *obj, resolver.context(), resolver.dir());
</del><ins>+ return new BidiRun(start, end, obj, resolver.context(), resolver.dir());
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>& runs, int start, int end, RenderObject* obj, InlineBidiResolver& resolver)
</del><ins>+void RenderBlockFlow::appendRunsForObject(BidiRunList<BidiRun>* runs, int start, int end, RenderObject& obj, InlineBidiResolver& resolver)
</ins><span class="cx"> {
</span><span class="cx"> if (start > end || shouldSkipCreatingRunsForObject(obj))
</span><span class="cx"> return;
</span><span class="lines">@@ -88,33 +87,34 @@
</span><span class="cx"> if (haveNextMidpoint)
</span><span class="cx"> nextMidpoint = lineMidpointState.midpoints()[lineMidpointState.currentMidpoint()];
</span><span class="cx"> if (lineMidpointState.betweenMidpoints()) {
</span><del>- if (!(haveNextMidpoint && nextMidpoint.renderer() == obj))
</del><ins>+ if (!haveNextMidpoint || (&obj != nextMidpoint.renderer()))
</ins><span class="cx"> return;
</span><span class="cx"> // This is a new start point. Stop ignoring objects and
</span><span class="cx"> // adjust our start.
</span><del>- lineMidpointState.setBetweenMidpoints(false);
</del><span class="cx"> start = nextMidpoint.offset();
</span><span class="cx"> lineMidpointState.incrementCurrentMidpoint();
</span><del>- if (start < end)
- return appendRunsForObject(runs, start, end, obj, resolver);
</del><ins>+ if (start < end) {
+ appendRunsForObject(runs, start, end, obj, resolver);
+ return;
+ }
</ins><span class="cx"> } else {
</span><del>- if (!haveNextMidpoint || (obj != nextMidpoint.renderer())) {
- runs.addRun(createRun(start, end, obj, resolver));
</del><ins>+ if (!haveNextMidpoint || (&obj != nextMidpoint.renderer())) {
+ if (runs)
+ runs->addRun(createRun(start, end, obj, resolver));
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // An end midpoint has been encountered within our object. We need to append a run with our endpoint.
</span><span class="cx"> if (static_cast<int>(nextMidpoint.offset() + 1) <= end) {
</span><del>- lineMidpointState.setBetweenMidpoints(true);
</del><span class="cx"> lineMidpointState.incrementCurrentMidpoint();
</span><span class="cx"> // The end of the line is before the object we're inspecting. Skip everything and return
</span><span class="cx"> if (nextMidpoint.refersToEndOfPreviousNode())
</span><span class="cx"> return;
</span><del>- if (static_cast<int>(nextMidpoint.offset() + 1) > start)
- runs.addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));
</del><ins>+ if (static_cast<int>(nextMidpoint.offset() + 1) > start && runs)
+ runs->addRun(createRun(start, nextMidpoint.offset() + 1, obj, resolver));
</ins><span class="cx"> appendRunsForObject(runs, nextMidpoint.offset() + 1, end, obj, resolver);
</span><del>- } else
- runs.addRun(createRun(start, end, obj, resolver));
</del><ins>+ } else if (runs)
+ runs->addRun(createRun(start, end, obj, resolver));
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1011,15 +1011,25 @@
</span><span class="cx"> lastRootBox()->appendFloat(floatingObject->renderer());
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void setUpResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
</del><ins>+static inline void notifyResolverToResumeInIsolate(InlineBidiResolver& resolver, RenderObject* root, RenderObject* startObject)
</ins><span class="cx"> {
</span><span class="cx"> if (root != startObject) {
</span><span class="cx"> RenderObject* parent = startObject->parent();
</span><del>- setUpResolverToResumeInIsolate(resolver, root, parent);
</del><ins>+ notifyResolverToResumeInIsolate(resolver, root, parent);
</ins><span class="cx"> notifyObserverEnteredObject(&resolver, startObject);
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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);
+}
+
</ins><span class="cx"> // FIXME: BidiResolver should have this logic.
</span><span class="cx"> static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver, BidiRunList<BidiRun>& bidiRuns, const InlineIterator& endOfRuns, VisualDirectionOverride override, bool previousLineBrokeCleanly)
</span><span class="cx"> {
</span><span class="lines">@@ -1056,7 +1066,7 @@
</span><span class="cx"> }
</span><span class="cx"> isolatedResolver.setStatus(BidiStatus(direction, isOverride(unicodeBidi)));
</span><span class="cx">
</span><del>- setUpResolverToResumeInIsolate(isolatedResolver, isolatedInline, &startObject);
</del><ins>+ setUpResolverToResumeInIsolate(isolatedResolver, topResolver, isolatedRun, isolatedInline, &startObject);
</ins><span class="cx">
</span><span class="cx"> // The starting position is the beginning of the first run within the isolate that was identified
</span><span class="cx"> // during the earlier call to createBidiRunsForLine. This can be but is not necessarily the
</span><span class="lines">@@ -1079,6 +1089,8 @@
</span><span class="cx"> // to the top resolver's list for later processing.
</span><span class="cx"> if (!isolatedResolver.isolatedRuns().isEmpty()) {
</span><span class="cx"> topResolver.isolatedRuns().appendVector(isolatedResolver.isolatedRuns());
</span><ins>+ for (auto* run : isolatedResolver.isolatedRuns())
+ topResolver.setMidpointForIsolatedRun(run, isolatedResolver.midpointForIsolatedRun(run));
</ins><span class="cx"> isolatedResolver.isolatedRuns().clear();
</span><span class="cx"> currentRoot = isolatedInline;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCorerenderinglineBreakingContexth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/line/BreakingContext.h (186685 => 186686)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/line/BreakingContext.h 2015-07-10 20:34:30 UTC (rev 186685)
+++ trunk/Source/WebCore/rendering/line/BreakingContext.h 2015-07-10 20:39:16 UTC (rev 186686)
</span><span class="lines">@@ -971,7 +971,7 @@
</span><span class="cx"> lastSpaceWordSpacing = applyWordSpacing ? wordSpacing : 0;
</span><span class="cx"> wordSpacingForWordMeasurement = (applyWordSpacing && wordMeasurements.last().width) ? wordSpacing : 0;
</span><span class="cx"> lastSpace = m_current.offset(); // e.g., "Foo goo", don't add in any of the ignored spaces.
</span><del>- m_lineMidpointState.stopIgnoringSpaces(InlineIterator(0, m_current.renderer(), m_current.offset()));
</del><ins>+ m_lineMidpointState.stopIgnoringSpaces(InlineIterator(nullptr, m_current.renderer(), m_current.offset()));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (isSVGText && m_current.offset()) {
</span><span class="lines">@@ -1147,7 +1147,7 @@
</span><span class="cx"> currpoint.increment();
</span><span class="cx"> if (currpoint == lBreak) {
</span><span class="cx"> // We hit the line break before the start point. Shave off the start point.
</span><del>- lineMidpointState.decreaseNumMidpoints();
</del><ins>+ lineMidpointState.decrementNumMidpoints();
</ins><span class="cx"> if (endpoint.renderer()->style().collapseWhiteSpace() && endpoint.renderer()->isText()) {
</span><span class="cx"> endpoint.fastDecrement();
</span><span class="cx"> return TrailingObjects::DoNotCollapseFirstSpace;
</span></span></pre>
</div>
</div>
</body>
</html>