fix: emit will-navigate for sandboxed contents (#22188) (#22328)

shell/browser/common_web_contents_delegate.cc

@@ -282,8 +282,21 @@ content::WebContents* CommonWebContentsDelegate::OpenURLFromTab(
   load_url_params.should_replace_current_entry =
       params.should_replace_current_entry;
   load_url_params.is_renderer_initiated = params.is_renderer_initiated;
+  load_url_params.started_from_context_menu = params.started_from_context_menu;
   load_url_params.initiator_origin = params.initiator_origin;
-  load_url_params.should_clear_history_list = true;
+  load_url_params.source_site_instance = params.source_site_instance;
+  load_url_params.frame_tree_node_id = params.frame_tree_node_id;
+  load_url_params.redirect_chain = params.redirect_chain;
+  load_url_params.has_user_gesture = params.user_gesture;
+  load_url_params.blob_url_loader_factory = params.blob_url_loader_factory;
+  load_url_params.href_translate = params.href_translate;
+  load_url_params.reload_type = params.reload_type;
+
+  if (params.post_data) {
+    load_url_params.load_type =
+        content::NavigationController::LOAD_TYPE_HTTP_POST;
+    load_url_params.post_data = params.post_data;
+  }
 
   source->GetController().LoadURLWithParams(load_url_params);
   return source;

shell/renderer/electron_sandboxed_renderer_client.cc

@@ -296,4 +296,12 @@ void ElectronSandboxedRendererClient::WillReleaseScriptContext(
   InvokeHiddenCallback(context, kLifecycleKey, "onExit");
 }
 
+bool ElectronSandboxedRendererClient::ShouldFork(blink::WebLocalFrame* frame,
+                                                 const GURL& url,
+                                                 const std::string& http_method,
+                                                 bool is_initial_navigation,
+                                                 bool is_server_redirect) {
+  return true;
+}
+
 }  // namespace electron

shell/renderer/electron_sandboxed_renderer_client.h

@@ -37,6 +37,11 @@ class ElectronSandboxedRendererClient : public RendererClientBase {
   void RenderViewCreated(content::RenderView*) override;
   void RunScriptsAtDocumentStart(content::RenderFrame* render_frame) override;
   void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override;
+  bool ShouldFork(blink::WebLocalFrame* frame,
+                  const GURL& url,
+                  const std::string& http_method,
+                  bool is_initial_navigation,
+                  bool is_server_redirect) override;
 
  private:
   std::unique_ptr<base::ProcessMetrics> metrics_;

spec-main/api-browser-window-spec.ts

@@ -419,110 +419,168 @@ describe('BrowserWindow module', () => {
     })
   })
 
