Browse Source

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
Samuel Attard 4 years ago
parent
commit
b1e2185c5e

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

@@ -769,12 +769,7 @@ content::WebContents* WebContents::OpenURLFromTab(
          params.post_data);
     return nullptr;
   }
-  if (!weak_this)
-    return nullptr;
 
-  // Give user a chance to cancel navigation.
-  if (Emit("will-navigate", params.url))
-    return nullptr;
   if (!weak_this)
     return nullptr;
 

+ 24 - 0
shell/browser/electron_navigation_throttle.cc

@@ -20,6 +20,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 = JavascriptEnvironment::GetIsolate();
+  v8::HandleScope scope(isolate);
+  api::WebContents* api_contents = api::WebContents::From(contents);
+  if (!api_contents) {
+    // 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;
 

+ 39 - 1
spec-main/api-browser-window-spec.ts

@@ -451,7 +451,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();
@@ -515,6 +521,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', () => {