fix(ui): clear stale dirty queues when widgets change scenes

Remove pending layout, style and style-state entries from the old
scene during rebinding without invalidating shared accessibility
identities. Requeue dirty layouts in the destination scene while
preserving their invalidation reasons.

Fix a use-after-free exposed by TabFocus.WidgetCreatorDefaultsAreIntentional
after deleting a WebView.

Add regression coverage for WebView deletion, subtree and reverse
rebinding, and coalesced layout work.
This commit is contained in:
Martín Lucas Golini
2026-10-13 00:17:21 -03:00
parent c59920c72c
commit d1ea03f420
6 changed files with 154 additions and 8 deletions
+2
View File
@@ -79,6 +79,8 @@ class EE_API UILayout : public UIWidget {
virtual void onLayoutUpdate();
virtual void onUISceneNodeChange( UISceneNode* scene );
virtual void tryUpdateLayout();
virtual void updateLayoutTree();
+4
View File
@@ -1346,6 +1346,10 @@ class EE_API UISceneNode : public SceneNode {
*/
void onWidgetDelete( Node* node );
/** Removes this scene's queued widget work without invalidating accessibility identities.
* Used on deletion and before a widget changes its owning UI scene. */
void removeWidgetFromDirtyQueues( UIWidget* widget );
void onWidgetAccessibilitySourceDelete( UIWidget* widget );
/** The manager this scene's widgets report to, or nullptr when none exists yet. Never creates
+9
View File
@@ -51,6 +51,15 @@ void UILayout::onLayoutUpdate() {
sendCommonEvent( Event::OnLayoutUpdate );
}
void UILayout::onUISceneNodeChange( UISceneNode* scene ) {
UIWidget::onUISceneNodeChange( scene );
// The old scene no longer owns this pending pass. Preserve its reasons in the new scene
// rather than leaving mDirtyLayout set with no queue entry to process it. A dirty ancestor
// may have coalesced this layout's old entry, so use its flag instead of queue membership.
if ( scene && mDirtyLayout )
scene->invalidateLayout( this, mDirtyReasons );
}
const Sizef& UILayout::getSize() const {
if ( mDirtyLayout )
const_cast<UILayout*>( this )->updateLayout();
+9 -8
View File
@@ -1378,17 +1378,18 @@ void UISceneNode::onWidgetDelete( Node* node ) {
manager->onWidgetDelete( widget );
}
if ( node->isLayout() ) {
mDirtyLayouts.erase( node->asType<UILayout>() );
}
mDirtyStyle.erase( widget );
mDirtyStyleState.erase( widget );
mDirtyStyleStateCSSAnimations.erase( widget );
removeWidgetFromDirtyQueues( widget );
}
}
void UISceneNode::removeWidgetFromDirtyQueues( UIWidget* widget ) {
if ( widget->isLayout() )
mDirtyLayouts.erase( widget->asType<UILayout>() );
mDirtyStyle.erase( widget );
mDirtyStyleState.erase( widget );
mDirtyStyleStateCSSAnimations.erase( widget );
}
void UISceneNode::onWidgetAccessibilitySourceDelete( UIWidget* widget ) {
if ( mHostUISceneNode ) {
mHostUISceneNode->onWidgetAccessibilitySourceDelete( widget );
+5
View File
@@ -2195,6 +2195,11 @@ void UIWidget::onMarginChange() {
void UIWidget::onThemeLoaded() {}
void UIWidget::onUISceneNodeChange( UISceneNode* scene ) {
// Dirty queues belong to the local scene, even when embedded scenes share an accessibility
// manager. Otherwise deletion in the new scene leaves stale pointers queued in the old one.
if ( mUISceneNode )
mUISceneNode->removeWidgetFromDirtyQueues( this );
// Lifecycle bookkeeping never creates a manager, even with accessibility disabled.
auto* previousManager =
mUISceneNode ? mUISceneNode->getExistingAccessibilityManager() : nullptr;
+125
View File
@@ -14,6 +14,7 @@
#include <eepp/ui/css/stylesheetspecification.hpp>
#include <eepp/ui/uiapplication.hpp>
#include <eepp/ui/uifiledialog.hpp>
#include <eepp/ui/uihtmlinput.hpp>
#include <eepp/ui/uiiconthememanager.hpp>
#include <eepp/ui/uilayout.hpp>
#include <eepp/ui/uimessagebox.hpp>
@@ -21,6 +22,7 @@
#include <eepp/ui/uiscenenode.hpp>
#include <eepp/ui/uitextinput.hpp>
#include <eepp/ui/uithememanager.hpp>
#include <eepp/ui/uiwebview.hpp>
#include <eepp/ui/uiwidget.hpp>
#include <eepp/ui/uiwindow.hpp>
#include <eepp/window/cursor.hpp>
@@ -630,6 +632,18 @@ class InvalidationTestSceneNode : public UISceneNode {
size_t pendingLayoutCount() const { return mDirtyLayouts.size(); }
bool hasPendingLayout( UILayout* layout ) const { return mDirtyLayouts.count( layout ) != 0; }
bool hasPendingStyle( UIWidget* widget ) const { return mDirtyStyle.count( widget ) != 0; }
bool hasPendingStyleState( UIWidget* widget ) const {
return mDirtyStyleState.count( widget ) != 0;
}
bool hasPendingStyleStateAnimations( UIWidget* widget ) const {
return mDirtyStyleStateCSSAnimations.count( widget ) != 0;
}
size_t processedStyleRootCount() const { return mDirtyStylesSnapshot.size(); }
UIWidget* processedStyleRoot( size_t index ) const { return mDirtyStylesSnapshot[index].first; }
@@ -1005,6 +1019,117 @@ class InvalidationTestLayout : public UILayout {
LayoutInvalidationFlags lastReasons{ 0 };
};
UTEST( UISceneNode, WebViewDeletionDoesNotLeaveHostLayoutWork ) {
Engine::instance()->createWindow( WindowSettings( 320, 240, "WebView Dirty Queue Ownership",
WindowStyle::Default | WindowStyle::Hidden,
WindowBackend::Default, 32, {}, 1, false,
true ),
ContextSettings( false, 0, 0, GLv_default, true, false ) );
auto* host = InvalidationTestSceneNode::New();
init_test_scene_node( host );
host->setAccessibilityPolicy( AccessibilityPolicy::Disabled );
host->flushDirtyStyleAndLayout();
auto* webView = UIWebView::New();
auto* document = webView->getDocumentSceneNode()->getRoot()->findByTag( "webview::doc" );
ASSERT_TRUE( document != nullptr );
ASSERT_TRUE( document->isLayout() );
auto* layout = document->asType<UILayout>();
EXPECT_EQ( layout->getUISceneNode(), webView->getDocumentSceneNode() );
EXPECT_FALSE( host->hasPendingLayout( layout ) );
EXPECT_FALSE( host->hasPendingStyle( layout ) );
EXPECT_FALSE( host->hasPendingStyleState( layout ) );
EXPECT_FALSE( host->hasPendingStyleStateAnimations( layout ) );
eeDelete( webView );
// Compare queue membership only: the document pointer is no longer dereferenceable.
const bool staleLayout = host->hasPendingLayout( layout );
EXPECT_FALSE( staleLayout );
EXPECT_FALSE( host->hasPendingStyle( layout ) );
EXPECT_FALSE( host->hasPendingStyleState( layout ) );
EXPECT_FALSE( host->hasPendingStyleStateAnimations( layout ) );
// Keep the ownership failure readable when deliberately testing without the fix.
if ( !staleLayout ) {
auto* input = UIHTMLInput::New();
EXPECT_EQ( input->getUISceneNode(), host );
EXPECT_TRUE( input->getChildWidget() != nullptr );
host->flushDirtyStyleAndLayout();
}
Engine::destroySingleton();
}
UTEST( UISceneNode, DirtyLayoutAndStyleWorkFollowsSceneRebinding ) {
Engine::instance()->createWindow( WindowSettings( 320, 240, "Dirty Queue Scene Rebinding",
WindowStyle::Default | WindowStyle::Hidden,
WindowBackend::Default, 32, {}, 1, false,
true ),
ContextSettings( false, 0, 0, GLv_default, true, false ) );
auto* host = InvalidationTestSceneNode::New();
init_test_scene_node( host );
host->setAccessibilityPolicy( AccessibilityPolicy::Disabled );
auto* embedded = InvalidationTestSceneNode::New();
embedded->setParent( host->getRoot() );
host->setStyleSheet( "* { opacity: 1; }" );
auto* layout = eeNew( InvalidationTestLayout, () );
auto* child = UIWidget::New();
child->setParent( layout );
host->flushDirtyStyleAndLayout();
embedded->flushDirtyStyleAndLayout();
layout->setLayoutDirty( LayoutInvalidation::TextFormatting );
host->invalidateStyle( layout );
host->invalidateStyleState( layout, true );
host->invalidateStyle( child );
host->invalidateStyleState( child, true );
EXPECT_TRUE( host->hasPendingLayout( layout ) );
layout->setParent( embedded->getRoot() );
EXPECT_EQ( layout->getUISceneNode(), embedded );
EXPECT_FALSE( host->hasPendingLayout( layout ) );
EXPECT_FALSE( host->hasPendingStyle( layout ) );
EXPECT_FALSE( host->hasPendingStyleState( layout ) );
EXPECT_FALSE( host->hasPendingStyleStateAnimations( layout ) );
EXPECT_FALSE( host->hasPendingStyle( child ) );
EXPECT_FALSE( host->hasPendingStyleState( child ) );
EXPECT_FALSE( host->hasPendingStyleStateAnimations( child ) );
EXPECT_TRUE( embedded->hasPendingLayout( layout ) );
EXPECT_TRUE( embedded->hasPendingStyle( layout ) );
EXPECT_TRUE( embedded->hasPendingStyleState( layout ) );
EXPECT_TRUE( embedded->hasPendingStyleStateAnimations( layout ) );
embedded->flushDirtyStyleAndLayout();
EXPECT_TRUE( layout->lastReasons & LayoutInvalidation::TextFormatting );
EXPECT_FALSE( layout->isLayoutDirty() );
layout->setLayoutDirty( LayoutInvalidation::TextFormatting );
layout->setParent( host->getRoot() );
EXPECT_FALSE( embedded->hasPendingLayout( layout ) );
EXPECT_FALSE( embedded->hasPendingStyle( layout ) );
EXPECT_FALSE( embedded->hasPendingStyleState( layout ) );
EXPECT_TRUE( host->hasPendingLayout( layout ) );
host->updateDirtyLayouts();
EXPECT_TRUE( layout->lastReasons & LayoutInvalidation::TextFormatting );
EXPECT_FALSE( layout->isLayoutDirty() );
// Coalescing into a dirty parent must not strand a child moved into another scene.
auto* parent = eeNew( InvalidationTestLayout, () );
layout->setParent( parent );
host->flushDirtyStyleAndLayout();
layout->setLayoutDirty( LayoutInvalidation::TextFormatting );
parent->setLayoutDirty( LayoutInvalidation::Self );
EXPECT_FALSE( host->hasPendingLayout( layout ) );
EXPECT_TRUE( layout->isLayoutDirty() );
layout->setParent( embedded->getRoot() );
EXPECT_TRUE( embedded->hasPendingLayout( layout ) );
embedded->updateDirtyLayouts();
EXPECT_TRUE( layout->lastReasons & LayoutInvalidation::TextFormatting );
EXPECT_FALSE( layout->isLayoutDirty() );
eeDelete( layout );
host->flushDirtyStyleAndLayout();
embedded->flushDirtyStyleAndLayout();
Engine::destroySingleton();
}
UTEST( UISceneNode, LeafLayoutInvalidationPreservesSiblingRootsAndDescendantReasons ) {
Engine::instance()->createWindow( WindowSettings( 320, 240, "Leaf Layout Invalidation",
WindowStyle::Default, WindowBackend::Default,