Browse Source

refactor: wire will-navigate up to a navigation throttle instead of OpenURL (#25109)

* refactor: wire will-navigate up to a navigation throttle instead of OpenURL (#25065)

* refactor: wire will-navigate up to a navigation throttle instead of OpenURL

* spec: add test for x-site _top navigation

* chore: old code be old
Samuel Attard 4 years ago
parent
commit
945defd47e

+ 0 - 6
shell/browser/api/electron_api_web_contents.cc

@@ -741,12 +741,6 @@ content::WebContents* WebContents::OpenURLFromTab(
     return nullptr;
   }
 
-  // Give user a chance to cancel navigation.
-  if (Emit("will-navigate", params.url))
-    return nullptr;
-
-  // Don't load the URL if the web contents was marked as destroyed from a
-  // will-navigate event listener
   if (IsDestroyed())
     return nullptr;
 

+ 24 - 0
shell/browser/electron_navigation_throttle.cc

@@ -19,6 +19,30 @@ const char* ElectronNavigationThrottle::GetNameForLogging() {
   return "ElectronNavigationThrottle";
 }
 
+content::NavigationThrottle::ThrottleCheckResult
+ElectronNavigationThrottle::WillStartRequest() {
+  auto* handle = navigation_handle();
+  auto* contents = handle->GetWebContents();
+  if (!contents) {
+    NOTREACHED();
+    return PROCEED;
+  }
+
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::HandleScope scope(isolate);
+  auto api_contents = electron::api::WebContents::From(isolate, contents);
+  if (api_contents.IsEmpty()) {
+    // No need to emit any event if the WebContents is not available in JS.
+    return PROCEED;
+  }
+
+  if (handle->IsRendererInitiated() && handle->IsInMainFrame() &&
+      api_contents->EmitNavigationEvent("will-navigate", handle)) {
+    return CANCEL;
+  }
+  return PROCEED;
+}
+
 content::NavigationThrottle::ThrottleCheckResult
 ElectronNavigationThrottle::WillRedirectRequest() {
   auto* handle = navigation_handle();

+ 2 - 0
shell/browser/electron_navigation_throttle.h

@@ -14,6 +14,8 @@ class ElectronNavigationThrottle : public content::NavigationThrottle {
   explicit ElectronNavigationThrottle(content::NavigationHandle* handle);
   ~ElectronNavigationThrottle() override;
 
+  ElectronNavigationThrottle::ThrottleCheckResult WillStartRequest() override;
+
   ElectronNavigationThrottle::ThrottleCheckResult WillRedirectRequest()
       override;
 

+ 40 - 2
spec-main/api-browser-window-spec.ts

@@ -8,7 +8,7 @@ import { AddressInfo } from 'net';
 import { app, BrowserWindow, BrowserView, ipcMain, OnBeforeSendHeadersListenerDetails, protocol, screen, webContents, session, WebContents } from 'electron';
 
 import { emittedOnce } from './events-helpers';
-import { ifit, ifdescribe } from './spec-helpers';
+import { delay, ifit, ifdescribe } from './spec-helpers';
 import { closeWindow } from './window-helpers';
 
 const features = process.electronBinding('features');
@@ -434,7 +434,13 @@ describe('BrowserWindow module', () => {
         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 = http.createServer((req, res) => {
+            if (req.url === '/navigate-top') {
+              res.end('<a target=_top href="/">navigate _top</a>');
+            } else {
+              res.end('');
+            }
+          });
           server.listen(0, '127.0.0.1', () => {
             url = `http://127.0.0.1:${(server.address() as AddressInfo).port}/`;
             done();
@@ -494,6 +500,38 @@ describe('BrowserWindow module', () => {
           expect(navigatedTo).to.equal(url);
           expect(w.webContents.getURL()).to.equal('about:blank');
         });
+
+        it('is triggered when a cross-origin iframe navigates _top', async () => {
+          await w.loadURL(`data:text/html,<iframe src="http://127.0.0.1:${(server.address() as AddressInfo).port}/navigate-top"></iframe>`);
+          await delay(1000);
+          w.webContents.debugger.attach('1.1');
+          const targets = await w.webContents.debugger.sendCommand('Target.getTargets');
+          const iframeTarget = targets.targetInfos.find((t: any) => t.type === 'iframe');
+          const { sessionId } = await w.webContents.debugger.sendCommand('Target.attachToTarget', {
+            targetId: iframeTarget.targetId,
+            flatten: true
+          });
+          await w.webContents.debugger.sendCommand('Input.dispatchMouseEvent', {
+            type: 'mousePressed',
+            x: 10,
+            y: 10,
+            clickCount: 1,
+            button: 'left'
+          }, sessionId);
+          await w.webContents.debugger.sendCommand('Input.dispatchMouseEvent', {
+            type: 'mouseReleased',
+            x: 10,
+            y: 10,
+            clickCount: 1,
+            button: 'left'
+          }, sessionId);
+          let willNavigateEmitted = false;
+          w.webContents.on('will-navigate', () => {
+            willNavigateEmitted = true;
+          });
+          await emittedOnce(w.webContents, 'did-navigate');
+          expect(willNavigateEmitted).to.be.true();
+        });
       });
 
       describe('will-redirect event', () => {