From 6dfa7b5383248ab9444aa3b803360c7a9e384773 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jun 2015 13:45:06 +0800 Subject: [PATCH] Don't rely on guest_process_id for searching guest --- atom/browser/atom_browser_client.cc | 128 ++++++++++++---------------- atom/browser/web_view_manager.cc | 50 +++++------ atom/browser/web_view_manager.h | 14 ++- 3 files changed, 80 insertions(+), 112 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 69f745ac7a72..f987cec5ac5e 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -34,22 +34,33 @@ namespace { // Next navigation should not restart renderer process. bool g_suppress_renderer_process_restart = false; -struct FindByProcessId { - explicit FindByProcessId(int child_process_id) - : child_process_id_(child_process_id) { - } - - bool operator() (NativeWindow* const window) { - content::WebContents* web_contents = window->GetWebContents(); - if (!web_contents) - return false; - - int id = window->GetWebContents()->GetRenderProcessHost()->GetID(); - return id == child_process_id_; - } - - int child_process_id_; +// Find out the owner of the child process according to child_process_id. +enum ChildProcessOwner { + OWNER_NATIVE_WINDOW, + OWNER_GUEST_WEB_CONTENTS, + OWNER_NONE, // it might be devtools though. }; +ChildProcessOwner GetChildProcessOwner(int process_id, + NativeWindow** window, + WebViewManager::WebViewInfo* info) { + // First search for NativeWindow. + for (auto native_window : *WindowList::GetInstance()) { + content::WebContents* web_contents = native_window->GetWebContents(); + if (web_contents && + process_id == web_contents->GetRenderProcessHost()->GetID()) { + *window = native_window; + return OWNER_NATIVE_WINDOW; + } + } + + // Then search for guest WebContents. + auto process = content::RenderProcessHost::FromID(process_id); + if (WebViewManager::GetInfoForProcess(process, info)) { + return OWNER_GUEST_WEB_CONTENTS; + } + + return OWNER_NONE; +} } // namespace @@ -82,8 +93,7 @@ content::AccessTokenStore* AtomBrowserClient::CreateAccessTokenStore() { } void AtomBrowserClient::OverrideWebkitPrefs( - content::RenderViewHost* render_view_host, - content::WebPreferences* prefs) { + content::RenderViewHost* host, content::WebPreferences* prefs) { prefs->javascript_enabled = true; prefs->web_security_enabled = true; prefs->javascript_can_open_windows_automatically = true; @@ -102,17 +112,15 @@ void AtomBrowserClient::OverrideWebkitPrefs( prefs->allow_running_insecure_content = false; // Turn off web security for devtools. - auto web_contents = content::WebContents::FromRenderViewHost( - render_view_host); + auto web_contents = content::WebContents::FromRenderViewHost(host); if (web_contents && web_contents->GetURL().SchemeIs("chrome-devtools")) { prefs->web_security_enabled = false; return; } // Custom preferences of guest page. - auto process = render_view_host->GetProcess(); WebViewManager::WebViewInfo info; - if (WebViewManager::GetInfoForProcess(process, &info)) { + if (WebViewManager::GetInfoForWebContents(web_contents, &info)) { prefs->web_security_enabled = !info.disable_web_security; return; } @@ -136,73 +144,47 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation( return; } + // Restart renderer process for all navigations except "javacript:" scheme. + if (url.SchemeIs(url::kJavaScriptScheme)) + return; + if (current_instance->HasProcess()) dying_render_process_ = current_instance->GetProcess(); - - if (!url.SchemeIs(url::kJavaScriptScheme)) { - // Restart renderer process for all navigations except javacript: scheme. - *new_instance = content::SiteInstance::CreateForURL(browser_context, url); - } + *new_instance = content::SiteInstance::CreateForURL(browser_context, url); } void AtomBrowserClient::AppendExtraCommandLineSwitches( base::CommandLine* command_line, - int child_process_id) { + int process_id) { std::string process_type = command_line->GetSwitchValueASCII("type"); if (process_type != "renderer") return; - WindowList* list = WindowList::GetInstance(); - NativeWindow* window = nullptr; + NativeWindow* window; + WebViewManager::WebViewInfo info; + ChildProcessOwner owner = GetChildProcessOwner(process_id, &window, &info); - // Find the owner of this child process. - WindowList::const_iterator iter = std::find_if( - list->begin(), list->end(), FindByProcessId(child_process_id)); - if (iter != list->end()) - window = *iter; - - // If the render process is a newly started one, which means the window still + // If the render process is a newly started one, which means the owner still // uses the old going-to-be-swapped render process, then we try to find the - // window from the swapped render process. - if (!window && dying_render_process_) { - int dying_process_id = dying_render_process_->GetID(); - WindowList::const_iterator iter = std::find_if( - list->begin(), list->end(), FindByProcessId(dying_process_id)); - if (iter != list->end()) { - window = *iter; - child_process_id = dying_process_id; - } else { - // It appears that the dying process doesn't belong to a BrowserWindow, - // then it might be a guest process, if it is we should update its - // process ID in the WebViewManager. - auto child_process = content::RenderProcessHost::FromID(child_process_id); - // Update the process ID in webview guests. - WebViewManager::UpdateGuestProcessID(dying_render_process_, - child_process); - } + // owner from the swapped render process. + if (owner == OWNER_NONE) { + process_id = dying_render_process_->GetID(); + owner = GetChildProcessOwner(process_id, &window, &info); } - if (window) { - window->AppendExtraCommandLineSwitches(command_line, child_process_id); - } else { - // Append commnad line arguments for guest web view. - auto child_process = content::RenderProcessHost::FromID(child_process_id); - WebViewManager::WebViewInfo info; - if (WebViewManager::GetInfoForProcess(child_process, &info)) { - command_line->AppendSwitchASCII( - switches::kGuestInstanceID, - base::IntToString(info.guest_instance_id)); - command_line->AppendSwitchASCII( - switches::kNodeIntegration, - info.node_integration ? "true" : "false"); - if (info.plugins) - command_line->AppendSwitch(switches::kEnablePlugins); - if (!info.preload_script.empty()) - command_line->AppendSwitchPath( - switches::kPreloadScript, - info.preload_script); - } + if (owner == OWNER_NATIVE_WINDOW) { + window->AppendExtraCommandLineSwitches(command_line, process_id); + } else if (owner == OWNER_GUEST_WEB_CONTENTS) { + command_line->AppendSwitchASCII( + switches::kGuestInstanceID, base::IntToString(info.guest_instance_id)); + command_line->AppendSwitchASCII( + switches::kNodeIntegration, info.node_integration ? "true" : "false"); + if (info.plugins) + command_line->AppendSwitch(switches::kEnablePlugins); + if (!info.preload_script.empty()) + command_line->AppendSwitchPath( + switches::kPreloadScript, info.preload_script); } dying_render_process_ = nullptr; diff --git a/atom/browser/web_view_manager.cc b/atom/browser/web_view_manager.cc index 77a772da9adf..0ce31ffe0c8c 100644 --- a/atom/browser/web_view_manager.cc +++ b/atom/browser/web_view_manager.cc @@ -29,23 +29,27 @@ bool WebViewManager::GetInfoForProcess(content::RenderProcessHost* process, auto manager = GetManagerFromProcess(process); if (!manager) return false; - return manager->GetInfo(process->GetID(), info); + base::AutoLock auto_lock(manager->lock_); + for (auto iter : manager->webview_info_map_) + if (iter.first->GetRenderProcessHost() == process) { + *info = iter.second; + return true; + } + return false; } // static -void WebViewManager::UpdateGuestProcessID( - content::RenderProcessHost* old_process, - content::RenderProcessHost* new_process) { - auto manager = GetManagerFromProcess(old_process); - if (manager) { - base::AutoLock auto_lock(manager->lock_); - int old_id = old_process->GetID(); - int new_id = new_process->GetID(); - if (!ContainsKey(manager->webview_info_map_, old_id)) - return; - manager->webview_info_map_[new_id] = manager->webview_info_map_[old_id]; - manager->webview_info_map_.erase(old_id); - } +bool WebViewManager::GetInfoForWebContents(content::WebContents* web_contents, + WebViewInfo* info) { + auto manager = GetManagerFromProcess(web_contents->GetRenderProcessHost()); + if (!manager) + return false; + base::AutoLock auto_lock(manager->lock_); + auto iter = manager->webview_info_map_.find(web_contents); + if (iter == manager->webview_info_map_.end()) + return false; + *info = iter->second; + return true; } WebViewManager::WebViewManager(content::BrowserContext* context) { @@ -61,9 +65,7 @@ void WebViewManager::AddGuest(int guest_instance_id, const WebViewInfo& info) { base::AutoLock auto_lock(lock_); web_contents_embdder_map_[guest_instance_id] = { web_contents, embedder }; - - int guest_process_id = web_contents->GetRenderProcessHost()->GetID(); - webview_info_map_[guest_process_id] = info; + webview_info_map_[web_contents] = info; // Map the element in embedder to guest. int owner_process_id = embedder->GetRenderProcessHost()->GetID(); @@ -78,9 +80,7 @@ void WebViewManager::RemoveGuest(int guest_instance_id) { auto web_contents = web_contents_embdder_map_[guest_instance_id].web_contents; web_contents_embdder_map_.erase(guest_instance_id); - - int guest_process_id = web_contents->GetRenderProcessHost()->GetID(); - webview_info_map_.erase(guest_process_id); + webview_info_map_.erase(web_contents); // Remove the record of element in embedder too. for (const auto& element : element_instance_id_to_guest_map_) @@ -90,16 +90,6 @@ void WebViewManager::RemoveGuest(int guest_instance_id) { } } -bool WebViewManager::GetInfo(int guest_process_id, WebViewInfo* webview_info) { - base::AutoLock auto_lock(lock_); - WebViewInfoMap::iterator iter = webview_info_map_.find(guest_process_id); - if (iter != webview_info_map_.end()) { - *webview_info = iter->second; - return true; - } - return false; -} - content::WebContents* WebViewManager::GetGuestByInstanceID( int owner_process_id, int element_instance_id) { diff --git a/atom/browser/web_view_manager.h b/atom/browser/web_view_manager.h index 2327b2e02b4c..24a33fb6221d 100644 --- a/atom/browser/web_view_manager.h +++ b/atom/browser/web_view_manager.h @@ -34,9 +34,9 @@ class WebViewManager : public content::BrowserPluginGuestManager { static bool GetInfoForProcess(content::RenderProcessHost* process, WebViewInfo* info); - // Updates the guest process ID. - static void UpdateGuestProcessID(content::RenderProcessHost* old_process, - content::RenderProcessHost* new_process); + // Same with GetInfoForProcess but search for |web_contents| instead. + static bool GetInfoForWebContents(content::WebContents* web_contents, + WebViewInfo* info); explicit WebViewManager(content::BrowserContext* context); virtual ~WebViewManager(); @@ -48,10 +48,6 @@ class WebViewManager : public content::BrowserPluginGuestManager { const WebViewInfo& info); void RemoveGuest(int guest_instance_id); - // Looks up the information for the embedder for a given render - // view, if one exists. Called on the IO thread. - bool GetInfo(int guest_process_id, WebViewInfo* webview_info); - protected: // content::BrowserPluginGuestManager: content::WebContents* GetGuestByInstanceID(int owner_process_id, @@ -89,8 +85,8 @@ class WebViewManager : public content::BrowserPluginGuestManager { // (embedder_process_id, element_instance_id) => guest_instance_id std::map element_instance_id_to_guest_map_; - typedef std::map WebViewInfoMap; - // guest_process_id => (guest_instance_id, embedder, ...) + typedef std::map WebViewInfoMap; + // web_contents => (guest_instance_id, embedder, ...) WebViewInfoMap webview_info_map_; base::Lock lock_;