Title: [186984] trunk
- Revision
- 186984
- Author
- [email protected]
- Date
- 2015-07-17 22:20:47 -0700 (Fri, 17 Jul 2015)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (186983 => 186984)
--- trunk/LayoutTests/ChangeLog 2015-07-18 04:44:13 UTC (rev 186983)
+++ trunk/LayoutTests/ChangeLog 2015-07-18 05:20:47 UTC (rev 186984)
@@ -1,3 +1,20 @@
+2015-07-17 Zalan Bujtas <[email protected]>
+
+ 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.
+
2015-07-17 Andy Estes <[email protected]>
[iOS] Further tighten the sandbox around pages fetched with Content-Disposition: attachment
Added: trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2-expected.txt (0 => 186984)
--- 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)
@@ -0,0 +1,3 @@
+PASS if no assert or crash.
+
+foobar
Added: trunk/LayoutTests/fast/regions/crash-when-renderer-is-in-multiple-selection-subtrees2.html (0 => 186984)
--- 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)
@@ -0,0 +1,23 @@
+<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>
Modified: trunk/Source/WebCore/ChangeLog (186983 => 186984)
--- 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)
@@ -1,3 +1,25 @@
+2015-07-17 Zalan Bujtas <[email protected]>
+
+ 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):
+
2015-07-17 Andy Estes <[email protected]>
[iOS] Further tighten the sandbox around pages fetched with Content-Disposition: attachment
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (186983 => 186984)
--- 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)
@@ -75,26 +75,14 @@
};
struct SelectionIterator {
- RenderObject* m_current;
- Vector<RenderMultiColumnSpannerPlaceholder*> m_spannerStack;
-
- SelectionIterator(RenderObject* o)
+ SelectionIterator(RenderObject* start)
+ : m_current(start)
{
- m_current = o;
checkForSpanner();
}
- void checkForSpanner()
+ RenderObject* current() const
{
- if (!is<RenderMultiColumnSpannerPlaceholder>(m_current))
- return;
- auto& placeholder = downcast<RenderMultiColumnSpannerPlaceholder>(*m_current);
- m_spannerStack.append(&placeholder);
- m_current = placeholder.spanner();
- }
-
- RenderObject* current()
- {
return m_current;
}
@@ -111,6 +99,19 @@
}
return m_current;
}
+
+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;
};
RenderView::RenderView(Document& document, Ref<RenderStyle>&& style)
@@ -1019,14 +1020,18 @@
root.selectionData().selectionEnd()->setSelectionStateIfNeeded(SelectionEnd);
}
- 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();
+ 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 (¤tRenderer->selectionRoot() != &root)
+ continue;
+ currentRenderer->setSelectionStateIfNeeded(SelectionInside);
}
if (blockRepaintMode != RepaintNothing)
@@ -1036,36 +1041,33 @@
// put them in the new objects list.
SelectedObjectMap newSelectedObjects;
SelectedBlockMap newSelectedBlocks;
- 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);
+ 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);
#if ENABLE(SERVICE_CONTROLS)
for (auto& rect : selectionInfo->collectedSelectionRects())
m_selectionRectGatherer.addRect(selectionInfo->repaintContainer(), rect);
- if (!o->isTextOrLineBreak())
+ if (!currentRenderer->isTextOrLineBreak())
m_selectionRectGatherer.setTextOnly(false);
#endif
- newSelectedObjects.set(o, WTF::move(selectionInfo));
+ newSelectedObjects.set(currentRenderer, WTF::move(selectionInfo));
- RenderBlock* cb = o->containingBlock();
- while (cb && !cb->isRenderView()) {
- std::unique_ptr<RenderBlockSelectionInfo>& blockInfo = newSelectedBlocks.add(cb, nullptr).iterator->value;
+ RenderBlock* containingBlock = currentRenderer->containingBlock();
+ while (containingBlock && !containingBlock->isRenderView()) {
+ std::unique_ptr<RenderBlockSelectionInfo>& blockInfo = newSelectedBlocks.add(containingBlock, nullptr).iterator->value;
if (blockInfo)
break;
- blockInfo = std::make_unique<RenderBlockSelectionInfo>(*cb);
- cb = cb->containingBlock();
+ blockInfo = std::make_unique<RenderBlockSelectionInfo>(*containingBlock);
+ containingBlock = containingBlock->containingBlock();
#if ENABLE(SERVICE_CONTROLS)
m_selectionRectGatherer.addGapRects(blockInfo->repaintContainer(), blockInfo->rects());
#endif
}
}
-
- o = selectionIterator.next();
}
if (blockRepaintMode == RepaintNothing)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
