diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index 73ae779aa..a1e7c551e 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -277,11 +277,14 @@ void CefBrowserInfo::RemoveFrame(content::RenderFrameHost* host) { { auto it2 = frame_info_set_.find(frame_info); - // Explicitly Detach everything but the current main frame. + // Explicitly Detach everything. const auto& other_frame_info = *it2; - if (other_frame_info->frame_ && !other_frame_info->IsCurrentMainFrame()) { + if (other_frame_info->frame_) { + const bool is_current_main_frame = other_frame_info->IsCurrentMainFrame(); if (other_frame_info->frame_->Detach( - CefFrameHostImpl::DetachReason::RENDER_FRAME_DELETED)) { + CefFrameHostImpl::DetachReason::RENDER_FRAME_DELETED, + is_current_main_frame)) { + DCHECK(!is_current_main_frame); MaybeNotifyFrameDetached(browser_, other_frame_info->frame_); } } @@ -477,7 +480,8 @@ void CefBrowserInfo::SetMainFrame(CefRefPtr browser, CefRefPtr old_frame; if (main_frame_) { old_frame = main_frame_; - if (old_frame->Detach(CefFrameHostImpl::DetachReason::NEW_MAIN_FRAME)) { + if (old_frame->Detach(CefFrameHostImpl::DetachReason::NEW_MAIN_FRAME, + /*is_current_main_frame=*/false)) { MaybeNotifyFrameDetached(browser, old_frame); } } @@ -556,11 +560,14 @@ void CefBrowserInfo::RemoveAllFrames( frame_id_map_.clear(); frame_token_to_id_map_.clear(); - // Explicitly Detach everything but the current main frame. + // Explicitly Detach everything. for (auto& info : frame_info_set_) { - if (info->frame_ && !info->IsCurrentMainFrame()) { + if (info->frame_) { + const bool is_current_main_frame = info->IsCurrentMainFrame(); if (info->frame_->Detach( - CefFrameHostImpl::DetachReason::BROWSER_DESTROYED)) { + CefFrameHostImpl::DetachReason::BROWSER_DESTROYED, + is_current_main_frame)) { + DCHECK(!is_current_main_frame); MaybeNotifyFrameDetached(old_browser, info->frame_); } } diff --git a/libcef/browser/frame_host_impl.cc b/libcef/browser/frame_host_impl.cc index 001ccb298..1d15251a1 100644 --- a/libcef/browser/frame_host_impl.cc +++ b/libcef/browser/frame_host_impl.cc @@ -495,7 +495,7 @@ bool CefFrameHostImpl::IsDetached() const { return !GetRenderFrameHost(); } -bool CefFrameHostImpl::Detach(DetachReason reason) { +bool CefFrameHostImpl::Detach(DetachReason reason, bool is_current_main_frame) { CEF_REQUIRE_UIT(); if (VLOG_IS_ON(1)) { @@ -516,24 +516,29 @@ bool CefFrameHostImpl::Detach(DetachReason reason) { << ", is_connected=" << render_frame_.is_bound() << ")"; } - // May be called multiple times (e.g. from CefBrowserInfo SetMainFrame and - // RemoveFrame). - bool first_detach = false; + // This method may be called multiple times (e.g. from CefBrowserInfo + // SetMainFrame and RemoveFrame). + bool is_first_complete_detach = false; // Should not be called for temporary frames. CHECK(!is_temporary()); - { - base::AutoLock lock_scope(state_lock_); - if (browser_info_) { - first_detach = true; - browser_info_ = nullptr; - } - } + // Must be a main frame if |is_current_main_frame| is true. + CHECK(!is_current_main_frame || is_main_frame_); - // In case we never attached, clean up. - while (!queued_renderer_actions_.empty()) { - queued_renderer_actions_.pop(); + if (!is_current_main_frame) { + { + base::AutoLock lock_scope(state_lock_); + if (browser_info_) { + is_first_complete_detach = true; + browser_info_ = nullptr; + } + } + + // In case we never attached, clean up. + while (!queued_renderer_actions_.empty()) { + queued_renderer_actions_.pop(); + } } if (render_frame_.is_bound()) { @@ -543,7 +548,7 @@ bool CefFrameHostImpl::Detach(DetachReason reason) { render_frame_.reset(); render_frame_host_ = nullptr; - return first_detach; + return is_first_complete_detach; } void CefFrameHostImpl::MaybeReAttach( diff --git a/libcef/browser/frame_host_impl.h b/libcef/browser/frame_host_impl.h index 6d699b2b1..bcd335529 100644 --- a/libcef/browser/frame_host_impl.h +++ b/libcef/browser/frame_host_impl.h @@ -136,9 +136,11 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame { // Owned frame objects will be detached explicitly when the associated // RenderFrame is deleted. Temporary frame objects will be detached - // implicitly via CefBrowserInfo::browser() returning nullptr. Returns true - // if this was the first call to Detach() for the frame. - bool Detach(DetachReason reason); + // implicitly via CefBrowserInfo::browser() returning nullptr. If + // |is_current_main_frame| is true then only the RenderFrameHost references + // will be released as we want the frame object itself to remain valid. + // Returns true if the frame is completely detached for the first time. + bool Detach(DetachReason reason, bool is_current_main_frame); // A frame has swapped to active status from prerendering or the back-forward // cache. We may need to re-attach if the RFH has changed. See