-  describe('navigation events', () => {
-    let w = null as unknown as BrowserWindow
-    beforeEach(() => {
-      w = new BrowserWindow({show: false, webPreferences: {nodeIntegration: true}})
-    })
-    afterEach(async () => {
-      await closeWindow(w)
-      w = null as unknown as BrowserWindow
-    })
+  for (const sandbox of [false, true]) {
+    describe(`navigation events${sandbox ? ' with sandbox' : ''}`, () => {
+      let w = null as unknown as BrowserWindow
+      beforeEach(() => {
+        w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: false, sandbox } })
+      })
+      afterEach(async () => {
+        await closeWindow(w)
+        w = null as unknown as BrowserWindow
+      })
 
-    describe('will-navigate event', () => {
-      it('allows the window to be closed from the event listener', (done) => {
-        w.webContents.once('will-navigate', () => {
-          w.close()
-          done()
+      describe('will-navigate event', () => {
+        let server = null as unknown as http.Server
+        let url = null as unknown as string
+        before((done) => {
+          server = http.createServer((req, res) => { res.end('') })
+          server.listen(0, '127.0.0.1', () => {
+            url = `http://127.0.0.1:${(server.address() as AddressInfo).port}/`
+            done()
+          })
         })
-        w.loadFile(path.join(fixtures, 'pages', 'will-navigate.html'))
-      })
-    })
 
-    describe('will-redirect event', () => {
-      let server = null as unknown as http.Server
-      let url = null as unknown as string
-      before((done) => {
-        server = http.createServer((req, res) => {
-          if (req.url === '/302') {
-            res.setHeader('Location', '/200')
-            res.statusCode = 302
-            res.end()
-          } else if (req.url === '/navigate-302') {
-            res.end(`<html><body><script>window.location='${url}/302'</script></body></html>`)
-          } else {
-            res.end()
-          }
+        after(() => {
+          server.close()
         })
-        server.listen(0, '127.0.0.1', () => {
-          url = `http://127.0.0.1:${(server.address() as AddressInfo).port}`
-          done()
+
+        it('allows the window to be closed from the event listener', (done) => {
+          w.webContents.once('will-navigate', () => {
+            w.close()
+            done()
+          })
+          w.loadFile(path.join(fixtures, 'pages', 'will-navigate.html'))
         })
-      })
 
-      after(() => {
-        server.close()
-      })
-      it('is emitted on redirects', (done) => {
-        w.webContents.on('will-redirect', (event, url) => {
-          done()
+        it('can be prevented', (done) => {
+          let willNavigate = false
+          w.webContents.once('will-navigate', (e) => {
+            willNavigate = true
+            e.preventDefault()
+          })
+          w.webContents.on('did-stop-loading', () => {
+            if (willNavigate) {
+              // i.e. it shouldn't have had '?navigated' appended to it.
+              expect(w.webContents.getURL().endsWith('will-navigate.html')).to.be.true()
+              done()
+            }
+          })
+          w.loadFile(path.join(fixtures, 'pages', 'will-navigate.html'))
         })
-        w.loadURL(`${url}/302`)
-      })
 
-      it('is emitted after will-navigate on redirects', (done) => {
-        let navigateCalled = false
-        w.webContents.on('will-navigate', () => {
-          navigateCalled = true
+        it('is triggered when navigating from file: to http:', async () => {
+          await w.loadFile(path.join(fixtures, 'api', 'blank.html'))
+          w.webContents.executeJavaScript(`location.href = ${JSON.stringify(url)}`)
+          const navigatedTo = await new Promise(resolve => {
+            w.webContents.once('will-navigate', (e, url) => {
+              e.preventDefault()
+              resolve(url)
+            })
+          })
+          expect(navigatedTo).to.equal(url)
+          expect(w.webContents.getURL()).to.match(/^file:/)
         })
-        w.webContents.on('will-redirect', (event, url) => {
-          expect(navigateCalled).to.equal(true, 'should have called will-navigate first')
-          done()
+
+        it('is triggered when navigating from about:blank to http:', async () => {
+          await w.loadURL('about:blank')
+          w.webContents.executeJavaScript(`location.href = ${JSON.stringify(url)}`)
+          const navigatedTo = await new Promise(resolve => {
+            w.webContents.once('will-navigate', (e, url) => {
+              e.preventDefault()
+              resolve(url)
+            })
+          })
+          expect(navigatedTo).to.equal(url)
+          expect(w.webContents.getURL()).to.equal('about:blank')
+        })
+      })
+
+      describe('will-redirect event', () => {
+        let server = null as unknown as http.Server
+        let url = null as unknown as string
+        before((done) => {
+          server = http.createServer((req, res) => {
+            if (req.url === '/302') {
+              res.setHeader('Location', '/200')
+              res.statusCode = 302
+              res.end()
+            } else if (req.url === '/navigate-302') {
+              res.end(`<html><body><script>window.location='${url}/302'</script></body></html>`)
+            } else {
+              res.end()
+            }
+          })
+          server.listen(0, '127.0.0.1', () => {
+            url = `http://127.0.0.1:${(server.address() as AddressInfo).port}`
+            done()
+          })
         })
-        w.loadURL(`${url}/navigate-302`)
-      })
 
-      it('is emitted before did-stop-loading on redirects', (done) => {
-        let stopCalled = false
-        w.webContents.on('did-stop-loading', () => {
-          stopCalled = true
+        after(() => {
+          server.close()
         })
-        w.webContents.on('will-redirect', (event, url) => {
-          expect(stopCalled).to.equal(false, 'should not have called did-stop-loading first')
-          done()
+        it('is emitted on redirects', (done) => {
+          w.webContents.on('will-redirect', () => {
+            done()
+          })
+          w.loadURL(`${url}/302`)
         })
-        w.loadURL(`${url}/302`)
-      })
 
-      it('allows the window to be closed from the event listener', (done) => {
-        w.webContents.once('will-redirect', (event, input) => {
-          w.close()
-          done()
+        it('is emitted after will-navigate on redirects', (done) => {
+          let navigateCalled = false
+          w.webContents.on('will-navigate', () => {
+            navigateCalled = true
+          })
+          w.webContents.on('will-redirect', () => {
+            expect(navigateCalled).to.equal(true, 'should have called will-navigate first')
+            done()
+          })
+          w.loadURL(`${url}/navigate-302`)
         })
-        w.loadURL(`${url}/302`)
-      })
 
-      it('can be prevented', (done) => {
-        w.webContents.once('will-redirect', (event) => {
-          event.preventDefault()
-        })
-        w.webContents.on('will-navigate', (e, u) => {
-          expect(u).to.equal(`${url}/302`)
+        it('is emitted before did-stop-loading on redirects', (done) => {
+          let stopCalled = false
+          w.webContents.on('did-stop-loading', () => {
+            stopCalled = true
+          })
+          w.webContents.on('will-redirect', () => {
+            expect(stopCalled).to.equal(false, 'should not have called did-stop-loading first')
+            done()
+          })
+          w.loadURL(`${url}/302`)
         })
-        w.webContents.on('did-stop-loading', () => {
-          expect(w.webContents.getURL()).to.equal(
-            `${url}/navigate-302`,
-            'url should not have changed after navigation event'
-          )
-          done()
+
+        it('allows the window to be closed from the event listener', (done) => {
+          w.webContents.once('will-redirect', () => {
+            w.close()
+            done()
+          })
+          w.loadURL(`${url}/302`)
         })
-        w.webContents.on('will-redirect', (e, u) => {
-          expect(u).to.equal(`${url}/200`)
+
+        it('can be prevented', (done) => {
+          w.webContents.once('will-redirect', (event) => {
+            event.preventDefault()
+          })
+          w.webContents.on('will-navigate', (e, u) => {
+            expect(u).to.equal(`${url}/302`)
+          })
+          w.webContents.on('did-stop-loading', () => {
+            expect(w.webContents.getURL()).to.equal(
+              `${url}/navigate-302`,
+              'url should not have changed after navigation event'
+            )
+            done()
+          })
+          w.webContents.on('will-redirect', (e, u) => {
+            expect(u).to.equal(`${url}/200`)
+          })
+          w.loadURL(`${url}/navigate-302`)
         })
-        w.loadURL(`${url}/navigate-302`)
       })
     })
-  })
+  }
 
   describe('focus and visibility', () => {
     let w = null as unknown as BrowserWindow
@@ -2039,8 +2097,7 @@ describe('BrowserWindow module', () => {
             'did-finish-load',
             'did-frame-finish-load',
             'did-navigate-in-page',
-            // TODO(nornagon): sandboxed pages should also emit will-navigate
-            // 'will-navigate',
+            'will-navigate',
             'did-start-loading',
             'did-stop-loading',
             'did-frame-finish-load',

spec/fixtures/pages/will-navigate.html

@@ -1,7 +1,7 @@
 <html>
 <body>
 <script type="text/javascript" charset="utf-8">
-  location.reload();
+  location.href += '?navigated'
 </script>
 </body>
 </html>