refactor: remove internal navigation controller (#28839)

filenames.auto.gni

@@ -232,7 +232,6 @@ auto_filenames = {
     "lib/browser/ipc-main-internal-utils.ts",
     "lib/browser/ipc-main-internal.ts",
     "lib/browser/message-port-main.ts",
-    "lib/browser/navigation-controller.ts",
     "lib/browser/rpc-server.ts",
     "lib/common/api/clipboard.ts",
     "lib/common/api/deprecate.ts",

lib/browser/api/web-contents.ts

@@ -4,7 +4,6 @@ import type { BrowserWindowConstructorOptions, LoadURLOptions } from 'electron/m
 import * as url from 'url';
 import * as path from 'path';
 import { openGuestWindow, makeWebPreferences, parseContentTypeFormat } from '@electron/internal/browser/guest-window-manager';
-import { NavigationController } from '@electron/internal/browser/navigation-controller';
 import { ipcMainInternal } from '@electron/internal/browser/ipc-main-internal';
 import * as ipcMainUtils from '@electron/internal/browser/ipc-main-internal-utils';
 import { MessagePortMain } from '@electron/internal/browser/message-port-main';
@@ -407,6 +406,80 @@ WebContents.prototype.loadFile = function (filePath, options = {}) {
   }));
 };
 
+WebContents.prototype.loadURL = function (url, options) {
+  if (!options) {
+    options = {};
+  }
+
+  const p = new Promise<void>((resolve, reject) => {
+    const resolveAndCleanup = () => {
+      removeListeners();
+      resolve();
+    };
+    const rejectAndCleanup = (errorCode: number, errorDescription: string, url: string) => {
+      const err = new Error(`${errorDescription} (${errorCode}) loading '${typeof url === 'string' ? url.substr(0, 2048) : url}'`);
+      Object.assign(err, { errno: errorCode, code: errorDescription, url });
+      removeListeners();
+      reject(err);
+    };
+    const finishListener = () => {
+      resolveAndCleanup();
+    };
+    const failListener = (event: Electron.Event, errorCode: number, errorDescription: string, validatedURL: string, isMainFrame: boolean) => {
+      if (isMainFrame) {
+        rejectAndCleanup(errorCode, errorDescription, validatedURL);
+      }
+    };
+
+    let navigationStarted = false;
+    const navigationListener = (event: Electron.Event, url: string, isSameDocument: boolean, isMainFrame: boolean) => {
+      if (isMainFrame) {
+        if (navigationStarted && !isSameDocument) {
+          // the webcontents has started another unrelated navigation in the
+          // main frame (probably from the app calling `loadURL` again); reject
+          // the promise
+          // We should only consider the request aborted if the "navigation" is
+          // actually navigating and not simply transitioning URL state in the
+          // current context.  E.g. pushState and `location.hash` changes are
+          // considered navigation events but are triggered with isSameDocument.
+          // We can ignore these to allow virtual routing on page load as long
+          // as the routing does not leave the document
+          return rejectAndCleanup(-3, 'ERR_ABORTED', url);
+        }
+        navigationStarted = true;
+      }
+    };
+    const stopLoadingListener = () => {
+      // By the time we get here, either 'finish' or 'fail' should have fired
+      // if the navigation occurred. However, in some situations (e.g. when
+      // attempting to load a page with a bad scheme), loading will stop
+      // without emitting finish or fail. In this case, we reject the promise
+      // with a generic failure.
+      // TODO(jeremy): enumerate all the cases in which this can happen. If
+      // the only one is with a bad scheme, perhaps ERR_INVALID_ARGUMENT
+      // would be more appropriate.
+      rejectAndCleanup(-2, 'ERR_FAILED', url);
+    };
+    const removeListeners = () => {
+      this.removeListener('did-finish-load', finishListener);
+      this.removeListener('did-fail-load', failListener);
+      this.removeListener('did-start-navigation', navigationListener);
+      this.removeListener('did-stop-loading', stopLoadingListener);
+      this.removeListener('destroyed', stopLoadingListener);
+    };
+    this.on('did-finish-load', finishListener);
+    this.on('did-fail-load', failListener);
+    this.on('did-start-navigation', navigationListener);
+    this.on('did-stop-loading', stopLoadingListener);
+    this.on('destroyed', stopLoadingListener);
+  });
+  // Add a no-op rejection handler to silence the unhandled rejection error.
+  p.catch(() => {});
+  this._loadURL(url, options);
+  this.emit('load-url', url, options);
+  return p;
+};
+
 WebContents.prototype.setWindowOpenHandler = function (handler: (details: Electron.HandlerDetails) => ({action: 'allow'} | {action: 'deny', overrideBrowserWindowOptions?: BrowserWindowConstructorOptions})) {
   this._windowOpenHandler = handler;
 };
