From 543178ce676f83e813a4e4973798d966efdff088 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 11 May 2016 12:18:43 -0400 Subject: [PATCH] Call RenderProcessHost::Send from correct thread (issue #1881) --- libcef/browser/browser_info_manager.cc | 61 +++++++++++++++--------- libcef/browser/browser_info_manager.h | 8 ++-- libcef/browser/browser_message_filter.cc | 49 +++++++++++++------ libcef/browser/browser_message_filter.h | 4 +- libcef/browser/content_browser_client.cc | 2 +- 5 files changed, 81 insertions(+), 43 deletions(-) diff --git a/libcef/browser/browser_info_manager.cc b/libcef/browser/browser_info_manager.cc index a898f51af..f5a195a45 100644 --- a/libcef/browser/browser_info_manager.cc +++ b/libcef/browser/browser_info_manager.cc @@ -18,6 +18,7 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" +#include "content/public/common/child_process_host.h" #include "content/common/view_messages.h" namespace { @@ -102,16 +103,16 @@ scoped_refptr CefBrowserInfoManager::CreatePopupBrowserInfo( // that happens re-visit the implementation of this class. DCHECK_EQ(host, main_frame_host->GetProcess()); - const int render_process_routing_id = host->GetID(); + const int render_process_id = host->GetID(); const int render_view_routing_id = view_host->GetRoutingID(); const int render_frame_routing_id = main_frame_host->GetRoutingID(); scoped_refptr browser_info = new CefBrowserInfo(++next_browser_id_, true); browser_info->render_id_manager()->add_render_view_id( - render_process_routing_id, render_view_routing_id); + render_process_id, render_view_routing_id); browser_info->render_id_manager()->add_render_frame_id( - render_process_routing_id, render_frame_routing_id); + render_process_id, render_frame_routing_id); browser_info_list_.push_back(browser_info); if (is_windowless) @@ -122,11 +123,11 @@ scoped_refptr CefBrowserInfoManager::CreatePopupBrowserInfo( pending_new_browser_info_list_.begin(); for (; it != pending_new_browser_info_list_.end(); ++it) { PendingNewBrowserInfo* info = *it; - if (info->host == host && + if (info->render_process_id == render_process_id && info->render_view_routing_id == render_view_routing_id && info->render_frame_routing_id == render_frame_routing_id) { - SendNewBrowserInfoResponse(host, browser_info.get(), false, - info->reply_msg); + SendNewBrowserInfoResponse(render_process_id, browser_info.get(), + false, info->reply_msg); pending_new_browser_info_list_.erase(it); break; @@ -137,15 +138,16 @@ scoped_refptr CefBrowserInfoManager::CreatePopupBrowserInfo( } void CefBrowserInfoManager::OnCreateWindow( - content::RenderProcessHost* host, + int render_process_id, const ViewHostMsg_CreateWindow_Params& params) { + DCHECK_NE(render_process_id, content::ChildProcessHost::kInvalidUniqueID); DCHECK_GT(params.opener_id, 0); DCHECK_GT(params.opener_render_frame_id, 0); std::unique_ptr pending_popup( new CefBrowserInfoManager::PendingPopup); pending_popup->step = CefBrowserInfoManager::PendingPopup::ON_CREATE_WINDOW; - pending_popup->opener_process_id = host->GetID(); + pending_popup->opener_process_id = render_process_id; pending_popup->opener_view_id = params.opener_id; pending_popup->opener_frame_id = params.opener_render_frame_id; pending_popup->target_url = params.target_url; @@ -320,29 +322,28 @@ void CefBrowserInfoManager::WebContentsCreated( } void CefBrowserInfoManager::OnGetNewBrowserInfo( - content::RenderProcessHost* host, + int render_process_id, int render_view_routing_id, int render_frame_routing_id, IPC::Message* reply_msg) { - DCHECK(host); + DCHECK_NE(render_process_id, content::ChildProcessHost::kInvalidUniqueID); DCHECK_GT(render_view_routing_id, 0); DCHECK_GT(render_frame_routing_id, 0); DCHECK(reply_msg); base::AutoLock lock_scope(browser_info_lock_); - const int render_process_routing_id = host->GetID(); bool is_guest_view = false; scoped_refptr browser_info = GetBrowserInfo( - render_process_routing_id, render_view_routing_id, - render_process_routing_id, render_frame_routing_id, + render_process_id, render_view_routing_id, + render_process_id, render_frame_routing_id, &is_guest_view); if (browser_info.get()) { // Send the response immediately. - SendNewBrowserInfoResponse(host, browser_info.get(), is_guest_view, - reply_msg); + SendNewBrowserInfoResponse(render_process_id, browser_info.get(), + is_guest_view, reply_msg); return; } @@ -353,7 +354,7 @@ void CefBrowserInfoManager::OnGetNewBrowserInfo( pending_new_browser_info_list_.begin(); for (; it != pending_new_browser_info_list_.end(); ++it) { PendingNewBrowserInfo* info = *it; - if (info->host == host && + if (info->render_process_id == render_process_id && info->render_view_routing_id == render_view_routing_id && info->render_frame_routing_id == render_frame_routing_id) { NOTREACHED(); @@ -364,7 +365,7 @@ void CefBrowserInfoManager::OnGetNewBrowserInfo( // Queue the request. std::unique_ptr pending(new PendingNewBrowserInfo()); - pending->host = host; + pending->render_process_id = render_process_id; pending->render_view_routing_id = render_view_routing_id; pending->render_frame_routing_id = render_frame_routing_id; pending->reply_msg = reply_msg; @@ -438,12 +439,14 @@ void CefBrowserInfoManager::RenderProcessHostDestroyed( content::RenderProcessHost* host) { base::AutoLock lock_scope(browser_info_lock_); + const int render_process_id = host->GetID(); + // 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->host == host) + if (info->render_process_id == render_process_id) it = pending_new_browser_info_list_.erase(it); else ++it; @@ -453,10 +456,10 @@ void CefBrowserInfoManager::RenderProcessHostDestroyed( void CefBrowserInfoManager::FilterPendingPopupURL( int render_process_id, std::unique_ptr pending_popup) { - content::RenderProcessHost* rph = + content::RenderProcessHost* host = content::RenderProcessHost::FromID(render_process_id); - DCHECK(rph); - rph->FilterURL(false, &pending_popup->target_url); + DCHECK(host); + host->FilterURL(false, &pending_popup->target_url); GetInstance()->PushPendingPopup(std::move(pending_popup)); } @@ -549,10 +552,24 @@ scoped_refptr CefBrowserInfoManager::GetBrowserInfo( // static void CefBrowserInfoManager::SendNewBrowserInfoResponse( - content::RenderProcessHost* host, + int render_process_id, CefBrowserInfo* browser_info, bool is_guest_view, IPC::Message* reply_msg) { + if (!CEF_CURRENTLY_ON_UIT()) { + CEF_POST_TASK(CEF_UIT, + base::Bind(&CefBrowserInfoManager::SendNewBrowserInfoResponse, + render_process_id, browser_info, is_guest_view, reply_msg)); + return; + } + + content::RenderProcessHost* host = + content::RenderProcessHost::FromID(render_process_id); + if (!host) { + delete reply_msg; + return; + } + CefProcessHostMsg_GetNewBrowserInfo_Params params; params.browser_id = browser_info->browser_id(); params.is_windowless = browser_info->is_windowless(); diff --git a/libcef/browser/browser_info_manager.h b/libcef/browser/browser_info_manager.h index 5ba6dec32..71065c5b0 100644 --- a/libcef/browser/browser_info_manager.h +++ b/libcef/browser/browser_info_manager.h @@ -60,7 +60,7 @@ class CefBrowserInfoManager : public content::RenderProcessHostObserver { // Called from CefBrowserMessageFilter::OnCreateWindow. See comments on // PendingPopup for more information. - void OnCreateWindow(content::RenderProcessHost* host, + void OnCreateWindow(int render_process_id, const ViewHostMsg_CreateWindow_Params& params); // Called from CefContentBrowserClient::CanCreateWindow. See comments on @@ -103,7 +103,7 @@ class CefBrowserInfoManager : public content::RenderProcessHostObserver { // already exist for traditional popup browsers depending on timing. See // comments on PendingPopup for more information. void OnGetNewBrowserInfo( - content::RenderProcessHost* host, + int render_process_id, int render_view_routing_id, int render_frame_routing_id, IPC::Message* reply_msg); @@ -203,14 +203,14 @@ class CefBrowserInfoManager : public content::RenderProcessHostObserver { // Send the response for a pending OnGetNewBrowserInfo request. static void SendNewBrowserInfoResponse( - content::RenderProcessHost* host, + int render_process_id, CefBrowserInfo* browser_info, bool is_guest_view, IPC::Message* reply_msg); // Pending request for OnGetNewBrowserInfo. struct PendingNewBrowserInfo { - content::RenderProcessHost* host; + int render_process_id; int render_view_routing_id; int render_frame_routing_id; IPC::Message* reply_msg; diff --git a/libcef/browser/browser_message_filter.cc b/libcef/browser/browser_message_filter.cc index fff2f931d..dab679bac 100644 --- a/libcef/browser/browser_message_filter.cc +++ b/libcef/browser/browser_message_filter.cc @@ -1,4 +1,4 @@ -/// Copyright (c) 2012 The Chromium Embedded Framework Authors. +// Copyright (c) 2012 The Chromium Embedded Framework Authors. // Portions (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -19,10 +19,10 @@ #include "content/common/frame_messages.h" #include "content/common/view_messages.h" #include "content/public/browser/render_process_host.h" +#include "content/public/common/child_process_host.h" -CefBrowserMessageFilter::CefBrowserMessageFilter( - content::RenderProcessHost* host) - : host_(host), +CefBrowserMessageFilter::CefBrowserMessageFilter(int render_process_id) + : render_process_id_(render_process_id), sender_(NULL) { } @@ -34,7 +34,7 @@ void CefBrowserMessageFilter::OnFilterAdded(IPC::Sender* sender) { } void CefBrowserMessageFilter::OnFilterRemoved() { - host_ = NULL; + render_process_id_ = content::ChildProcessHost::kInvalidUniqueID; sender_ = NULL; } @@ -63,7 +63,21 @@ bool CefBrowserMessageFilter::OnMessageReceived(const IPC::Message& message) { } bool CefBrowserMessageFilter::Send(IPC::Message* message) { - return host_->Send(message); + if (!CEF_CURRENTLY_ON_UIT()) { + CEF_POST_TASK(CEF_UIT, + base::Bind(base::IgnoreResult(&CefBrowserMessageFilter::Send), this, + message)); + return true; + } + + content::RenderProcessHost* host = + content::RenderProcessHost::FromID(render_process_id_); + if (!host) { + delete message; + return false; + } + + return host->Send(message); } void CefBrowserMessageFilter::OnGetNewRenderThreadInfo( @@ -87,17 +101,24 @@ void CefBrowserMessageFilter::OnGetNewBrowserInfo( int render_view_routing_id, int render_frame_routing_id, IPC::Message* reply_msg) { - CefBrowserInfoManager::GetInstance()->OnGetNewBrowserInfo( - host_, - render_view_routing_id, - render_frame_routing_id, - reply_msg); + if (render_process_id_ != content::ChildProcessHost::kInvalidUniqueID) { + CefBrowserInfoManager::GetInstance()->OnGetNewBrowserInfo( + render_process_id_, + render_view_routing_id, + render_frame_routing_id, + reply_msg); + } else { + delete reply_msg; + } } void CefBrowserMessageFilter::OnCreateWindow( const ViewHostMsg_CreateWindow_Params& params, IPC::Message* reply_msg) { - CefBrowserInfoManager::GetInstance()->OnCreateWindow(host_, params); + if (render_process_id_ != content::ChildProcessHost::kInvalidUniqueID) { + CefBrowserInfoManager::GetInstance()->OnCreateWindow(render_process_id_, + params); + } // Reply message is not used. delete reply_msg; @@ -111,11 +132,11 @@ void CefBrowserMessageFilter::OnFrameFocused(int32_t render_frame_routing_id) { return; } - if (!host_) + if (render_process_id_ == content::ChildProcessHost::kInvalidUniqueID) return; CefRefPtr browser = - CefBrowserHostImpl::GetBrowserForFrame(host_->GetID(), + CefBrowserHostImpl::GetBrowserForFrame(render_process_id_, render_frame_routing_id); if (browser.get()) browser->SetFocusedFrame(render_frame_routing_id); diff --git a/libcef/browser/browser_message_filter.h b/libcef/browser/browser_message_filter.h index bb80feba3..b34619015 100644 --- a/libcef/browser/browser_message_filter.h +++ b/libcef/browser/browser_message_filter.h @@ -24,7 +24,7 @@ struct ViewHostMsg_CreateWindow_Params; // This class sends and receives control messages on the browser process. class CefBrowserMessageFilter : public IPC::MessageFilter { public: - explicit CefBrowserMessageFilter(content::RenderProcessHost* host); + explicit CefBrowserMessageFilter(int render_process_id); ~CefBrowserMessageFilter() override; // IPC::ChannelProxy::MessageFilter implementation. @@ -46,7 +46,7 @@ class CefBrowserMessageFilter : public IPC::MessageFilter { IPC::Message* reply_msg); void OnFrameFocused(int32_t render_frame_routing_id); - content::RenderProcessHost* host_; + int render_process_id_; IPC::Sender* sender_; DISALLOW_COPY_AND_ASSIGN(CefBrowserMessageFilter); diff --git a/libcef/browser/content_browser_client.cc b/libcef/browser/content_browser_client.cc index ec3af7628..87992007f 100644 --- a/libcef/browser/content_browser_client.cc +++ b/libcef/browser/content_browser_client.cc @@ -425,7 +425,7 @@ void CefContentBrowserClient::RenderProcessWillLaunch( base::CommandLine::ForCurrentProcess(); const int id = host->GetID(); - host->GetChannel()->AddFilter(new CefBrowserMessageFilter(host)); + host->GetChannel()->AddFilter(new CefBrowserMessageFilter(id)); host->AddFilter(new printing::PrintingMessageFilter(id)); if (!command_line->HasSwitch(switches::kDisableSpellChecking)) {