Fix #122132: Crash canceling nested popups

Moving the cursor away from the extensions "Add Repository" popup
could crash because the "Repositories" popover was closed while the
popup it created remains open.

Resolve by clearing wmEventHandler_UI::context.region_popup
which match the freed region, following similar logic already used by
WM_event_modal_handler_region_replace.

Note that this bug was exposed by [0].

[0]: 38d11482f5e961e4d05930550452436da446dd4c
This commit is contained in:
Campbell Barton 2024-05-23 15:16:17 +10:00
parent 63599db598
commit b25eefbf9a
3 changed files with 30 additions and 1 deletions

@ -807,7 +807,16 @@ void ED_region_exit(bContext *C, ARegion *region)
CTX_wm_region_set(C, region);
WM_event_remove_handlers(C, &region->handlers);
WM_event_modal_handler_region_replace(win, region, nullptr);
if (region->regiontype == RGN_TYPE_TEMPORARY) {
/* This may be a popup region such as a popover or splash screen.
* In the case of popups which spawn popups it's possible for
* the parent popup to be freed *before* a popup which created it.
* The child may have a reference to the freed parent unless cleared here, see: #122132.
*
* Having parent popups freed before the popups they spawn could be investigated although
* they're not technically nested as they're both stored in #Screen::regionbase. */
WM_event_ui_handler_region_popup_replace(win, region, nullptr);
}
WM_draw_region_free(region);
/* The region is not in a state that it can be visible in anymore. Reinitializing is needed. */
region->visible = false;

@ -575,6 +575,9 @@ void WM_event_modal_handler_area_replace(wmWindow *win,
void WM_event_modal_handler_region_replace(wmWindow *win,
const ARegion *old_region,
ARegion *new_region);
void WM_event_ui_handler_region_popup_replace(wmWindow *win,
const ARegion *old_region,
ARegion *new_region);
/**
* Called on exit or remove area, only here call cancel callback.

@ -850,6 +850,7 @@ static eHandlerActionFlag wm_handler_ui_call(bContext *C,
CTX_wm_region_set(C, handler->context.region);
}
if (handler->context.region_popup) {
BLI_assert(screen_temp_region_exists(handler->context.region_popup));
CTX_wm_region_popup_set(C, handler->context.region_popup);
}
@ -859,6 +860,7 @@ static eHandlerActionFlag wm_handler_ui_call(bContext *C,
if ((retval != WM_UI_HANDLER_BREAK) || always_pass) {
CTX_wm_area_set(C, area);
CTX_wm_region_set(C, region);
BLI_assert((region_popup == nullptr) || screen_temp_region_exists(region_popup));
CTX_wm_region_popup_set(C, region_popup);
}
else {
@ -2228,6 +2230,7 @@ void WM_event_remove_handlers(bContext *C, ListBase *handlers)
CTX_wm_region_set(C, handler->context.region);
}
if (handler->context.region_popup) {
BLI_assert(screen_temp_region_exists(handler->context.region_popup));
CTX_wm_region_popup_set(C, handler->context.region_popup);
}
@ -4550,6 +4553,20 @@ void WM_event_modal_handler_region_replace(wmWindow *win,
}
}
void WM_event_ui_handler_region_popup_replace(wmWindow *win,
const ARegion *old_region,
ARegion *new_region)
{
LISTBASE_FOREACH (wmEventHandler *, handler_base, &win->modalhandlers) {
if (handler_base->type == WM_HANDLER_TYPE_UI) {
wmEventHandler_UI *handler = (wmEventHandler_UI *)handler_base;
if (handler->context.region_popup == old_region) {
handler->context.region_popup = new_region;
}
}
}
}
wmEventHandler_Keymap *WM_event_add_keymap_handler(ListBase *handlers, wmKeyMap *keymap)
{
if (!keymap) {