bcarrella
/
cef
mirror of https://bitbucket.org/chromiumembedded/cef
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix raw_ptr leak of main frame RFH during WebContents close (see #3660)

This commit is contained in:
Marshall Greenblatt 2024-10-22 14:48:06 -04:00
parent a747221b01
commit 6514b929c4
3 changed files with 39 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
libcef/browser/browser_info.cc
View File

@ -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<CefBrowserHostBase> browser,
CefRefPtr<CefFrameHostImpl> 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_);
}
}

35
libcef/browser/frame_host_impl.cc
View File

@ -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(

8
libcef/browser/frame_host_impl.h
View File

@ -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