@@ -475,24 +548,6 @@ const loggingEnabled = () => {
 
 // Add JavaScript wrappers for WebContents class.
 WebContents.prototype._init = function () {
-  // The navigation controller.
-  const navigationController = new NavigationController(this);
-  this.loadURL = navigationController.loadURL.bind(navigationController);
-  this.getURL = navigationController.getURL.bind(navigationController);
-  this.stop = navigationController.stop.bind(navigationController);
-  this.reload = navigationController.reload.bind(navigationController);
-  this.reloadIgnoringCache = navigationController.reloadIgnoringCache.bind(navigationController);
-  this.canGoBack = navigationController.canGoBack.bind(navigationController);
-  this.canGoForward = navigationController.canGoForward.bind(navigationController);
-  this.canGoToIndex = navigationController.canGoToIndex.bind(navigationController);
-  this.canGoToOffset = navigationController.canGoToOffset.bind(navigationController);
-  this.clearHistory = navigationController.clearHistory.bind(navigationController);
-  this.goBack = navigationController.goBack.bind(navigationController);
-  this.goForward = navigationController.goForward.bind(navigationController);
-  this.goToIndex = navigationController.goToIndex.bind(navigationController);
-  this.goToOffset = navigationController.goToOffset.bind(navigationController);
-  this.getActiveIndex = navigationController.getActiveIndex.bind(navigationController);
-  this.length = navigationController.length.bind(navigationController);
   // Read off the ID at construction time, so that it's accessible even after
   // the underlying C++ WebContents is destroyed.
   const id = this.id;

lib/browser/navigation-controller.ts

@@ -1,228 +0,0 @@
-import type { WebContents, LoadURLOptions } from 'electron/main';
-import { EventEmitter } from 'events';
-
-// JavaScript implementation of Chromium's NavigationController.
-// Instead of relying on Chromium for history control, we completely 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 every time.
-export class NavigationController extends EventEmitter {
-  currentIndex: number = -1;
-  inPageIndex: number = -1;
-  pendingIndex: number = -1;
-  history: string[] = [];
-
-  constructor (private webContents: WebContents) {
-    super();
-    this.clearHistory();
-
-    // webContents may have already navigated to a page.
-    if (this.webContents._getURL()) {
-      this.currentIndex++;
-      this.history.push(this.webContents._getURL());
-    }
-    this.webContents.on('navigation-entry-committed' as any, (event: Electron.Event, url: string, inPage: boolean, replaceEntry: boolean) => {
-      if (this.inPageIndex > -1 && !inPage) {
-        // Navigated to a new page, clear in-page mark.
-        this.inPageIndex = -1;
-      } else if (this.inPageIndex === -1 && inPage && !replaceEntry) {
-        // Started in-page navigations.
-        this.inPageIndex = this.currentIndex;
-      }
-      if (this.pendingIndex >= 0) {
-        // Go to index.
-        this.currentIndex = this.pendingIndex;
-        this.pendingIndex = -1;
-        this.history[this.currentIndex] = url;
-      } else if (replaceEntry) {
-        // Non-user initialized navigation.
-        this.history[this.currentIndex] = url;
-      } else {
-        // Normal navigation. Clear history.
-        this.history = this.history.slice(0, this.currentIndex + 1);
-        this.currentIndex++;
-        this.history.push(url);
-      }
-    });
-  }
-
-  loadURL (url: string, options?: LoadURLOptions): Promise<void> {
-    if (options == null) {
-      options = {};
-    }
-    const p = new Promise<void>((resolve, reject) => {
-      const resolveAndCleanup = () => {
-        removeListeners();
-        resolve();
-      };
-      const rejectAndCleanup = (errorCode: number, errorDescription: string, url: string) => {
-        const err = new Error(`${errorDescription} (${errorCode}) loading '${typeof url === 'string' ? url.substr(0, 2048) : url}'`);
-        Object.assign(err, { errno: errorCode, code: errorDescription, url });
-        removeListeners();
-        reject(err);
-      };
-      const finishListener = () => {
-        resolveAndCleanup();
-      };
-      const failListener = (event: Electron.Event, errorCode: number, errorDescription: string, validatedURL: string, isMainFrame: boolean) => {
-        if (isMainFrame) {
-          rejectAndCleanup(errorCode, errorDescription, validatedURL);
-        }
-      };
-
-      let navigationStarted = false;
-      const navigationListener = (event: Electron.Event, url: string, isSameDocument: boolean, isMainFrame: boolean) => {
-        if (isMainFrame) {
-          if (navigationStarted && !isSameDocument) {
-            // the webcontents has started another unrelated navigation in the
-            // main frame (probably from the app calling `loadURL` again); reject
-            // the promise
-            // We should only consider the request aborted if the "navigation" is
-            // actually navigating and not simply transitioning URL state in the
-            // current context.  E.g. pushState and `location.hash` changes are
-            // considered navigation events but are triggered with isSameDocument.
-            // We can ignore these to allow virtual routing on page load as long
-            // as the routing does not leave the document
-            return rejectAndCleanup(-3, 'ERR_ABORTED', url);
-          }
-          navigationStarted = true;
-        }
-      };
-      const stopLoadingListener = () => {
-        // By the time we get here, either 'finish' or 'fail' should have fired
-        // if the navigation occurred. However, in some situations (e.g. when
-        // attempting to load a page with a bad scheme), loading will stop
-        // without emitting finish or fail. In this case, we reject the promise
-        // with a generic failure.
-        // TODO(jeremy): enumerate all the cases in which this can happen. If
-        // the only one is with a bad scheme, perhaps ERR_INVALID_ARGUMENT
-        // would be more appropriate.
-        rejectAndCleanup(-2, 'ERR_FAILED', url);
-      };
-      const removeListeners = () => {
-        this.webContents.removeListener('did-finish-load', finishListener);
-        this.webContents.removeListener('did-fail-load', failListener);
-        this.webContents.removeListener('did-start-navigation', navigationListener);
-        this.webContents.removeListener('did-stop-loading', stopLoadingListener);
-        this.webContents.removeListener('destroyed', stopLoadingListener);
-      };
-      this.webContents.on('did-finish-load', finishListener);
-      this.webContents.on('did-fail-load', failListener);
-      this.webContents.on('did-start-navigation', navigationListener);
-      this.webContents.on('did-stop-loading', stopLoadingListener);
-      this.webContents.on('destroyed', stopLoadingListener);
-    });
-    // Add a no-op rejection handler to silence the unhandled rejection error.
-    p.catch(() => {});
-    this.pendingIndex = -1;
-    this.webContents._loadURL(url, options);
-    this.webContents.emit('load-url', url, options);
-    return p;
-  }
-
-  getURL () {
-    if (this.currentIndex === -1) {
-      return '';
-    } else {
-      return this.history[this.currentIndex];
-    }
-  }
-
-  stop () {
-    this.pendingIndex = -1;
-    return this.webContents._stop();
-  }
-
-  reload () {
-    this.pendingIndex = this.currentIndex;
-    return this.webContents._loadURL(this.getURL(), {});
-  }
-
-  reloadIgnoringCache () {
-    this.pendingIndex = this.currentIndex;
-    return this.webContents._loadURL(this.getURL(), {
-      extraHeaders: 'pragma: no-cache\n',
-      reloadIgnoringCache: true
-    });
-  }
-
-  canGoBack () {
-    return this.getActiveIndex() > 0;
-  }
-
-  canGoForward () {
-    return this.getActiveIndex() < this.history.length - 1;
-  }
-
-  canGoToIndex (index: number) {
-    return index >= 0 && index < this.history.length;
-  }
-
-  canGoToOffset (offset: number) {
-    return this.canGoToIndex(this.currentIndex + offset);
-  }
-
-  clearHistory () {
-    this.history = [];
-    this.currentIndex = -1;
-    this.pendingIndex = -1;
-    this.inPageIndex = -1;
-  }
-
-  goBack () {
-    if (!this.canGoBack()) {
-      return;
-    }
-    this.pendingIndex = this.getActiveIndex() - 1;
-    if (this.inPageIndex > -1 && this.pendingIndex >= this.inPageIndex) {
-      return this.webContents._goBack();
-    } else {
-      return this.webContents._loadURL(this.history[this.pendingIndex], {});
-    }
-  }
-
-  goForward () {
-    if (!this.canGoForward()) {
-      return;
-    }
-    this.pendingIndex = this.getActiveIndex() + 1;
-    if (this.inPageIndex > -1 && this.pendingIndex >= this.inPageIndex) {
-      return this.webContents._goForward();
-    } else {
-      return this.webContents._loadURL(this.history[this.pendingIndex], {});
-    }
-  }
-
-  goToIndex (index: number) {
-    if (!this.canGoToIndex(index)) {
-      return;
-    }
-    this.pendingIndex = index;
-    return this.webContents._loadURL(this.history[this.pendingIndex], {});
-  }
-
-  goToOffset (offset: number) {
-    if (!this.canGoToOffset(offset)) {
-      return;
-    }
-    const pendingIndex = this.currentIndex + offset;
-    if (this.inPageIndex > -1 && pendingIndex >= this.inPageIndex) {
-      this.pendingIndex = pendingIndex;
-      return this.webContents._goToOffset(offset);
-    } else {
-      return this.goToIndex(pendingIndex);
-    }
-  }
-
-  getActiveIndex () {
-    if (this.pendingIndex === -1) {
-      return this.currentIndex;
-    } else {
-      return this.pendingIndex;
-    }
-  }
-
-  length () {
-    return this.history.length;
-  }
-}

shell/browser/api/electron_api_web_contents.cc

@@ -1907,6 +1907,10 @@ bool WebContents::Equal(const WebContents* web_contents) const {
   return ID() == web_contents->ID();
 }
 
+GURL WebContents::GetURL() const {
+  return web_contents()->GetLastCommittedURL();
+}
+
 void WebContents::LoadURL(const GURL& url,
                           const gin_helper::Dictionary& options) {
   if (!url.is_valid() || url.spec().size() > url::kMaxURLChars) {
@@ -1957,7 +1961,6 @@ void WebContents::LoadURL(const GURL& url,
   auto weak_this = GetWeakPtr();
 
   params.transition_type = ui::PAGE_TRANSITION_TYPED;
-  params.should_clear_history_list = true;
   params.override_user_agent = content::NavigationController::UA_OVERRIDE_TRUE;
   // Discord non-committed entries to ensure that we don't re-use a pending
   // entry
@@ -1974,6 +1977,23 @@ void WebContents::LoadURL(const GURL& url,
   NotifyUserActivation();
 }
 
+// TODO(MarshallOfSound): Figure out what we need to do with post data here, I
+// believe the default behavior when we pass "true" is to phone out to the
+// delegate and then the controller expects this method to be called again with
+// "false" if the user approves the reload.  For now this would result in
+// ".reload()" calls on POST data domains failing silently.  Passing false would
+// result in them succeeding, but reposting which although more correct could be
+// considering a breaking change.
+void WebContents::Reload() {
+  web_contents()->GetController().Reload(content::ReloadType::NORMAL,
+                                         /* check_for_repost */ true);
+}
+
+void WebContents::ReloadIgnoringCache() {
+  web_contents()->GetController().Reload(content::ReloadType::BYPASSING_CACHE,
+                                         /* check_for_repost */ true);
+}
+
 void WebContents::DownloadURL(const GURL& url) {
   auto* browser_context = web_contents()->GetBrowserContext();
   auto* download_manager =
@@ -1984,10 +2004,6 @@ void WebContents::DownloadURL(const GURL& url) {
   download_manager->DownloadUrl(std::move(download_params));
 }
 
-GURL WebContents::GetURL() const {
-  return web_contents()->GetURL();
-}
-
 std::u16string WebContents::GetTitle() const {
   return web_contents()->GetTitle();
 }
@@ -2008,16 +2024,56 @@ void WebContents::Stop() {
   web_contents()->Stop();
 }
 
+bool WebContents::CanGoBack() const {
+  return web_contents()->GetController().CanGoBack();
+}
+
 void WebContents::GoBack() {
-  web_contents()->GetController().GoBack();
+  if (CanGoBack())
+    web_contents()->GetController().GoBack();
+}
+
+bool WebContents::CanGoForward() const {
+  return web_contents()->GetController().CanGoForward();
 }
 
 void WebContents::GoForward() {
-  web_contents()->GetController().GoForward();
+  if (CanGoForward())
+    web_contents()->GetController().GoForward();
+}
+
+bool WebContents::CanGoToOffset(int offset) const {
+  return web_contents()->GetController().CanGoToOffset(offset);
 }
 
 void WebContents::GoToOffset(int offset) {
-  web_contents()->GetController().GoToOffset(offset);
+  if (CanGoToOffset(offset))
+    web_contents()->GetController().GoToOffset(offset);
+}
+
+bool WebContents::CanGoToIndex(int index) const {
+  return index >= 0 && index < GetHistoryLength();
+}
+
+void WebContents::GoToIndex(int index) {
+  if (CanGoToIndex(index))
+    web_contents()->GetController().GoToIndex(index);
+}
+
+int WebContents::GetActiveIndex() const {
+  return web_contents()->GetController().GetCurrentEntryIndex();
+}
+
+void WebContents::ClearHistory() {
+  // In some rare cases (normally while there is no real history) we are in a
+  // state where we can't prune navigation entries
+  if (web_contents()->GetController().CanPruneAllButLastCommitted()) {
+    web_contents()->GetController().PruneAllButLastCommitted();
+  }
+}
+
+int WebContents::GetHistoryLength() const {
+  return web_contents()->GetController().GetEntryCount();
 }
 
 const std::string WebContents::GetWebRTCIPHandlingPolicy() const {
@@ -3521,16 +3577,26 @@ v8::Local<v8::ObjectTemplate> WebContents::FillObjectTemplate(
       .SetMethod("getOSProcessId", &WebContents::GetOSProcessID)
       .SetMethod("equal", &WebContents::Equal)
       .SetMethod("_loadURL", &WebContents::LoadURL)
+      .SetMethod("reload", &WebContents::Reload)
+      .SetMethod("reloadIgnoringCache", &WebContents::ReloadIgnoringCache)
       .SetMethod("downloadURL", &WebContents::DownloadURL)
-      .SetMethod("_getURL", &WebContents::GetURL)
+      .SetMethod("getURL", &WebContents::GetURL)
       .SetMethod("getTitle", &WebContents::GetTitle)
       .SetMethod("isLoading", &WebContents::IsLoading)
       .SetMethod("isLoadingMainFrame", &WebContents::IsLoadingMainFrame)
       .SetMethod("isWaitingForResponse", &WebContents::IsWaitingForResponse)
-      .SetMethod("_stop", &WebContents::Stop)
-      .SetMethod("_goBack", &WebContents::GoBack)
-      .SetMethod("_goForward", &WebContents::GoForward)
-      .SetMethod("_goToOffset", &WebContents::GoToOffset)
+      .SetMethod("stop", &WebContents::Stop)
+      .SetMethod("canGoBack", &WebContents::CanGoBack)
+      .SetMethod("goBack", &WebContents::GoBack)
+      .SetMethod("canGoForward", &WebContents::CanGoForward)
+      .SetMethod("goForward", &WebContents::GoForward)
+      .SetMethod("canGoToOffset", &WebContents::CanGoToOffset)
+      .SetMethod("goToOffset", &WebContents::GoToOffset)
+      .SetMethod("canGoToIndex", &WebContents::CanGoToIndex)
+      .SetMethod("goToIndex", &WebContents::GoToIndex)
+      .SetMethod("getActiveIndex", &WebContents::GetActiveIndex)
+      .SetMethod("clearHistory", &WebContents::ClearHistory)
+      .SetMethod("length", &WebContents::GetHistoryLength)
       .SetMethod("isCrashed", &WebContents::IsCrashed)
       .SetMethod("forcefullyCrashRenderer",
                  &WebContents::ForcefullyCrashRenderer)

shell/browser/api/electron_api_web_contents.h

@@ -154,6 +154,8 @@ class WebContents : public gin::Wrappable<WebContents>,
   Type GetType() const;
   bool Equal(const WebContents* web_contents) const;
   void LoadURL(const GURL& url, const gin_helper::Dictionary& options);
+  void Reload();
+  void ReloadIgnoringCache();
   void DownloadURL(const GURL& url);
   GURL GetURL() const;
   std::u16string GetTitle() const;
@@ -161,10 +163,17 @@ class WebContents : public gin::Wrappable<WebContents>,
   bool IsLoadingMainFrame() const;
   bool IsWaitingForResponse() const;
   void Stop();
-  void ReloadIgnoringCache();
+  bool CanGoBack() const;
   void GoBack();
+  bool CanGoForward() const;
   void GoForward();
+  bool CanGoToOffset(int offset) const;
   void GoToOffset(int offset);
+  bool CanGoToIndex(int index) const;
+  void GoToIndex(int index);
+  int GetActiveIndex() const;
+  void ClearHistory();
+  int GetHistoryLength() const;
   const std::string GetWebRTCIPHandlingPolicy() const;
   void SetWebRTCIPHandlingPolicy(const std::string& webrtc_ip_handling_policy);
   bool IsCrashed() const;