diff --git a/third_party/blink/renderer/core/exported/web_page_popup_impl.cc b/third_party/blink/renderer/core/exported/web_page_popup_impl.cc --- a/third_party/blink/renderer/core/exported/web_page_popup_impl.cc +++ b/third_party/blink/renderer/core/exported/web_page_popup_impl.cc @@ -1026,12 +1026,23 @@ void WebPagePopupImpl::Close() { // has already run on another stack, and destroyed |page_|. If the popup is // closed from the browser via IPC to RenderWidget, then we come here first // and want to synchronously Cancel() immediately. if (page_) { // We set |closing_| here to inform ClosePopup() that it is being run // synchronously from inside Close(). closing_ = true; // This should end up running ClosePopup() though the PopupClient. Cancel(); + // Cancel() may not reach ClosePopup() if WebViewImpl::page_popup_ no + // longer points at this popup (e.g. it was overwritten via a re-entrant + // OpenPagePopup). In that case |page_| is still set, the popup's frame is + // still attached, and PagePopupController::popup_ still references |this|. + // Tear down here so the controller cannot dispatch a virtual call through + // |this| after Release() frees it below. + if (page_) { + PagePopupController::From(*page_)->ClearPagePopupClient(); + DestroyPage(); + } } // TODO(dtapuska): WidgetBase shutdown should happen before Page is diff --git a/third_party/blink/renderer/core/page/page_popup_controller.h b/third_party/blink/renderer/core/page/page_popup_controller.h --- a/third_party/blink/renderer/core/page/page_popup_controller.h +++ b/third_party/blink/renderer/core/page/page_popup_controller.h @@ -33,6 +33,7 @@ #include +#include "base/memory/raw_ref.h" #include "third_party/blink/renderer/platform/bindings/script_wrappable.h" #include "third_party/blink/renderer/platform/heap/collection_support/heap_vector.h" #include "third_party/blink/renderer/platform/heap/garbage_collected.h" @@ -89,7 +90,10 @@ class PagePopupController : public ScriptWrappable, public Supplement { void debugLog(const String&); private: - PagePopup& popup_; + // The referent (WebPagePopupImpl) is off-heap RefCounted/USING_FAST_MALLOC + // and can be freed before this Oilpan object; use raw_ref so BackupRefPtr + // quarantines the slot rather than allowing a vtable hijack. + const raw_ref popup_; std::optional popup_origin_; Vector options_bounds_; diff --git a/third_party/blink/renderer/core/page/page_popup_controller.cc b/third_party/blink/renderer/core/page/page_popup_controller.cc --- a/third_party/blink/renderer/core/page/page_popup_controller.cc +++ b/third_party/blink/renderer/core/page/page_popup_controller.cc @@ -117,7 +117,10 @@ void PagePopupController::ClearPagePopupClient() { } void PagePopupController::setWindowRect(int x, int y, int width, int height) { - popup_.SetWindowRect(gfx::Rect(x, y, width, height)); + if (!popup_client_) { + return; + } + popup_->SetWindowRect(gfx::Rect(x, y, width, height)); popup_origin_ = gfx::Point(x, y); popup_client_->SetMenuListOptionsBoundsInAXTree(options_bounds_,