<!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>[186984] 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/186984">186984</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2015-07-17 22:20:47 -0700 (Fri, 17 Jul 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/169105">r169105</a>): Do not assign a renderer to multiple selection subtrees.
https://bugs.webkit.org/show_bug.cgi?id=147038
rdar://problem/21819351
Reviewed by David Kilzer.
A renderer should never be assigned to multiple selection subtrees. (Currently RenderObject maintains the last selection state.)
RenderView::applySubtreeSelection() loops from the start to the end of the selection to find renderers that are inside the selection.
However, in case of regions (when multiple selection roots are present) traversing the renderer tree by calling RenderObject::nextInPreOrder() could
end up going across selection roots.
This patch ensures that we assign renderers to a specific selection only when the current selection root and the renderer's selection root match.
Source/WebCore:
Test: fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html
* rendering/RenderView.cpp:
(WebCore::SelectionIterator::SelectionIterator):
(WebCore::SelectionIterator::current):
(WebCore::SelectionIterator::checkForSpanner):
(WebCore::RenderView::applySubtreeSelection):
LayoutTests:
* fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt: Added.
* fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderViewcpp">trunk/Source/WebCore/rendering/RenderView.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastregionscrashwhenrendererisinmultipleselectionsubtrees2expectedtxt">trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastregionscrashwhenrendererisinmultipleselectionsubtrees2html">trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (186983 => 186984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-07-18 04:44:13 UTC (rev 186983)
+++ trunk/LayoutTests/ChangeLog 2015-07-18 05:20:47 UTC (rev 186984)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2015-07-17 Zalan Bujtas <zalan@apple.com>
+
+ REGRESSION (r169105): Do not assign a renderer to multiple selection subtrees.
+ https://bugs.webkit.org/show_bug.cgi?id=147038
+ rdar://problem/21819351
+
+ Reviewed by David Kilzer.
+
+ A renderer should never be assigned to multiple selection subtrees. (Currently RenderObject maintains the last selection state.)
+ RenderView::applySubtreeSelection() loops from the start to the end of the selection to find renderers that are inside the selection.
+ However, in case of regions (when multiple selection roots are present) traversing the renderer tree by calling RenderObject::nextInPreOrder() could
+ end up going across selection roots.
+ This patch ensures that we assign renderers to a specific selection only when the current selection root and the renderer's selection root match.
+
+ * fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt: Added.
+ * fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html: Added.
+
</ins><span class="cx"> 2015-07-17 Andy Estes <aestes@apple.com>
</span><span class="cx">
</span><span class="cx"> [iOS] Further tighten the sandbox around pages fetched with Content-Disposition: attachment
</span></span></pre></div>
<a id="trunkLayoutTestsfastregionscrashwhenrendererisinmultipleselectionsubtrees2expectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt (0 => 186984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt 2015-07-18 05:20:47 UTC (rev 186984)
</span><span class="lines">@@ -0,0 +1,3 @@
</span><ins>+PASS if no assert or crash.
+
+foobar
</ins></span></pre></div>
<a id="trunkLayoutTestsfastregionscrashwhenrendererisinmultipleselectionsubtrees2html"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html (0 => 186984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html (rev 0)
+++ trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html 2015-07-18 05:20:47 UTC (rev 186984)
</span><span class="lines">@@ -0,0 +1,23 @@
</span><ins>+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+</script>
+<style>
+ div { -webkit-flow-into: content-flow; }
+ * { -webkit-flow-into: thread; }
+</style>
+<body onload="shuffle()"><p>PASS if no assert or crash.</p><div>content</div>foobar</body>
+<script>
+ document.execCommand("SelectAll");
+</script>
+<script>
+function shuffle() {
+ var element = document.getElementsByTagName("div")[0];
+ element.parentNode.removeChild(element);
+ document.body.offsetTop;
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+</script>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (186983 => 186984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-07-18 04:44:13 UTC (rev 186983)
+++ trunk/Source/WebCore/ChangeLog 2015-07-18 05:20:47 UTC (rev 186984)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2015-07-17 Zalan Bujtas <zalan@apple.com>
+
+ REGRESSION (r169105): Do not assign a renderer to multiple selection subtrees.
+ https://bugs.webkit.org/show_bug.cgi?id=147038
+ rdar://problem/21819351
+
+ Reviewed by David Kilzer.
+
+ A renderer should never be assigned to multiple selection subtrees. (Currently RenderObject maintains the last selection state.)
+ RenderView::applySubtreeSelection() loops from the start to the end of the selection to find renderers that are inside the selection.
+ However, in case of regions (when multiple selection roots are present) traversing the renderer tree by calling RenderObject::nextInPreOrder() could
+ end up going across selection roots.
+ This patch ensures that we assign renderers to a specific selection only when the current selection root and the renderer's selection root match.
+
+ Test: fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html
+
+ * rendering/RenderView.cpp:
+ (WebCore::SelectionIterator::SelectionIterator):
+ (WebCore::SelectionIterator::current):
+ (WebCore::SelectionIterator::checkForSpanner):
+ (WebCore::RenderView::applySubtreeSelection):
+
</ins><span class="cx"> 2015-07-17 Andy Estes <aestes@apple.com>
</span><span class="cx">
</span><span class="cx"> [iOS] Further tighten the sandbox around pages fetched with Content-Disposition: attachment
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderViewcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderView.cpp (186983 => 186984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderView.cpp 2015-07-18 04:44:13 UTC (rev 186983)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2015-07-18 05:20:47 UTC (rev 186984)
</span><span class="lines">@@ -75,26 +75,14 @@
</span><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> struct SelectionIterator {
</span><del>- RenderObject* m_current;
- Vector<RenderMultiColumnSpannerPlaceholder*> m_spannerStack;
-
- SelectionIterator(RenderObject* o)
</del><ins>+ SelectionIterator(RenderObject* start)
+ : m_current(start)
</ins><span class="cx"> {
</span><del>- m_current = o;
</del><span class="cx"> checkForSpanner();
</span><span class="cx"> }
</span><span class="cx">
</span><del>- void checkForSpanner()
</del><ins>+ RenderObject* current() const
</ins><span class="cx"> {
</span><del>- if (!is<RenderMultiColumnSpannerPlaceholder>(m_current))
- return;
- auto& placeholder = downcast<RenderMultiColumnSpannerPlaceholder>(*m_current);
- m_spannerStack.append(&placeholder);
- m_current = placeholder.spanner();
- }
-
- RenderObject* current()
- {
</del><span class="cx"> return m_current;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -111,6 +99,19 @@
</span><span class="cx"> }
</span><span class="cx"> return m_current;
</span><span class="cx"> }
</span><ins>+
+private:
+ void checkForSpanner()
+ {
+ if (!is<RenderMultiColumnSpannerPlaceholder>(m_current))
+ return;
+ auto& placeholder = downcast<RenderMultiColumnSpannerPlaceholder>(*m_current);
+ m_spannerStack.append(&placeholder);
+ m_current = placeholder.spanner();
+ }
+
+ RenderObject* m_current { nullptr };
+ Vector<RenderMultiColumnSpannerPlaceholder*> m_spannerStack;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> RenderView::RenderView(Document& document, Ref<RenderStyle>&& style)
</span><span class="lines">@@ -1019,14 +1020,18 @@
</span><span class="cx"> root.selectionData().selectionEnd()->setSelectionStateIfNeeded(SelectionEnd);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- RenderObject* o = root.selectionData().selectionStart();
- RenderObject* stop = rendererAfterPosition(root.selectionData().selectionEnd(), root.selectionData().selectionEndPos());
- SelectionIterator selectionIterator(o);
-
- while (o && o != stop) {
- if (o != root.selectionData().selectionStart() && o != root.selectionData().selectionEnd() && o->canBeSelectionLeaf())
- o->setSelectionStateIfNeeded(SelectionInside);
- o = selectionIterator.next();
</del><ins>+ RenderObject* selectionStart = root.selectionData().selectionStart();
+ RenderObject* selectionEnd = rendererAfterPosition(root.selectionData().selectionEnd(), root.selectionData().selectionEndPos());
+ SelectionIterator selectionIterator(selectionStart);
+ for (RenderObject* currentRenderer = selectionStart; currentRenderer && currentRenderer != selectionEnd; currentRenderer = selectionIterator.next()) {
+ if (currentRenderer == root.selectionData().selectionStart() || currentRenderer == root.selectionData().selectionEnd())
+ continue;
+ if (!currentRenderer->canBeSelectionLeaf())
+ continue;
+ // FIXME: Move this logic to SelectionIterator::next()
+ if (&currentRenderer->selectionRoot() != &root)
+ continue;
+ currentRenderer->setSelectionStateIfNeeded(SelectionInside);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (blockRepaintMode != RepaintNothing)
</span><span class="lines">@@ -1036,36 +1041,33 @@
</span><span class="cx"> // put them in the new objects list.
</span><span class="cx"> SelectedObjectMap newSelectedObjects;
</span><span class="cx"> SelectedBlockMap newSelectedBlocks;
</span><del>- o = root.selectionData().selectionStart();
- selectionIterator = SelectionIterator(o);
- while (o && o != stop) {
- if (isValidObjectForNewSelection(root, *o)) {
- std::unique_ptr<RenderSelectionInfo> selectionInfo = std::make_unique<RenderSelectionInfo>(*o, true);
</del><ins>+ selectionIterator = SelectionIterator(selectionStart);
+ for (RenderObject* currentRenderer = selectionStart; currentRenderer && currentRenderer != selectionEnd; currentRenderer = selectionIterator.next()) {
+ if (isValidObjectForNewSelection(root, *currentRenderer)) {
+ std::unique_ptr<RenderSelectionInfo> selectionInfo = std::make_unique<RenderSelectionInfo>(*currentRenderer, true);
</ins><span class="cx">
</span><span class="cx"> #if ENABLE(SERVICE_CONTROLS)
</span><span class="cx"> for (auto& rect : selectionInfo->collectedSelectionRects())
</span><span class="cx"> m_selectionRectGatherer.addRect(selectionInfo->repaintContainer(), rect);
</span><del>- if (!o->isTextOrLineBreak())
</del><ins>+ if (!currentRenderer->isTextOrLineBreak())
</ins><span class="cx"> m_selectionRectGatherer.setTextOnly(false);
</span><span class="cx"> #endif
</span><span class="cx">
</span><del>- newSelectedObjects.set(o, WTF::move(selectionInfo));
</del><ins>+ newSelectedObjects.set(currentRenderer, WTF::move(selectionInfo));
</ins><span class="cx">
</span><del>- RenderBlock* cb = o->containingBlock();
- while (cb && !cb->isRenderView()) {
- std::unique_ptr<RenderBlockSelectionInfo>& blockInfo = newSelectedBlocks.add(cb, nullptr).iterator->value;
</del><ins>+ RenderBlock* containingBlock = currentRenderer->containingBlock();
+ while (containingBlock && !containingBlock->isRenderView()) {
+ std::unique_ptr<RenderBlockSelectionInfo>& blockInfo = newSelectedBlocks.add(containingBlock, nullptr).iterator->value;
</ins><span class="cx"> if (blockInfo)
</span><span class="cx"> break;
</span><del>- blockInfo = std::make_unique<RenderBlockSelectionInfo>(*cb);
- cb = cb->containingBlock();
</del><ins>+ blockInfo = std::make_unique<RenderBlockSelectionInfo>(*containingBlock);
+ containingBlock = containingBlock->containingBlock();
</ins><span class="cx">
</span><span class="cx"> #if ENABLE(SERVICE_CONTROLS)
</span><span class="cx"> m_selectionRectGatherer.addGapRects(blockInfo->repaintContainer(), blockInfo->rects());
</span><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>-
- o = selectionIterator.next();
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> if (blockRepaintMode == RepaintNothing)
</span></span></pre>
</div>
</div>
</body>
</html>