Merge pull request #1502 from atom/renderer-restart

Manage navigation history with JavaScript on user side

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)

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;

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

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) ->

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(

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;
 

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;

atom/browser/web_view_manager.cc

@@ -10,18 +10,42 @@
 
 namespace atom {
 
+namespace {
+
+WebViewManager* GetManagerFromProcess(content::RenderProcessHost* process) {
+  if (!process)
+    return nullptr;
+  auto context = process->GetBrowserContext();
+  if (!context)
+    return nullptr;
+  return static_cast<WebViewManager*>(context->GetGuestManager());
+}
+
+}  // namespace
+
 // static
 bool WebViewManager::GetInfoForProcess(content::RenderProcessHost* process,
                                        WebViewInfo* info) {
-  if (!process)
-    return false;
-  auto context = process->GetBrowserContext();
-  if (!context)
-    return false;
-  auto manager = context->GetGuestManager();
+  auto manager = GetManagerFromProcess(process);
   if (!manager)
     return false;
-  return static_cast<WebViewManager*>(manager)->GetInfo(process->GetID(), info);
+  return manager->GetInfo(process->GetID(), info);
+}
+
+// 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);
+  }
 }
 
 WebViewManager::WebViewManager(content::BrowserContext* context) {

atom/browser/web_view_manager.h

@@ -34,6 +34,10 @@ 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);
+
   explicit WebViewManager(content::BrowserContext* context);
   virtual ~WebViewManager();
 

filenames.gypi

@@ -18,6 +18,7 @@
       'atom/browser/api/lib/ipc.coffee',
       'atom/browser/api/lib/menu.coffee',
       'atom/browser/api/lib/menu-item.coffee',
+      'atom/browser/api/lib/navigation-controller.coffee',
       'atom/browser/api/lib/power-monitor.coffee',
       'atom/browser/api/lib/protocol.coffee',
       'atom/browser/api/lib/screen.coffee',

script/lib/config.py

@@ -7,7 +7,7 @@ import sys
 
 
 BASE_URL = 'http://gh-contractor-zcbenz.s3.amazonaws.com/libchromiumcontent'
-LIBCHROMIUMCONTENT_COMMIT = '0529dc17ca2f950b671cf12f82260cc397b4cebb'
+LIBCHROMIUMCONTENT_COMMIT = '0fb4fbe55f5a967b960644f0fdc2013668d5a46a'
 
 PLATFORM = {
   'cygwin': 'win32',

spec/fixtures/pages/native-module.html

@@ -0,0 +1,9 @@
+<html>
+<body>
+<script type="text/javascript" charset="utf-8">
+  var path = require('path');
+  console.log(typeof require(path.join(__dirname, '..', '..', 'node_modules', 'runas')));
+</script>
+</body>
+</html>
+

spec/webview-spec.coffee

@@ -47,6 +47,19 @@ describe '<webview> tag', ->
       webview.src = "file://#{fixtures}/pages/d.html"
       document.body.appendChild webview
 
+    it 'loads native modules when navigation happens', (done) ->
+      listener = (e) ->
+        webview.removeEventListener 'did-finish-load', listener
+        listener2 = (e) ->
+          assert.equal e.message, 'function'
+          done()
+        webview.addEventListener 'console-message', listener2
+        webview.src = "file://#{fixtures}/pages/native-module.html"
+      webview.addEventListener 'did-finish-load', listener
+      webview.setAttribute 'nodeintegration', 'on'
+      webview.src = "file://#{fixtures}/pages/native-module.html"
+      document.body.appendChild webview
+
   describe 'preload attribute', ->
     it 'loads the script before other scripts in window', (done) ->
       listener = (e) ->