From 6dfa7b5383248ab9444aa3b803360c7a9e384773 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jun 2015 13:45:06 +0800 Subject: [PATCH 1/5] 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_; From d4be2da70ee330f64d5236d151d5f5bb531d07ef Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jun 2015 14:08:56 +0800 Subject: [PATCH 2/5] Don't rely on process_id to search for NativeWindow --- atom/browser/api/atom_api_web_contents.cc | 2 +- atom/browser/atom_browser_client.cc | 53 ++++++++++------------- atom/browser/atom_browser_client.h | 3 -- atom/browser/native_window.cc | 2 +- atom/browser/native_window.h | 3 +- atom/browser/web_view_manager.cc | 28 +++--------- atom/browser/web_view_manager.h | 10 ++--- 7 files changed, 36 insertions(+), 65 deletions(-) diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index cfc8304d0611..e06d2b2419b3 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -85,7 +85,7 @@ v8::Persistent template_; // Get the window that has the |guest| embedded. NativeWindow* GetWindowFromGuest(const content::WebContents* guest) { WebViewManager::WebViewInfo info; - if (WebViewManager::GetInfoForProcess(guest->GetRenderProcessHost(), &info)) + if (WebViewManager::GetInfoForWebContents(guest, &info)) return NativeWindow::FromWebContents(info.embedder); else return nullptr; diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index f987cec5ac5e..50a851bb5248 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -31,33 +31,38 @@ namespace atom { namespace { +// The default routing id of WebContents, since in Electron a WebContents +// always owns its own RenderProcessHost, this ID is same for every WebContents. +int kDefaultRoutingID = 2; + // Next navigation should not restart renderer process. bool g_suppress_renderer_process_restart = false; +// Returns the WebContents from process. +inline content::WebContents* GetWebContentsFromProcess(int process_id) { + return content::WebContents::FromRenderViewHost( + content::RenderViewHost::FromID(process_id, kDefaultRoutingID)); +} + // Find out the owner of the child process according to child_process_id. -enum ChildProcessOwner { +enum WebContentsOwner { OWNER_NATIVE_WINDOW, OWNER_GUEST_WEB_CONTENTS, OWNER_NONE, // it might be devtools though. }; -ChildProcessOwner GetChildProcessOwner(int process_id, - NativeWindow** window, - WebViewManager::WebViewInfo* info) { +WebContentsOwner GetWebContentsOwner(content::WebContents* web_contents, + 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()) { + for (auto native_window : *WindowList::GetInstance()) + if (web_contents == native_window->GetWebContents()) { *window = native_window; return OWNER_NATIVE_WINDOW; } - } // Then search for guest WebContents. - auto process = content::RenderProcessHost::FromID(process_id); - if (WebViewManager::GetInfoForProcess(process, info)) { + if (WebViewManager::GetInfoForWebContents(web_contents, info)) return OWNER_GUEST_WEB_CONTENTS; - } return OWNER_NONE; } @@ -69,8 +74,7 @@ void AtomBrowserClient::SuppressRendererProcessRestartForOnce() { g_suppress_renderer_process_restart = true; } -AtomBrowserClient::AtomBrowserClient() - : dying_render_process_(nullptr) { +AtomBrowserClient::AtomBrowserClient() { } AtomBrowserClient::~AtomBrowserClient() { @@ -148,9 +152,6 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation( if (url.SchemeIs(url::kJavaScriptScheme)) return; - if (current_instance->HasProcess()) - dying_render_process_ = current_instance->GetProcess(); - *new_instance = content::SiteInstance::CreateForURL(browser_context, url); } @@ -161,20 +162,16 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( if (process_type != "renderer") return; + auto web_contents = GetWebContentsFromProcess(process_id); + if (!web_contents) + return; + NativeWindow* window; WebViewManager::WebViewInfo info; - ChildProcessOwner owner = GetChildProcessOwner(process_id, &window, &info); - - // 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 - // owner from the swapped render process. - if (owner == OWNER_NONE) { - process_id = dying_render_process_->GetID(); - owner = GetChildProcessOwner(process_id, &window, &info); - } + WebContentsOwner owner = GetWebContentsOwner(web_contents, &window, &info); if (owner == OWNER_NATIVE_WINDOW) { - window->AppendExtraCommandLineSwitches(command_line, process_id); + window->AppendExtraCommandLineSwitches(command_line); } else if (owner == OWNER_GUEST_WEB_CONTENTS) { command_line->AppendSwitchASCII( switches::kGuestInstanceID, base::IntToString(info.guest_instance_id)); @@ -186,8 +183,6 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( command_line->AppendSwitchPath( switches::kPreloadScript, info.preload_script); } - - dying_render_process_ = nullptr; } void AtomBrowserClient::DidCreatePpapiPlugin( diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index a505daaea5cc..6aebc3128a48 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -46,9 +46,6 @@ class AtomBrowserClient : public brightray::BrowserClient { brightray::BrowserMainParts* OverrideCreateBrowserMainParts( const content::MainFunctionParams&) override; - // The render process which would be swapped out soon. - content::RenderProcessHost* dying_render_process_; - DISALLOW_COPY_AND_ASSIGN(AtomBrowserClient); }; diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 7fb0de344472..01916781ee6d 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -422,7 +422,7 @@ content::WebContents* NativeWindow::GetDevToolsWebContents() const { } void NativeWindow::AppendExtraCommandLineSwitches( - base::CommandLine* command_line, int child_process_id) { + base::CommandLine* command_line) { // Append --node-integration to renderer process. command_line->AppendSwitchASCII(switches::kNodeIntegration, node_integration_ ? "true" : "false"); diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index ce829c71b533..8f9d8e95a34e 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -192,8 +192,7 @@ class NativeWindow : public brightray::DefaultWebContentsDelegate, content::WebContents* GetDevToolsWebContents() const; // Called when renderer process is going to be started. - void AppendExtraCommandLineSwitches(base::CommandLine* command_line, - int child_process_id); + void AppendExtraCommandLineSwitches(base::CommandLine* command_line); void OverrideWebkitPrefs(content::WebPreferences* prefs); // Set fullscreen mode triggered by html api. diff --git a/atom/browser/web_view_manager.cc b/atom/browser/web_view_manager.cc index 0ce31ffe0c8c..7dba6c06dd38 100644 --- a/atom/browser/web_view_manager.cc +++ b/atom/browser/web_view_manager.cc @@ -12,10 +12,9 @@ namespace atom { namespace { -WebViewManager* GetManagerFromProcess(content::RenderProcessHost* process) { - if (!process) - return nullptr; - auto context = process->GetBrowserContext(); +WebViewManager* GetManagerFromWebContents( + const content::WebContents* web_contents) { + auto context = web_contents->GetBrowserContext(); if (!context) return nullptr; return static_cast(context->GetGuestManager()); @@ -24,24 +23,9 @@ WebViewManager* GetManagerFromProcess(content::RenderProcessHost* process) { } // namespace // static -bool WebViewManager::GetInfoForProcess(content::RenderProcessHost* process, - WebViewInfo* info) { - auto manager = GetManagerFromProcess(process); - if (!manager) - return false; - 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 -bool WebViewManager::GetInfoForWebContents(content::WebContents* web_contents, - WebViewInfo* info) { - auto manager = GetManagerFromProcess(web_contents->GetRenderProcessHost()); +bool WebViewManager::GetInfoForWebContents( + const content::WebContents* web_contents, WebViewInfo* info) { + auto manager = GetManagerFromWebContents(web_contents); if (!manager) return false; base::AutoLock auto_lock(manager->lock_); diff --git a/atom/browser/web_view_manager.h b/atom/browser/web_view_manager.h index 24a33fb6221d..b1bea92702cc 100644 --- a/atom/browser/web_view_manager.h +++ b/atom/browser/web_view_manager.h @@ -29,13 +29,9 @@ class WebViewManager : public content::BrowserPluginGuestManager { base::FilePath preload_script; }; - // Finds the WebViewManager attached with |process| and returns the + // Finds the WebViewManager attached with |web_contents| and returns the // WebViewInfo of it. - static bool GetInfoForProcess(content::RenderProcessHost* process, - WebViewInfo* info); - - // Same with GetInfoForProcess but search for |web_contents| instead. - static bool GetInfoForWebContents(content::WebContents* web_contents, + static bool GetInfoForWebContents(const content::WebContents* web_contents, WebViewInfo* info); explicit WebViewManager(content::BrowserContext* context); @@ -85,7 +81,7 @@ 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; + typedef std::map WebViewInfoMap; // web_contents => (guest_instance_id, embedder, ...) WebViewInfoMap webview_info_map_; From 34cd1435b47877686398b9acf52cbcabfcc439f2 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jun 2015 14:17:11 +0800 Subject: [PATCH 3/5] Clean up code --- atom/browser/atom_browser_client.cc | 33 ++++++++++------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 50a851bb5248..b8f63bc604a3 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -38,21 +38,20 @@ int kDefaultRoutingID = 2; // Next navigation should not restart renderer process. bool g_suppress_renderer_process_restart = false; -// Returns the WebContents from process. -inline content::WebContents* GetWebContentsFromProcess(int process_id) { - return content::WebContents::FromRenderViewHost( - content::RenderViewHost::FromID(process_id, kDefaultRoutingID)); -} - // Find out the owner of the child process according to child_process_id. -enum WebContentsOwner { +enum ProcessOwner { OWNER_NATIVE_WINDOW, OWNER_GUEST_WEB_CONTENTS, OWNER_NONE, // it might be devtools though. }; -WebContentsOwner GetWebContentsOwner(content::WebContents* web_contents, - NativeWindow** window, - WebViewManager::WebViewInfo* info) { +ProcessOwner GetProcessOwner(int process_id, + NativeWindow** window, + WebViewManager::WebViewInfo* info) { + auto web_contents = content::WebContents::FromRenderViewHost( + content::RenderViewHost::FromID(process_id, kDefaultRoutingID)); + if (!web_contents) + return OWNER_NONE; + // First search for NativeWindow. for (auto native_window : *WindowList::GetInstance()) if (web_contents == native_window->GetWebContents()) { @@ -115,14 +114,8 @@ void AtomBrowserClient::OverrideWebkitPrefs( prefs->allow_displaying_insecure_content = false; prefs->allow_running_insecure_content = false; - // Turn off web security for devtools. - 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 web_contents = content::WebContents::FromRenderViewHost(host); WebViewManager::WebViewInfo info; if (WebViewManager::GetInfoForWebContents(web_contents, &info)) { prefs->web_security_enabled = !info.disable_web_security; @@ -162,13 +155,9 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( if (process_type != "renderer") return; - auto web_contents = GetWebContentsFromProcess(process_id); - if (!web_contents) - return; - NativeWindow* window; WebViewManager::WebViewInfo info; - WebContentsOwner owner = GetWebContentsOwner(web_contents, &window, &info); + ProcessOwner owner = GetProcessOwner(process_id, &window, &info); if (owner == OWNER_NATIVE_WINDOW) { window->AppendExtraCommandLineSwitches(command_line); From 95a8f3fc7063c3221dbd2430330cbfea5ae076d4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jun 2015 14:33:22 +0800 Subject: [PATCH 4/5] Fix changing src would calling loadUrl for twice --- .../lib/web-view/web-view-attributes.coffee | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/atom/renderer/lib/web-view/web-view-attributes.coffee b/atom/renderer/lib/web-view/web-view-attributes.coffee index d2ea629f50d7..37002aefc4b7 100644 --- a/atom/renderer/lib/web-view/web-view-attributes.coffee +++ b/atom/renderer/lib/web-view/web-view-attributes.coffee @@ -28,7 +28,7 @@ class WebViewAttribute # Changes the attribute's value without triggering its mutation handler. setValueIgnoreMutation: (value) -> @ignoreMutation = true - @webViewImpl.webviewNode.setAttribute(@name, value || '') + @setValue value @ignoreMutation = false # Defines this attribute as a property on the webview node. @@ -119,6 +119,14 @@ class SrcAttribute extends WebViewAttribute else '' + setValueIgnoreMutation: (value) -> + WebViewAttribute::setValueIgnoreMutation value + # takeRecords() is needed to clear queued up src mutations. Without it, it + # is possible for this change to get picked up asyncronously by src's + # mutation observer |observer|, and then get handled even though we do not + # want to handle this mutation. + @observer.takeRecords() + handleMutation: (oldValue, newValue) -> # Once we have navigated, we don't allow clearing the src attribute. # Once enters a navigated state, it cannot return to a @@ -138,7 +146,10 @@ class SrcAttribute extends WebViewAttribute setupMutationObserver: -> @observer = new MutationObserver (mutations) => for mutation in mutations - @handleMutation mutation.oldValue, @getValue() + oldValue = mutation.oldValue + newValue = @getValue() + return if oldValue isnt newValue + @handleMutation oldValue, newValue params = attributes: true, attributeOldValue: true, From 3a3b05b2f06f108885f11a6c21015117371dd25b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jun 2015 14:53:19 +0800 Subject: [PATCH 5/5] Clean up code --- atom/browser/atom_browser_client.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index b8f63bc604a3..88aa5b77755c 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -31,22 +31,23 @@ namespace atom { namespace { -// The default routing id of WebContents, since in Electron a WebContents -// always owns its own RenderProcessHost, this ID is same for every WebContents. +// The default routing id of WebContents. +// In Electron each RenderProcessHost only has one WebContents, so this ID is +// same for every WebContents. int kDefaultRoutingID = 2; // Next navigation should not restart renderer process. bool g_suppress_renderer_process_restart = false; -// Find out the owner of the child process according to child_process_id. +// Find out the owner of the child process according to |process_id|. enum ProcessOwner { OWNER_NATIVE_WINDOW, OWNER_GUEST_WEB_CONTENTS, OWNER_NONE, // it might be devtools though. }; ProcessOwner GetProcessOwner(int process_id, - NativeWindow** window, - WebViewManager::WebViewInfo* info) { + NativeWindow** window, + WebViewManager::WebViewInfo* info) { auto web_contents = content::WebContents::FromRenderViewHost( content::RenderViewHost::FromID(process_id, kDefaultRoutingID)); if (!web_contents)