Merge pull request electron#1502 from atom/renderer-restart

Manage navigation history with JavaScript on user side
wtest · Apr 27, 2015 · 05ae196 · 05ae196
2 parents ca5ee0f + dde791d
commit 05ae196
Show file tree

Hide file tree

Showing 13 changed files with 192 additions and 105 deletions.
diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
@@ -18,6 +18,7 @@
 #include "atom/common/native_mate_converters/value_converter.h"
 #include "base/strings/utf_string_conversions.h"
 #include "brightray/browser/inspectable_web_contents.h"
+#include "brightray/browser/media/media_stream_devices_controller.h"
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
@@ -30,7 +31,6 @@
 #include "content/public/browser/web_contents.h"
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
-#include "vendor/brightray/browser/media/media_stream_devices_controller.h"
 
 #include "atom/common/node_includes.h"
 
@@ -140,6 +140,7 @@ content::WebContents* WebContents::OpenURLFromTab(
   load_url_params.is_renderer_initiated = params.is_renderer_initiated;
   load_url_params.transferred_global_request_id =
       params.transferred_global_request_id;
+  load_url_params.should_clear_history_list = true;
 
   web_contents()->GetController().LoadURLWithParams(load_url_params);
   return web_contents();
@@ -312,8 +313,7 @@ void WebContents::WebContentsDestroyed() {
 
 void WebContents::NavigationEntryCommitted(
     const content::LoadCommittedDetails& load_details) {
-  auto entry = web_contents()->GetController().GetLastCommittedEntry();
-  entry->SetVirtualURL(load_details.entry->GetOriginalRequestURL());
+  Emit("navigation-entry-commited", load_details.entry->GetURL());
 }
 
 void WebContents::DidAttach(int guest_proxy_routing_id) {
@@ -381,17 +381,11 @@ void WebContents::LoadURL(const GURL& url, const mate::Dictionary& options) {
                                         blink::WebReferrerPolicyDefault);
 
   params.transition_type = ui::PAGE_TRANSITION_TYPED;
+  params.should_clear_history_list = true;
   params.override_user_agent = content::NavigationController::UA_OVERRIDE_TRUE;
   web_contents()->GetController().LoadURLWithParams(params);
 }
 
-GURL WebContents::GetURL() const {
-  auto entry = web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry)
-    return GURL::EmptyGURL();
-  return entry->GetVirtualURL();
-}
-
 base::string16 WebContents::GetTitle() const {
   return web_contents()->GetTitle();
 }
@@ -415,46 +409,8 @@ void WebContents::Stop() {
   web_contents()->Stop();
 }
 
-void WebContents::Reload(const mate::Dictionary& options) {
-  // Navigating to a URL would always restart the renderer process, we want this
-  // because normal reloading will break our node integration.
-  // This is done by AtomBrowserClient::ShouldSwapProcessesForNavigation.
-  LoadURL(GetURL(), options);
-}
-
-void WebContents::ReloadIgnoringCache(const mate::Dictionary& options) {
-  // Hack to remove pending entries that ignores cache and treated as a fresh
-  // load.
+void WebContents::ReloadIgnoringCache() {
   web_contents()->GetController().ReloadIgnoringCache(false);
-  Reload(options);
-}
-
-bool WebContents::CanGoBack() const {
-  return web_contents()->GetController().CanGoBack();
-}
-
-bool WebContents::CanGoForward() const {
-  return web_contents()->GetController().CanGoForward();
-}
-
-bool WebContents::CanGoToOffset(int offset) const {
-  return web_contents()->GetController().CanGoToOffset(offset);
-}
-
-void WebContents::GoBack() {
-  web_contents()->GetController().GoBack();
-}
-
-void WebContents::GoForward() {
-  web_contents()->GetController().GoForward();
-}
-
-void WebContents::GoToIndex(int index) {
-  web_contents()->GetController().GoToIndex(index);
-}
-
-void WebContents::GoToOffset(int offset) {
-  web_contents()->GetController().GoToOffset(offset);
 }
 
 int WebContents::GetRoutingID() const {
@@ -599,21 +555,12 @@ mate::ObjectTemplateBuilder WebContents::GetObjectTemplateBuilder(
         .SetMethod("destroy", &WebContents::Destroy)
         .SetMethod("isAlive", &WebContents::IsAlive)
         .SetMethod("_loadUrl", &WebContents::LoadURL)
-        .SetMethod("getUrl", &WebContents::GetURL)
         .SetMethod("getTitle", &WebContents::GetTitle)
         .SetMethod("getFavicon", &WebContents::GetFavicon)
         .SetMethod("isLoading", &WebContents::IsLoading)
         .SetMethod("isWaitingForResponse", &WebContents::IsWaitingForResponse)
-        .SetMethod("stop", &WebContents::Stop)
-        .SetMethod("_reload", &WebContents::Reload)
+        .SetMethod("_stop", &WebContents::Stop)
         .SetMethod("_reloadIgnoringCache", &WebContents::ReloadIgnoringCache)
-        .SetMethod("canGoBack", &WebContents::CanGoBack)
-        .SetMethod("canGoForward", &WebContents::CanGoForward)
-        .SetMethod("canGoToOffset", &WebContents::CanGoToOffset)
-        .SetMethod("goBack", &WebContents::GoBack)
-        .SetMethod("goForward", &WebContents::GoForward)
-        .SetMethod("goToIndex", &WebContents::GoToIndex)
-        .SetMethod("goToOffset", &WebContents::GoToOffset)
         .SetMethod("getRoutingId", &WebContents::GetRoutingID)
         .SetMethod("getProcessId", &WebContents::GetProcessID)
         .SetMethod("isCrashed", &WebContents::IsCrashed)

diff --git a/atom/browser/api/atom_api_web_contents.h b/atom/browser/api/atom_api_web_contents.h
@@ -47,21 +47,12 @@ class WebContents : public mate::EventEmitter,
   void Destroy();
   bool IsAlive() const;
   void LoadURL(const GURL& url, const mate::Dictionary& options);
-  GURL GetURL() const;
   base::string16 GetTitle() const;
   gfx::Image GetFavicon() const;
   bool IsLoading() const;
   bool IsWaitingForResponse() const;
   void Stop();
-  void Reload(const mate::Dictionary& options);
-  void ReloadIgnoringCache(const mate::Dictionary& options);
-  bool CanGoBack() const;
-  bool CanGoForward() const;
-  bool CanGoToOffset(int offset) const;
-  void GoBack();
-  void GoForward();
-  void GoToIndex(int index);
-  void GoToOffset(int offset);
+  void ReloadIgnoringCache();
   int GetRoutingID() const;
   int GetProcessID() const;
   bool IsCrashed() const;

diff --git a/atom/browser/api/lib/navigation-controller.coffee b/atom/browser/api/lib/navigation-controller.coffee
@@ -0,0 +1,79 @@
+# JavaScript implementation of Chromium's NavigationController.
+# Instead of relying on Chromium for history control, we compeletely do history
+# control on user land, and only rely on WebContents.loadUrl for navigation.
+# This helps us avoid Chromium's various optimizations so we can ensure renderer
+# process is restarted everytime.
+class NavigationController
+  constructor: (@webContents) ->
+    @history = []
+    @currentIndex = -1
+    @pendingIndex = -1
+
+    @webContents.on 'navigation-entry-commited', (event, url) =>
+      if @pendingIndex is -1  # Normal navigation.
+        @history = @history.slice 0, @currentIndex + 1  # Clear history.
+        if @history[@currentIndex] isnt url
+          @currentIndex++
+          @history.push url
+      else  # Go to index.
+        @currentIndex = @pendingIndex
+        @pendingIndex = -1
+        @history[@currentIndex] = url
+
+  loadUrl: (url, options={}) ->
+    @pendingIndex = -1
+    @webContents._loadUrl url, options
+
+  getUrl: ->
+    if @currentIndex is -1
+      ''
+    else
+      @history[@currentIndex]
+
+  stop: ->
+    @pendingIndex = -1
+    @webContents._stop()
+
+  reload: ->
+    @pendingIndex = @currentIndex
+    @webContents._loadUrl @getUrl(), {}
+
+  reloadIgnoringCache: ->
+    @webContents._reloadIgnoringCache()  # Rely on WebContents to clear cache.
+    @reload()
+
+  canGoBack: ->
+    @getActiveIndex() > 0
+
+  canGoForward: ->
+    @getActiveIndex() < @history.length - 1
+
+  canGoToIndex: (index) ->
+    index >=0 and index < @history.length
+
+  canGoToOffset: (offset) ->
+    @canGoToIndex @currentIndex + offset
+
+  goBack: ->
+    return unless @canGoBack()
+    @pendingIndex = @getActiveIndex() - 1
+    @webContents._loadUrl @history[@pendingIndex], {}
+
+  goForward: ->
+    return unless @canGoForward()
+    @pendingIndex = @getActiveIndex() + 1
+    @webContents._loadUrl @history[@pendingIndex], {}
+
+  goToIndex: (index) ->
+    return unless @canGoToIndex index
+    @pendingIndex = index
+    @webContents._loadUrl @history[@pendingIndex], {}
+
+  goToOffset: (offset) ->
+    return unless @canGoToOffset offset
+    @goToIndex @currentIndex + offset
+
+  getActiveIndex: ->
+    if @pendingIndex is -1 then @currentIndex else @pendingIndex
+
+module.exports = NavigationController
diff --git a/atom/browser/api/lib/web-contents.coffee b/atom/browser/api/lib/web-contents.coffee
@@ -1,4 +1,5 @@
 EventEmitter = require('events').EventEmitter
+NavigationController = require './navigation-controller'
 binding = process.atomBinding 'web_contents'
 ipc = require 'ipc'
 
@@ -26,10 +27,12 @@ module.exports.wrap = (webContents) ->
   webContents.getId = -> "#{@getProcessId()}-#{@getRoutingId()}"
   webContents.equal = (other) -> @getId() is other.getId()
 
-  # Provide a default parameter for |urlOptions|.
-  webContents.loadUrl = (url, urlOptions={}) -> @_loadUrl url, urlOptions
-  webContents.reload = (urlOptions={}) -> @_reload urlOptions
-  webContents.reloadIgnoringCache = (urlOptions={}) -> @_reloadIgnoringCache urlOptions
+  # The navigation controller.
+  controller = new NavigationController(webContents)
+  webContents.controller = controller
+  for name, method of NavigationController.prototype when method instanceof Function
+    do (name, method) ->
+      webContents[name] = -> method.apply controller, arguments
 
   # Translate |disposition| to string for 'new-window' event.
   webContents.on '-new-window', (args..., disposition) ->

diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc
@@ -49,7 +49,7 @@ struct FindByProcessId {
 }  // namespace
 
 AtomBrowserClient::AtomBrowserClient()
-    : dying_render_process_(NULL) {
+    : dying_render_process_(nullptr) {
 }
 
 AtomBrowserClient::~AtomBrowserClient() {
@@ -119,27 +119,31 @@ void AtomBrowserClient::OverrideWebkitPrefs(
     window->OverrideWebkitPrefs(prefs);
 }
 
-bool AtomBrowserClient::ShouldSwapBrowsingInstancesForNavigation(
-    content::SiteInstance* site_instance,
-    const GURL& current_url,
-    const GURL& new_url) {
-  if (site_instance->HasProcess())
-    dying_render_process_ = site_instance->GetProcess();
-
-  // Restart renderer process for all navigations, this relies on a patch to
-  // Chromium: http://git.io/_PaNyg.
-  return true;
-}
-
 std::string AtomBrowserClient::GetApplicationLocale() {
   return l10n_util::GetApplicationLocale("");
 }
 
+void AtomBrowserClient::OverrideSiteInstanceForNavigation(
+    content::BrowserContext* browser_context,
+    content::SiteInstance* current_instance,
+    const GURL& url,
+    content::SiteInstance** new_instance) {
+  if (current_instance->HasProcess())
+    dying_render_process_ = current_instance->GetProcess();
+
+  // Restart renderer process for all navigations.
+  *new_instance = content::SiteInstance::CreateForURL(browser_context, url);
+}
+
 void AtomBrowserClient::AppendExtraCommandLineSwitches(
     base::CommandLine* command_line,
     int child_process_id) {
+  std::string process_type = command_line->GetSwitchValueASCII("type");
+  if (process_type != "renderer")
+    return;
+
   WindowList* list = WindowList::GetInstance();
-  NativeWindow* window = NULL;
+  NativeWindow* window = nullptr;
 
   // Find the owner of this child process.
   WindowList::const_iterator iter = std::find_if(
@@ -150,15 +154,25 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
   // If the render process is a newly started one, which means the window still
   // uses the old going-to-be-swapped render process, then we try to find the
   // window from the swapped render process.
-  if (window == NULL && dying_render_process_ != NULL) {
-    child_process_id = dying_render_process_->GetID();
+  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(child_process_id));
-    if (iter != list->end())
+        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);
+    }
   }
 
-  if (window != NULL) {
+  if (window) {
     window->AppendExtraCommandLineSwitches(command_line, child_process_id);
   } else {
     // Append commnad line arguments for guest web view.
@@ -180,7 +194,7 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches(
     }
   }
 
-  dying_render_process_ = NULL;
+  dying_render_process_ = nullptr;
 }
 
 brightray::BrowserMainParts* AtomBrowserClient::OverrideCreateBrowserMainParts(

diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h
@@ -27,11 +27,12 @@ class AtomBrowserClient : public brightray::BrowserClient {
   void ResourceDispatcherHostCreated() override;
   void OverrideWebkitPrefs(content::RenderViewHost* render_view_host,
                            content::WebPreferences* prefs) override;
-  bool ShouldSwapBrowsingInstancesForNavigation(
-      content::SiteInstance* site_instance,
-      const GURL& current_url,
-      const GURL& new_url) override;
   std::string GetApplicationLocale() override;
+  void OverrideSiteInstanceForNavigation(
+      content::BrowserContext* browser_context,
+      content::SiteInstance* current_instance,
+      const GURL& dest_url,
+      content::SiteInstance** new_instance);
   void AppendExtraCommandLineSwitches(base::CommandLine* command_line,
                                       int child_process_id) override;
 

diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc
@@ -565,6 +565,7 @@ content::WebContents* NativeWindow::OpenURLFromTab(
   load_url_params.is_renderer_initiated = params.is_renderer_initiated;
   load_url_params.transferred_global_request_id =
       params.transferred_global_request_id;
+  load_url_params.should_clear_history_list = true;
 
   source->GetController().LoadURLWithParams(load_url_params);
   return source;