Skip to content

GitLab

T
tangled
Commit 9c64e0ee authored by kojii@chromium.org's avatar kojii@chromium.org
Browse files
Options

Fix nested 'unicode-bidi: isolate' can cause infinite loop 

This patch fixes constructBidiRunsForLine() to handle the nested
'unicode-bidi: isolate' runs with the correct containing isolate.

crbug.com/274717 fixed the nested case by updating currentRoot to the
root of the last nested runs. While this fixed simple cases, it does
not set the correct root when nested in multiple levels.

The wrong root can let highestContainingIsolateWithinRoot() to find
ancestors up to the root elements. This will find ancestors that were
already processed, and results in an infinite loop.

BUG=520282

Review URL: https://codereview.chromium.org/1328633002

git-svn-id: svn://svn.chromium.org/blink/trunk@201847 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent de9bb114
Hide whitespace changes
Inline Side-by-side
Showing with 39 additions and 7 deletions
third_party/WebKit/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-crash.html 0 → 100644
View file @ 9c64e0ee
<!DOCTYPE html>
<style>
.isolate {
unicode-bidi: -webkit-isolate;
unicode-bidi: isolate;
}
</style>
<span class="isolate">
<span class="isolate">a</span>
<span class="isolate">
<span class="isolate">b</span>
</span>
</span>
third_party/WebKit/Source/core/layout/BidiRunForLine.cpp
View file @ 9c64e0ee
...@@ -137,11 +137,25 @@ void constructBidiRunsForLine(InlineBidiResolver& topResolver, ...@@ -137,11 +137,25 @@ void constructBidiRunsForLine(InlineBidiResolver& topResolver,
// of the resolver owning the runs. // of the resolver owning the runs.
ASSERT(&topResolver.runs() == &bidiRuns); ASSERT(&topResolver.runs() == &bidiRuns);
ASSERT(topResolver.position() != endOfLine); ASSERT(topResolver.position() != endOfLine);
LayoutObject* currentRoot = topResolver.position().root(); const LayoutObject* currentRoot = topResolver.position().root();
topResolver.createBidiRunsForLine(endOfLine, override, topResolver.createBidiRunsForLine(endOfLine, override,
previousLineBrokeCleanly); previousLineBrokeCleanly);
struct BidiRunsWithRoot {
const LayoutObject* root;
Vector<BidiRun*> isolatedRuns;
};
Vector<BidiRunsWithRoot> isolatedRunsStack;
while (true) {
if (topResolver.isolatedRuns().isEmpty()) {
if (isolatedRunsStack.isEmpty())
break;
topResolver.isolatedRuns().appendVector(isolatedRunsStack.last().isolatedRuns);
ASSERT(!topResolver.isolatedRuns().isEmpty());
currentRoot = isolatedRunsStack.last().root;
isolatedRunsStack.removeLast();
}
while (!topResolver.isolatedRuns().isEmpty()) {
// It does not matter which order we resolve the runs as long as we // It does not matter which order we resolve the runs as long as we
// resolve them all. // resolve them all.
BidiRun* isolatedRun = topResolver.isolatedRuns().last(); BidiRun* isolatedRun = topResolver.isolatedRuns().last();
...@@ -157,7 +171,8 @@ void constructBidiRunsForLine(InlineBidiResolver& topResolver, ...@@ -157,7 +171,8 @@ void constructBidiRunsForLine(InlineBidiResolver& topResolver,
// but that would be a layering violation for BidiResolver (which knows // but that would be a layering violation for BidiResolver (which knows
// nothing about LayoutObject). // nothing about LayoutObject).
LayoutInline* isolatedInline = toLayoutInline( LayoutInline* isolatedInline = toLayoutInline(
highestContainingIsolateWithinRoot(LineLayoutItem(startObj), LineLayoutItem(currentRoot))); highestContainingIsolateWithinRoot(LineLayoutItem(startObj),
LineLayoutItem(const_cast<LayoutObject*>(currentRoot))));
ASSERT(isolatedInline); ASSERT(isolatedInline);
InlineBidiResolver isolatedResolver; InlineBidiResolver isolatedResolver;
...@@ -199,12 +214,13 @@ void constructBidiRunsForLine(InlineBidiResolver& topResolver, ...@@ -199,12 +214,13 @@ void constructBidiRunsForLine(InlineBidiResolver& topResolver,
if (isolatedResolver.runs().runCount()) if (isolatedResolver.runs().runCount())
bidiRuns.replaceRunWithRuns(isolatedRun, isolatedResolver.runs()); bidiRuns.replaceRunWithRuns(isolatedRun, isolatedResolver.runs());
// If we encountered any nested isolate runs, just move them // If we encountered any nested isolate runs, save them for later
// to the top resolver's list for later processing. // processing.
if (!isolatedResolver.isolatedRuns().isEmpty()) { if (!isolatedResolver.isolatedRuns().isEmpty()) {
topResolver.isolatedRuns().appendVector( isolatedRunsStack.resize(isolatedRunsStack.size() + 1);
isolatedRunsStack.last().isolatedRuns.appendVector(
isolatedResolver.isolatedRuns()); isolatedResolver.isolatedRuns());
currentRoot = isolatedInline; isolatedRunsStack.last().root = isolatedInline;
restoreIsolatedMidpointStates(topResolver, isolatedResolver); restoreIsolatedMidpointStates(topResolver, isolatedResolver);
} }
} }
......
third_party/WebKit/Source/core/layout/line/InlineIterator.h
View file @ 9c64e0ee
...@@ -532,6 +532,7 @@ static inline LineLayoutItem highestContainingIsolateWithinRoot(LineLayoutItem o ...@@ -532,6 +532,7 @@ static inline LineLayoutItem highestContainingIsolateWithinRoot(LineLayoutItem o
containingIsolateObj = object; containingIsolateObj = object;
object = object.parent(); object = object.parent();
ASSERT(object);
} }
return containingIsolateObj; return containingIsolateObj;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment