Skip to content

Commit

Permalink
refactor: wire will-navigate up to a navigation throttle instead of O…
Browse files Browse the repository at this point in the history
…penURL (#25110)

* 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

* Update api-browser-window-spec.ts
  • Loading branch information
MarshallOfSound committed Aug 28, 2020
1 parent 75b9ab7 commit 9f1b24d
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 9 deletions.
6 changes: 0 additions & 6 deletions shell/browser/api/electron_api_web_contents.cc
Expand Up @@ -723,12 +723,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;

Expand Down
24 changes: 24 additions & 0 deletions shell/browser/electron_navigation_throttle.cc
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions shell/browser/electron_navigation_throttle.h
Expand Up @@ -14,6 +14,8 @@ class ElectronNavigationThrottle : public content::NavigationThrottle {
explicit ElectronNavigationThrottle(content::NavigationHandle* handle);
~ElectronNavigationThrottle() override;

ElectronNavigationThrottle::ThrottleCheckResult WillStartRequest() override;

ElectronNavigationThrottle::ThrottleCheckResult WillRedirectRequest()
override;

Expand Down
44 changes: 41 additions & 3 deletions spec-main/api-browser-window-spec.ts
Expand Up @@ -10,7 +10,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 { expect } = chai
Expand Down Expand Up @@ -435,7 +435,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()
Expand Down Expand Up @@ -495,7 +501,39 @@ 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', () => {
let server = null as unknown as http.Server
Expand Down

0 comments on commit 9f1b24d

Please sign in to comment.