Diff
Modified: trunk/LayoutTests/ChangeLog (223130 => 223131)
--- trunk/LayoutTests/ChangeLog 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/LayoutTests/ChangeLog 2017-10-10 13:02:47 UTC (rev 223131)
@@ -1,3 +1,14 @@
+2017-10-10 Antti Koivisto <an...@apple.com>
+
+ RenderObject::destroy() should only be invoked after renderer has been removed from the tree
+ https://bugs.webkit.org/show_bug.cgi?id=178075
+
+ Reviewed by Zalan Bujtas.
+
+ * accessibility/mac/textbox-role-reports-notifications.html:
+
+ This passed because spurious AXValueChanged notifications. Force layout to prevent coalescing between mutations.
+
2017-10-10 Joanmarie Diggs <jdi...@igalia.com>
AX: [ATK] STATE_CHECKABLE should be removed from radio buttons in radiogroups with aria-readonly="true"
Modified: trunk/LayoutTests/accessibility/mac/textbox-role-reports-notifications.html (223130 => 223131)
--- trunk/LayoutTests/accessibility/mac/textbox-role-reports-notifications.html 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/LayoutTests/accessibility/mac/textbox-role-reports-notifications.html 2017-10-10 13:02:47 UTC (rev 223131)
@@ -19,7 +19,9 @@
textboxAxElement.addNotificationListener(logNotification);
pendingNotifications = 3;
ariaTextBox.firstChild.deleteData(0, 5);
+ ariaTextBox.offsetWidth;
ariaTextBox.textContent = "changed textContent";
+ ariaTextBox.offsetWidth;
ariaTextBox.innerText = "changed innerText";
}
Modified: trunk/Source/WebCore/ChangeLog (223130 => 223131)
--- trunk/Source/WebCore/ChangeLog 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/ChangeLog 2017-10-10 13:02:47 UTC (rev 223131)
@@ -1,3 +1,52 @@
+2017-10-10 Antti Koivisto <an...@apple.com>
+
+ RenderObject::destroy() should only be invoked after renderer has been removed from the tree
+ https://bugs.webkit.org/show_bug.cgi?id=178075
+
+ Reviewed by Zalan Bujtas.
+
+ This patch fixes the remaining cases where the renderer is still in the tree while destroy()
+ is called and adds the assert.
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::removeLeftoverAnonymousBlock):
+ (WebCore::RenderBlock::takeChild):
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::willBeDestroyed):
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::~RenderLayer):
+
+ Null the parent pointers for m_scrollCorner/m_resizer.
+
+ (WebCore::RenderLayer::calculateClipRects const):
+ * rendering/RenderLayer.h:
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::willBeDestroyed):
+ (WebCore::RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers):
+ (WebCore::RenderObject::destroy):
+
+ Use RELEASE_ASSERT as these are cheap and important checks.
+ Also turn isBeingDestroyed test into RELEASE_ASSERT.
+ Remove AX call that no longer does anything.
+
+ (WebCore::RenderObject::destroyAndCleanupAnonymousWrappers): Deleted.
+ * rendering/RenderObject.h:
+ * rendering/RenderRubyBase.cpp:
+ (WebCore::RenderRubyBase::moveBlockChildren):
+ * rendering/RenderTableRow.cpp:
+ (WebCore::RenderTableRow::collapseAndDestroyAnonymousSiblingRows):
+ (WebCore::RenderTableRow::destroyAndCollapseAnonymousSiblingRows): Deleted.
+
+ Renamed and made this no longer destroy itself. The caller now takes care of that.
+ Removed an unnecessary lambda.
+
+ * rendering/RenderTableRow.h:
+ * style/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::tearDownRenderers):
+ (WebCore::RenderTreeUpdater::tearDownRenderer):
+ * style/RenderTreeUpdaterListItem.cpp:
+ (WebCore::RenderTreeUpdater::ListItem::updateMarker):
+
2017-10-09 Antti Koivisto <an...@apple.com>
Add isContinuation bit
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -764,15 +764,15 @@
child->nextSibling()->setPreviousSibling(child->previousSibling());
}
- child->setFirstChild(0);
- child->m_next = 0;
+ child->setFirstChild(nullptr);
+ child->m_next = nullptr;
// Remove all the information in the flow thread associated with the leftover anonymous block.
child->resetFragmentedFlowStateOnRemoval();
- child->setParent(0);
- child->setPreviousSibling(0);
- child->setNextSibling(0);
+ child->setParent(nullptr);
+ child->setPreviousSibling(nullptr);
+ child->setNextSibling(nullptr);
child->destroy();
}
@@ -875,7 +875,7 @@
// Delete the now-empty block's lines and nuke it.
nextBlock.deleteLines();
- nextBlock.destroy();
+ nextBlock.removeFromParentAndDestroy();
next = nullptr;
}
}
@@ -934,7 +934,8 @@
break;
}
setContinuation(nullptr);
- destroy();
+ // FIXME: This is dangerous.
+ removeFromParentAndDestroy();
}
}
return takenChild;
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -188,7 +188,7 @@
void RenderBoxModelObject::willBeDestroyed()
{
if (hasContinuation()) {
- continuation()->destroy();
+ continuation()->removeFromParentAndDestroy();
setContinuation(nullptr);
}
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -369,6 +369,9 @@
if (m_reflection)
removeReflection();
+ clearScrollCorner();
+ clearResizer();
+
FilterInfo::remove(*this);
// Child layers will be deleted by their corresponding render objects, so
@@ -6663,12 +6666,13 @@
auto corner = renderer().hasOverflowClip() ? actualRenderer->getUncachedPseudoStyle(PseudoStyleRequest(SCROLLBAR_CORNER), &actualRenderer->style()) : nullptr;
if (!corner) {
- m_scrollCorner = nullptr;
+ clearScrollCorner();
return;
}
if (!m_scrollCorner) {
m_scrollCorner = createRenderer<RenderScrollbarPart>(renderer().document(), WTFMove(*corner));
+ // FIXME: A renderer should be a child of its parent!
m_scrollCorner->setParent(&renderer());
m_scrollCorner->initializeStyle();
} else
@@ -6675,6 +6679,14 @@
m_scrollCorner->setStyle(WTFMove(*corner));
}
+void RenderLayer::clearScrollCorner()
+{
+ if (!m_scrollCorner)
+ return;
+ m_scrollCorner->setParent(nullptr);
+ m_scrollCorner = nullptr;
+}
+
void RenderLayer::updateResizerStyle()
{
RenderElement* actualRenderer = rendererForScrollbar(renderer());
@@ -6681,12 +6693,13 @@
auto resizer = renderer().hasOverflowClip() ? actualRenderer->getUncachedPseudoStyle(PseudoStyleRequest(RESIZER), &actualRenderer->style()) : nullptr;
if (!resizer) {
- m_resizer = nullptr;
+ clearResizer();
return;
}
if (!m_resizer) {
m_resizer = createRenderer<RenderScrollbarPart>(renderer().document(), WTFMove(*resizer));
+ // FIXME: A renderer should be a child of its parent!
m_resizer->setParent(&renderer());
m_resizer->initializeStyle();
} else
@@ -6693,6 +6706,14 @@
m_resizer->setStyle(WTFMove(*resizer));
}
+void RenderLayer::clearResizer()
+{
+ if (!m_resizer)
+ return;
+ m_resizer->setParent(nullptr);
+ m_resizer = nullptr;
+}
+
RenderLayer* RenderLayer::reflectionLayer() const
{
return m_reflection ? m_reflection->layer() : nullptr;
@@ -6702,6 +6723,7 @@
{
ASSERT(!m_reflection);
m_reflection = createRenderer<RenderReplica>(renderer().document(), createReflectionStyle());
+ // FIXME: A renderer should be a child of its parent!
m_reflection->setParent(&renderer()); // We create a 1-way connection.
m_reflection->initializeStyle();
}
Modified: trunk/Source/WebCore/rendering/RenderLayer.h (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderLayer.h 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderLayer.h 2017-10-10 13:02:47 UTC (rev 223131)
@@ -978,7 +978,9 @@
void positionOverflowControls(const IntSize&);
void updateScrollCornerStyle();
+ void clearScrollCorner();
void updateResizerStyle();
+ void clearResizer();
void drawPlatformResizerImage(GraphicsContext&, const LayoutRect& resizerCornerRect);
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -1428,22 +1428,9 @@
void RenderObject::willBeDestroyed()
{
- // For accessibility management, notify the parent of the imminent change to its child set.
- // We do it now, before remove(), while the parent pointer is still available.
- if (AXObjectCache* cache = document().existingAXObjectCache())
- cache->childrenChanged(this->parent());
-
- if (m_parent) {
- // FIXME: We should have always been removed from the parent before being destroyed.
- auto takenThis = m_parent->takeChild(*this);
- auto* leakedPtr = takenThis.release();
- UNUSED_PARAM(leakedPtr);
- }
-
+ ASSERT(!m_parent);
ASSERT(renderTreeBeingDestroyed() || !is<RenderElement>(*this) || !view().frameView().hasSlowRepaintObject(downcast<RenderElement>(*this)));
- // The remove() call above may invoke axObjectCache()->childrenChanged() on the parent, which may require the AX render
- // object for this renderer. So we remove the AX render object now, after the renderer is removed.
if (AXObjectCache* cache = document().existingAXObjectCache())
cache->remove(this);
@@ -1476,11 +1463,11 @@
parent()->setNeedsBoundariesUpdate();
}
-void RenderObject::destroyAndCleanupAnonymousWrappers()
+void RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers()
{
// If the tree is destroyed, there is no need for a clean-up phase.
if (renderTreeBeingDestroyed()) {
- destroy();
+ removeFromParentAndDestroy();
return;
}
@@ -1497,18 +1484,20 @@
destroyRootParent = destroyRootParent->parent();
}
- if (is<RenderTableRow>(*destroyRoot)) {
- downcast<RenderTableRow>(*destroyRoot).destroyAndCollapseAnonymousSiblingRows();
- return;
- }
+ if (is<RenderTableRow>(*destroyRoot))
+ downcast<RenderTableRow>(*destroyRoot).collapseAndDestroyAnonymousSiblingRows();
- destroyRoot->destroy();
+ destroyRoot->removeFromParentAndDestroy();
// WARNING: |this| is deleted here.
}
void RenderObject::destroy()
{
- ASSERT(!m_bitfields.beingDestroyed());
+ RELEASE_ASSERT(!m_parent);
+ RELEASE_ASSERT(!m_next);
+ RELEASE_ASSERT(!m_previous);
+ RELEASE_ASSERT(!m_bitfields.beingDestroyed());
+
m_bitfields.setBeingDestroyed(true);
#if PLATFORM(IOS)
Modified: trunk/Source/WebCore/rendering/RenderObject.h (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderObject.h 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2017-10-10 13:02:47 UTC (rev 223131)
@@ -721,7 +721,6 @@
// When performing a global document tear-down, or when going into the page cache, the renderer of the document is cleared.
bool renderTreeBeingDestroyed() const;
- void destroyAndCleanupAnonymousWrappers();
void destroy();
// Virtual function helpers for the deprecated Flexible Box Layout (display: -webkit-box).
@@ -748,6 +747,7 @@
virtual void imageChanged(WrappedImagePtr, const IntRect* = nullptr) { }
void removeFromParentAndDestroy();
+ void removeFromParentAndDestroyCleaningUpAnonymousWrappers();
CSSAnimationController& animation() const;
Modified: trunk/Source/WebCore/rendering/RenderRubyBase.cpp (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderRubyBase.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderRubyBase.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -125,7 +125,7 @@
RenderBlock* anonBlockThere = downcast<RenderBlock>(lastChildThere);
anonBlockHere->moveAllChildrenTo(anonBlockThere, true);
anonBlockHere->deleteLines();
- anonBlockHere->destroy();
+ anonBlockHere->removeFromParentAndDestroy();
}
// Move all remaining children normally.
moveChildrenTo(toBase, firstChild(), beforeChild);
Modified: trunk/Source/WebCore/rendering/RenderTableRow.cpp (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderTableRow.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderTableRow.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -284,44 +284,38 @@
return RenderTableRow::createTableRowWithStyle(parent.document(), parent.style());
}
-void RenderTableRow::destroyAndCollapseAnonymousSiblingRows()
+void RenderTableRow::collapseAndDestroyAnonymousSiblingRows()
{
- auto collapseAnonymousSiblingRows = [&] {
- auto* section = this->section();
- if (!section)
+ auto* section = this->section();
+ if (!section)
+ return;
+
+ // All siblings generated?
+ for (auto* current = section->firstRow(); current; current = current->nextRow()) {
+ if (current == this)
+ continue;
+ if (!current->isAnonymous())
return;
+ }
- // All siblings generated?
- for (auto* current = section->firstRow(); current; current = current->nextRow()) {
- if (current == this)
- continue;
- if (!current->isAnonymous())
- return;
+ RenderTableRow* rowToInsertInto = nullptr;
+ auto* currentRow = section->firstRow();
+ while (currentRow) {
+ if (currentRow == this) {
+ currentRow = currentRow->nextRow();
+ continue;
}
-
- RenderTableRow* rowToInsertInto = nullptr;
- auto* currentRow = section->firstRow();
- while (currentRow) {
- if (currentRow == this) {
- currentRow = currentRow->nextRow();
- continue;
- }
- if (!rowToInsertInto) {
- rowToInsertInto = currentRow;
- currentRow = currentRow->nextRow();
- continue;
- }
- currentRow->moveAllChildrenTo(rowToInsertInto);
- auto* destroyThis = currentRow;
+ if (!rowToInsertInto) {
+ rowToInsertInto = currentRow;
currentRow = currentRow->nextRow();
- destroyThis->destroy();
+ continue;
}
- if (rowToInsertInto)
- rowToInsertInto->setNeedsLayout();
- };
-
- collapseAnonymousSiblingRows();
- destroy();
+ currentRow->moveAllChildrenTo(rowToInsertInto);
+ auto toDestroy = section->takeChild(*currentRow);
+ currentRow = currentRow->nextRow();
+ }
+ if (rowToInsertInto)
+ rowToInsertInto->setNeedsLayout();
}
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderTableRow.h (223130 => 223131)
--- trunk/Source/WebCore/rendering/RenderTableRow.h 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/rendering/RenderTableRow.h 2017-10-10 13:02:47 UTC (rev 223131)
@@ -62,7 +62,7 @@
bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) override;
- void destroyAndCollapseAnonymousSiblingRows();
+ void collapseAndDestroyAnonymousSiblingRows();
private:
static RenderPtr<RenderTableRow> createTableRowWithStyle(Document&, const RenderStyle&);
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (223130 => 223131)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -520,7 +520,7 @@
element.clearStyleDerivedDataBeforeDetachingRenderer();
if (auto* renderer = element.renderer()) {
- renderer->destroyAndCleanupAnonymousWrappers();
+ renderer->removeFromParentAndDestroyCleaningUpAnonymousWrappers();
element.setRenderer(nullptr);
}
if (element.hasCustomStyleResolveCallbacks())
@@ -550,7 +550,7 @@
auto* renderer = text.renderer();
if (!renderer)
return;
- renderer->destroyAndCleanupAnonymousWrappers();
+ renderer->removeFromParentAndDestroyCleaningUpAnonymousWrappers();
text.setRenderer(nullptr);
}
Modified: trunk/Source/WebCore/style/RenderTreeUpdaterListItem.cpp (223130 => 223131)
--- trunk/Source/WebCore/style/RenderTreeUpdaterListItem.cpp 2017-10-10 12:47:00 UTC (rev 223130)
+++ trunk/Source/WebCore/style/RenderTreeUpdaterListItem.cpp 2017-10-10 13:02:47 UTC (rev 223131)
@@ -116,7 +116,7 @@
// If current parent is an anonymous block that has lost all its children, destroy it.
if (currentParent && currentParent->isAnonymousBlock() && !currentParent->firstChild() && !downcast<RenderBlock>(*currentParent).continuation())
- currentParent->destroy();
+ currentParent->removeFromParentAndDestroy();
}
}
