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

Fix crash when parent is destroyed during popup creation (issue #2041)

This commit is contained in:
Marshall Greenblatt 2017-05-09 16:45:57 -04:00
parent 25a0d02077
commit 753baf17b7
6 changed files with 112 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/capi/cef_life_span_handler_capi.h
View File

@ -76,7 +76,9 @@ typedef struct _cef_life_span_handler_t {
// is set to false (0) the new browser will not be scriptable and may not be
// hosted in the same renderer process as the source browser. Any
// modifications to |windowInfo| will be ignored if the parent browser is
// wrapped in a cef_browser_view_t.
// wrapped in a cef_browser_view_t. Popup browser creation will be canceled if
// the parent browser is destroyed before the popup browser creation completes
// (indicated by a call to OnAfterCreated for the popup browser).
///
int (CEF_CALLBACK *on_before_popup)(struct _cef_life_span_handler_t* self,
struct _cef_browser_t* browser, struct _cef_frame_t* frame,

4
include/cef_life_span_handler.h
View File

@ -71,7 +71,9 @@ class CefLifeSpanHandler : public virtual CefBaseRefCounted {
// false the new browser will not be scriptable and may not be hosted in the
// same renderer process as the source browser. Any modifications to
// |windowInfo| will be ignored if the parent browser is wrapped in a
// CefBrowserView.
// CefBrowserView. Popup browser creation will be canceled if the parent
// browser is destroyed before the popup browser creation completes (indicated
// by a call to OnAfterCreated for the popup browser).
///
/*--cef(optional_param=target_url,optional_param=target_frame_name)--*/
virtual bool OnBeforePopup(CefRefPtr<CefBrowser> browser,

10
libcef/browser/browser_host_impl.cc
View File

@ -352,6 +352,11 @@ CefRefPtr<CefBrowserHostImpl> CefBrowserHostImpl::CreateInternal(
DCHECK(!opener.get() || browser_info->is_popup());
if (opener) {
if (!opener->platform_delegate_) {
// The opener window is being destroyed. Cancel the popup.
return nullptr;
}
// Give the opener browser's platform delegate an opportunity to modify the
// new browser's platform delegate.
opener->platform_delegate_->PopupWebContentsCreated(
@ -383,7 +388,7 @@ CefRefPtr<CefBrowserHostImpl> CefBrowserHostImpl::CreateInternal(
// CefBrowserViewDelegate::OnBrowserCreated().
browser->platform_delegate_->NotifyBrowserCreated();
if (opener) {
if (opener && opener->platform_delegate_) {
// 3. Notify the opener browser's platform delegate. With Views this will
// result in a call to CefBrowserViewDelegate::OnPopupBrowserViewCreated().
opener->platform_delegate_->PopupBrowserCreated(browser.get(),
@ -2283,7 +2288,8 @@ void CefBrowserHostImpl::WebContentsCreated(
DCHECK(info->is_popup());
CefRefPtr<CefBrowserHostImpl> opener = GetBrowserForContents(source_contents);
DCHECK(opener.get());
if (!opener.get())
return;
// Popups must share the same BrowserContext as the parent.
CefBrowserContext* browser_context =

51
libcef/browser/browser_info_manager.cc
View File

@ -409,34 +409,55 @@ void CefBrowserInfoManager::GetBrowserInfoList(BrowserInfoList& list) {
void CefBrowserInfoManager::RenderProcessHostDestroyed(
content::RenderProcessHost* host) {
base::AutoLock lock_scope(browser_info_lock_);
const int render_process_id = host->GetID();
DCHECK_GT(render_process_id, 0);
// Remove all pending requests that reference the destroyed host.
PendingNewBrowserInfoList::iterator it =
pending_new_browser_info_list_.begin();
while (it != pending_new_browser_info_list_.end()) {
PendingNewBrowserInfo* info = *it;
if (info->render_process_id == render_process_id)
it = pending_new_browser_info_list_.erase(it);
else
++it;
{
base::AutoLock lock_scope(browser_info_lock_);
PendingNewBrowserInfoList::iterator it =
pending_new_browser_info_list_.begin();
while (it != pending_new_browser_info_list_.end()) {
PendingNewBrowserInfo* info = *it;
if (info->render_process_id == render_process_id)
it = pending_new_browser_info_list_.erase(it);
else
++it;
}
}
// Remove all pending popups that reference the destroyed host as the opener.
{
base::AutoLock lock_scope(pending_popup_lock_);
PendingPopupList::iterator it = pending_popup_list_.begin();
while (it != pending_popup_list_.end()) {
PendingPopup* popup = *it;
if (popup->opener_process_id == render_process_id) {
it = pending_popup_list_.erase(it);
} else {
++it;
}
}
}
}
void CefBrowserInfoManager::FilterPendingPopupURL(
int opener_process_id,
std::unique_ptr<CefBrowserInfoManager::PendingPopup> pending_popup) {
// |host| may be nullptr if the parent browser is destroyed while the popup is
// pending.
content::RenderProcessHost* host =
content::RenderProcessHost::FromID(opener_process_id);
DCHECK(host);
host->FilterURL(false, &pending_popup->target_url);
GetInstance()->PushPendingPopup(std::move(pending_popup));
if (host) {
host->FilterURL(false, &pending_popup->target_url);
GetInstance()->PushPendingPopup(std::move(pending_popup));
}
}
void CefBrowserInfoManager::PushPendingPopup(std::unique_ptr<PendingPopup> popup) {
void CefBrowserInfoManager::PushPendingPopup(
std::unique_ptr<PendingPopup> popup) {
base::AutoLock lock_scope(pending_popup_lock_);
pending_popup_list_.push_back(std::move(popup));
}

3
libcef/renderer/content_renderer_client.cc
View File

@ -829,9 +829,8 @@ void CefContentRendererClient::BrowserCreated(
render_view_routing_id,
render_frame_routing_id,
&params));
DCHECK_GT(params.browser_id, 0);
if (params.browser_id == 0) {
// The request failed for some reason.
// The popup may have been canceled during creation.
return;
}

64
tests/ceftests/request_context_unittest.cc
View File

@ -631,6 +631,12 @@ class PopupNavTestHandler : public TestHandler {
ALLOW_CLOSE_POPUP_LAST,
DENY,
NAVIGATE_AFTER_CREATION,
DESTROY_PARENT_BEFORE_CREATION,
DESTROY_PARENT_BEFORE_CREATION_FORCE,
DESTROY_PARENT_DURING_CREATION,
DESTROY_PARENT_DURING_CREATION_FORCE,
DESTROY_PARENT_AFTER_CREATION,
DESTROY_PARENT_AFTER_CREATION_FORCE,
};
enum RCMode {
RC_MODE_NONE,
@ -700,12 +706,26 @@ class PopupNavTestHandler : public TestHandler {
EXPECT_FALSE(user_gesture);
EXPECT_FALSE(*no_javascript_access);
if (mode_ == DESTROY_PARENT_DURING_CREATION ||
mode_ == DESTROY_PARENT_DURING_CREATION_FORCE) {
// Destroy the main (parent) browser while popup creation is pending.
CloseBrowser(browser, mode_ == DESTROY_PARENT_DURING_CREATION_FORCE);
}
return (mode_ == DENY); // Return true to cancel the popup.
}
void OnAfterCreated(CefRefPtr<CefBrowser> browser) override {
TestHandler::OnAfterCreated(browser);
if (browser->IsPopup() &&
(mode_ == DESTROY_PARENT_AFTER_CREATION ||
mode_ == DESTROY_PARENT_AFTER_CREATION_FORCE)) {
// Destroy the main (parent) browser immediately after the popup is
// created.
CloseBrowser(GetBrowser(), mode_ == DESTROY_PARENT_AFTER_CREATION_FORCE);
}
if (mode_ == NAVIGATE_AFTER_CREATION && browser->IsPopup()) {
// Navigate to the 2nd popup URL instead of the 1st popup URL.
browser->GetMainFrame()->LoadURL(kPopupNavPopupUrl2);
@ -755,6 +775,13 @@ class PopupNavTestHandler : public TestHandler {
frame->ExecuteJavaScript("doPopup()", kPopupNavPageUrl, 0);
if (mode_ == DESTROY_PARENT_BEFORE_CREATION ||
mode_ == DESTROY_PARENT_BEFORE_CREATION_FORCE) {
// Destroy the main (parent) browser immediately before the popup is
// created.
CloseBrowser(browser, mode_ == DESTROY_PARENT_BEFORE_CREATION_FORCE);
}
if (mode_ == DENY) {
// Wait a bit to make sure the popup window isn't created.
CefPostDelayedTask(TID_UI,
@ -796,7 +823,13 @@ class PopupNavTestHandler : public TestHandler {
// Destroy the test after the popup browser closes.
if (browser->IsPopup())
destroy_test = true;
} else if (mode_ == ALLOW_CLOSE_POPUP_LAST) {
} else if (mode_ == ALLOW_CLOSE_POPUP_LAST ||
mode_ == DESTROY_PARENT_BEFORE_CREATION ||
mode_ == DESTROY_PARENT_BEFORE_CREATION_FORCE ||
mode_ == DESTROY_PARENT_DURING_CREATION ||
mode_ == DESTROY_PARENT_DURING_CREATION_FORCE ||
mode_ == DESTROY_PARENT_AFTER_CREATION ||
mode_ == DESTROY_PARENT_AFTER_CREATION_FORCE) {
// Destroy the test after the main browser closes.
if (!browser->IsPopup())
destroy_test = true;
@ -812,7 +845,14 @@ class PopupNavTestHandler : public TestHandler {
EXPECT_TRUE(got_load_start_);
EXPECT_FALSE(got_load_error_);
EXPECT_TRUE(got_load_end_);
EXPECT_TRUE(got_on_before_popup_);
// OnBeforePopup may come before or after browser destruction with the
// DESTROY_PARENT_BEFORE_CREATION* tests.
if (mode_ != DESTROY_PARENT_BEFORE_CREATION &&
mode_ != DESTROY_PARENT_BEFORE_CREATION_FORCE) {
EXPECT_TRUE(got_on_before_popup_);
}
if (mode_ == ALLOW_CLOSE_POPUP_FIRST || mode_ == ALLOW_CLOSE_POPUP_LAST) {
EXPECT_TRUE(got_popup_load_start_);
EXPECT_FALSE(got_popup_load_error_);
@ -820,7 +860,13 @@ class PopupNavTestHandler : public TestHandler {
EXPECT_FALSE(got_popup_load_start2_);
EXPECT_FALSE(got_popup_load_error2_);
EXPECT_FALSE(got_popup_load_end2_);
} else if (mode_ == DENY) {
} else if (mode_ == DENY ||
mode_ == DESTROY_PARENT_BEFORE_CREATION ||
mode_ == DESTROY_PARENT_BEFORE_CREATION_FORCE ||
mode_ == DESTROY_PARENT_DURING_CREATION ||
mode_ == DESTROY_PARENT_DURING_CREATION_FORCE ||
mode_ == DESTROY_PARENT_AFTER_CREATION ||
mode_ == DESTROY_PARENT_AFTER_CREATION_FORCE) {
EXPECT_FALSE(got_popup_load_start_);
EXPECT_FALSE(got_popup_load_error_);
EXPECT_FALSE(got_popup_load_end_);
@ -887,6 +933,18 @@ POPUP_TEST_GROUP(Deny, DENY);
// internal objects are tracked correctly (see issue #1392).
POPUP_TEST_GROUP(NavigateAfterCreation, NAVIGATE_AFTER_CREATION);
// Test destroying the parent browser during or immediately after popup creation
// to verify that internal objects are tracked correctly (see issue #2041).
POPUP_TEST_GROUP(DestroyParentBeforeCreation, DESTROY_PARENT_BEFORE_CREATION);
POPUP_TEST_GROUP(DestroyParentBeforeCreationForce,
DESTROY_PARENT_BEFORE_CREATION_FORCE);
POPUP_TEST_GROUP(DestroyParentDuringCreation, DESTROY_PARENT_DURING_CREATION);
POPUP_TEST_GROUP(DestroyParentDuringCreationForce,
DESTROY_PARENT_DURING_CREATION_FORCE);
POPUP_TEST_GROUP(DestroyParentAfterCreation, DESTROY_PARENT_AFTER_CREATION);
POPUP_TEST_GROUP(DestroyParentAfterCreationForce,
DESTROY_PARENT_AFTER_CREATION_FORCE);
namespace {