From cb6f9945e314bbee7b87cce74011413f8fab12a1 Mon Sep 17 00:00:00 2001 From: Maja Frydrychowicz Date: Fri, 24 Apr 2020 16:50:44 +0000 Subject: [PATCH] Bug 1599257 - [remote] Block Page.navigate until navigation request is done r=remote-protocol-reviewers,whimboo Differential Revision: https://phabricator.services.mozilla.com/D71657 --- remote/domains/parent/Page.jsm | 67 +++++- .../parent/network/NetworkObserver.jsm | 1 + remote/test/browser/browser_cdp.js | 2 +- .../test/browser/dom/browser_describeNode.js | 6 +- remote/test/browser/page/browser.ini | 1 + .../page/browser_createIsolatedWorld.js | 5 +- .../browser/page/browser_frameNavigated.js | 2 +- remote/test/browser/page/browser_navigate.js | 194 ++++++++++++++++++ .../browser/page/browser_runtimeEvents.js | 2 +- .../browser_scriptToEvaluateOnNewDocument.js | 3 + .../runtime/browser_executionContext.js | 2 +- 11 files changed, 269 insertions(+), 16 deletions(-) create mode 100644 remote/test/browser/page/browser_navigate.js diff --git a/remote/domains/parent/Page.jsm b/remote/domains/parent/Page.jsm index 69a03e0b9a25..26af2823291a 100644 --- a/remote/domains/parent/Page.jsm +++ b/remote/domains/parent/Page.jsm @@ -94,26 +94,85 @@ class Page extends Domain { * intended transition type * @return {Object} * - frameId {string} frame id that has navigated (or failed to) - * - errorText {string} error message if navigation has failed - * (not supported) + * - errorText {string=} error message if navigation has failed * - loaderId {string} (not supported) */ async navigate(options = {}) { const { url, frameId, referrer, transitionType } = options; + if (typeof url != "string") { + throw new TypeError("url: string value expected"); + } + let validURL; + try { + validURL = Services.io.newURI(url); + } catch (e) { + throw new Error("Error: Cannot navigate to invalid URL"); + } if (frameId && frameId != this.session.browsingContext.id.toString()) { throw new UnsupportedError("frameId not supported"); } + const requestDone = new Promise(resolve => { + if (validURL.scheme == "data" || validURL.scheme == "about") { + resolve({}); + return; + } + let navigationRequestId, redirectedRequestId; + const _onNavigationRequest = function(_type, _ch, data) { + const { + url: requestURL, + requestId, + redirectedFrom = null, + isNavigationRequest, + } = data; + if (!isNavigationRequest) { + return; + } + if (validURL.spec === requestURL) { + navigationRequestId = redirectedRequestId = requestId; + } else if (redirectedFrom === redirectedRequestId) { + redirectedRequestId = requestId; + } + }; + + const _onRequestFinished = function(_type, _ch, data) { + const { requestId, errorCode } = data; + if ( + redirectedRequestId !== requestId || + errorCode == "NS_BINDING_REDIRECTED" + ) { + // handle next request in redirection chain + return; + } + this.session.networkObserver.off("request", _onNavigationRequest); + this.session.networkObserver.off("requestfinished", _onRequestFinished); + resolve({ errorCode, navigationRequestId }); + }.bind(this); + + this.session.networkObserver.on("request", _onNavigationRequest); + this.session.networkObserver.on("requestfinished", _onRequestFinished); + }); + const opts = { loadFlags: transitionToLoadFlag(transitionType), referrerURI: referrer, triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(), }; this.session.browsingContext.loadURI(url, opts); - - return { + // clients expect loaderId == requestId for a document navigation request + const { + // TODO as part of Bug 1599260 + // navigationRequestId: loaderId, + errorCode, + } = await requestDone; + const result = { frameId: this.session.browsingContext.id.toString(), + // loaderId, }; + if (errorCode) { + result.errorText = errorCode; + } + return result; } /** diff --git a/remote/domains/parent/network/NetworkObserver.jsm b/remote/domains/parent/network/NetworkObserver.jsm index 4acd0100e209..05713de3b9f6 100644 --- a/remote/domains/parent/network/NetworkObserver.jsm +++ b/remote/domains/parent/network/NetworkObserver.jsm @@ -283,6 +283,7 @@ class NetworkObserver { responseStorage.addResponseBody(httpChannel, body); this.emit("requestfinished", httpChannel, { requestId: requestId(httpChannel), + errorCode: getNetworkErrorStatusText(httpChannel.status), }); } diff --git a/remote/test/browser/browser_cdp.js b/remote/test/browser/browser_cdp.js index c7adfdd9fc83..0aac52831ba4 100644 --- a/remote/test/browser/browser_cdp.js +++ b/remote/test/browser/browser_cdp.js @@ -46,7 +46,7 @@ add_task(async function testCDP({ client }) { await Page.navigate({ url: toDataURL(``), }); - info("A new page has been loaded"); + info("A new page has been requested"); await loadEventFired; info("`Page.loadEventFired` fired"); diff --git a/remote/test/browser/dom/browser_describeNode.js b/remote/test/browser/dom/browser_describeNode.js index f10248542f12..bb354ae6ab46 100644 --- a/remote/test/browser/dom/browser_describeNode.js +++ b/remote/test/browser/dom/browser_describeNode.js @@ -100,11 +100,9 @@ add_task(async function objectIdNoAttributes({ client }) { }); add_task(async function objectIdDiffersForDifferentNodes({ client }) { - const { DOM, Page, Runtime } = client; + const { DOM, Runtime } = client; - await Page.enable(); - await Page.navigate({ url: DOC }); - await Page.loadEventFired(); + await loadURL(DOC); await Runtime.enable(); const { result: doc } = await Runtime.evaluate({ diff --git a/remote/test/browser/page/browser.ini b/remote/test/browser/page/browser.ini index 760ec8fd8130..6eb285c3b5c7 100644 --- a/remote/test/browser/page/browser.ini +++ b/remote/test/browser/page/browser.ini @@ -23,6 +23,7 @@ support-files = [browser_javascriptDialog_otherTarget.js] [browser_javascriptDialog_prompt.js] [browser_lifecycleEvent.js] +[browser_navigate.js] [browser_navigateToHistoryEntry.js] [browser_printToPDF.js] [browser_reload.js] diff --git a/remote/test/browser/page/browser_createIsolatedWorld.js b/remote/test/browser/page/browser_createIsolatedWorld.js index 5553cbbe9b18..45273f526792 100644 --- a/remote/test/browser/page/browser_createIsolatedWorld.js +++ b/remote/test/browser/page/browser_createIsolatedWorld.js @@ -47,10 +47,7 @@ add_task(async function contextCreatedAfterNavigation({ client }) { info("Page notifications are enabled"); info("Navigating..."); const { frameId } = await Page.navigate({ url: DOC }); - - // Workaround for Bug 1603776 TODO - const { frame } = await Page.frameNavigated(); - is(frame.url, DOC, "Navigated to expected url"); + await Page.loadEventFired(); const { executionContextId: isolatedId } = await Page.createIsolatedWorld({ frameId, diff --git a/remote/test/browser/page/browser_frameNavigated.js b/remote/test/browser/page/browser_frameNavigated.js index 06217209e1f4..f569492b8866 100644 --- a/remote/test/browser/page/browser_frameNavigated.js +++ b/remote/test/browser/page/browser_frameNavigated.js @@ -47,7 +47,7 @@ add_task(async function({ client }) { const url = RANDOM_ID_DOC; const { frameId } = await Page.navigate({ url }); - info("A new page has been loaded"); + info("A new page has been requested"); ok(frameId, "Page.navigate returned a frameId"); is( diff --git a/remote/test/browser/page/browser_navigate.js b/remote/test/browser/page/browser_navigate.js new file mode 100644 index 000000000000..ccc6c0ed29f0 --- /dev/null +++ b/remote/test/browser/page/browser_navigate.js @@ -0,0 +1,194 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const pageEmptyURL = + "http://example.com/browser/remote/test/browser/page/doc_empty.html"; + +add_task(async function testBasicNavigation({ client }) { + const { Page } = client; + await Page.enable(); + const loadEventFired = Page.loadEventFired(); + const { frameId, loaderId, errorText } = await Page.navigate({ + url: pageEmptyURL, + }); + + await compareFrame(frameId); + todo(!!loaderId, "Page.navigate returns loaderId"); + is(errorText, undefined, "No errorText on a successful navigation"); + + await loadEventFired; + is( + gBrowser.selectedBrowser.currentURI.spec, + pageEmptyURL, + "Expected URL loaded" + ); +}); + +add_task(async function testTwoNavigations({ client }) { + const { Page } = client; + await Page.enable(); + let loadEventFired = Page.loadEventFired(); + const { frameId, loaderId, errorText } = await Page.navigate({ + url: pageEmptyURL, + }); + await loadEventFired; + is( + gBrowser.selectedBrowser.currentURI.spec, + pageEmptyURL, + "Expected URL loaded" + ); + + loadEventFired = Page.loadEventFired(); + const { + frameId: frameId2, + loaderId: loaderId2, + errorText: errorText2, + } = await Page.navigate({ + url: pageEmptyURL, + }); + todo(loaderId !== loaderId2, "Page.navigate returns different loaderIds"); + is(errorText, undefined, "No errorText on a successful navigation"); + is(errorText2, undefined, "No errorText on a successful navigation"); + is(frameId, frameId2, "Page.navigate return same frameId"); + + await loadEventFired; + is( + gBrowser.selectedBrowser.currentURI.spec, + pageEmptyURL, + "Expected URL loaded" + ); +}); + +add_task(async function testRedirect({ client }) { + const { Page } = client; + const sjsURL = + "http://example.com/browser/remote/test/browser/page/sjs_redirect.sjs"; + const redirectURL = `${sjsURL}?${pageEmptyURL}`; + await Page.enable(); + const loadEventFired = Page.loadEventFired(); + const { frameId, loaderId, errorText } = await Page.navigate({ + url: redirectURL, + }); + todo(!!loaderId, "Page.navigate returns loaderId"); + is(errorText, undefined, "No errorText on a successful navigation"); + ok(!!frameId, "Page.navigate returns frameId"); + + await loadEventFired; + is( + gBrowser.selectedBrowser.currentURI.spec, + pageEmptyURL, + "Expected URL loaded" + ); +}); + +add_task(async function testUnknownHost({ client }) { + const { Page } = client; + const { frameId, loaderId, errorText } = await Page.navigate({ + url: "http://example-does-not-exist.com", + }); + ok(!!frameId, "Page.navigate returns frameId"); + todo(!!loaderId, "Page.navigate returns loaderId"); + is(errorText, "NS_ERROR_UNKNOWN_HOST", "Failed navigation returns errorText"); +}); + +add_task(async function testExpiredCertificate({ client }) { + const { Page } = client; + const { frameId, loaderId, errorText } = await Page.navigate({ + url: "https://expired.example.com", + }); + ok(!!frameId, "Page.navigate returns frameId"); + todo(!!loaderId, "Page.navigate returns loaderId"); + is( + errorText, + "SEC_ERROR_EXPIRED_CERTIFICATE", + "Failed navigation returns errorText" + ); +}); + +add_task(async function testUnknownCertificate({ client }) { + const { Page } = client; + const { frameId, loaderId, errorText } = await Page.navigate({ + url: "https://self-signed.example.com", + }); + ok(!!frameId, "Page.navigate returns frameId"); + todo(!!loaderId, "Page.navigate returns loaderId"); + is(errorText, "SSL_ERROR_UNKNOWN", "Failed navigation returns errorText"); +}); + +add_task(async function testNotFound({ client }) { + const { Page } = client; + const { frameId, loaderId, errorText } = await Page.navigate({ + url: "http://example.com/browser/remote/doesnotexist.html", + }); + ok(!!frameId, "Page.navigate returns frameId"); + todo(!!loaderId, "Page.navigate returns loaderId"); + is(errorText, undefined, "No errorText on a 404"); +}); + +add_task(async function testInvalidURL({ client }) { + const { Page } = client; + let message = ""; + for (let url of ["blah.com", "foo", "https\n//", "http", ""]) { + message = ""; + try { + await Page.navigate({ url }); + } catch (e) { + message = e.response.message; + } + ok(message.includes("invalid URL"), `Invalid url ${url} causes error`); + } + + for (let url of [2, {}, true]) { + message = ""; + try { + await Page.navigate({ url }); + } catch (e) { + message = e.response.message; + } + ok( + message.includes("string value expected"), + `Invalid url ${url} causes error` + ); + } +}); + +add_task(async function testDataURL({ client }) { + const { Page } = client; + const url = toDataURL("first"); + await Page.enable(); + const loadEventFired = Page.loadEventFired(); + const { frameId, loaderId, errorText } = await Page.navigate({ url }); + await compareFrame(frameId); + is(errorText, undefined, "No errorText on a successful navigation"); + todo(!!loaderId, "Page.navigate returns loaderId"); + + await loadEventFired; + is(gBrowser.selectedBrowser.currentURI.spec, url, "Expected URL loaded"); +}); + +add_task(async function testAbout({ client }) { + const { Page } = client; + await Page.enable(); + let loadEventFired = Page.loadEventFired(); + const { frameId, loaderId, errorText } = await Page.navigate({ + url: "about:blank", + }); + todo(!!loaderId, "Page.navigate returns loaderId"); + is(errorText, undefined, "No errorText on a successful navigation"); + await compareFrame(frameId); + + await loadEventFired; + is( + gBrowser.selectedBrowser.currentURI.spec, + "about:blank", + "Expected URL loaded" + ); +}); + +async function compareFrame(frameId) { + const frames = await getFlattendFrameList(); + const currentFrame = Array.from(frames.values())[0]; + is(frameId, currentFrame.id, "Page.navigate returns expected frameId"); +} diff --git a/remote/test/browser/page/browser_runtimeEvents.js b/remote/test/browser/page/browser_runtimeEvents.js index 6f2d539a6145..8869ddad5973 100644 --- a/remote/test/browser/page/browser_runtimeEvents.js +++ b/remote/test/browser/page/browser_runtimeEvents.js @@ -62,7 +62,7 @@ add_task(async function testCDP({ client }) { const onExecutionContextCreated2 = Runtime.executionContextCreated(); const url = toDataURL("test-page"); const { frameId } = await Page.navigate({ url }); - info("A new page has been loaded"); + info("A new page has been requested"); ok(frameId, "Page.navigate returned a frameId"); is( frameId, diff --git a/remote/test/browser/page/browser_scriptToEvaluateOnNewDocument.js b/remote/test/browser/page/browser_scriptToEvaluateOnNewDocument.js index 3c6dfcf2ab05..df1e2c0afe4f 100644 --- a/remote/test/browser/page/browser_scriptToEvaluateOnNewDocument.js +++ b/remote/test/browser/page/browser_scriptToEvaluateOnNewDocument.js @@ -46,10 +46,13 @@ add_task(async function addScriptAfterNavigation({ client }) { add_task(async function addWithIsolatedWorldAndNavigate({ client }) { const { Page, Runtime } = client; + Page.enable(); + const loadEventFired = Page.loadEventFired(); const contextsCreated = recordContextCreated(Runtime, 3); await Runtime.enable(); info(`Navigating to ${DOC}`); const { frameId } = await Page.navigate({ url: DOC }); + await loadEventFired; await Page.addScriptToEvaluateOnNewDocument({ source: "1 + 1;", worldName: WORLD, diff --git a/remote/test/browser/runtime/browser_executionContext.js b/remote/test/browser/runtime/browser_executionContext.js index e15715faca54..37bef041ef04 100644 --- a/remote/test/browser/runtime/browser_executionContext.js +++ b/remote/test/browser/runtime/browser_executionContext.js @@ -72,7 +72,7 @@ async function testNavigate({ Runtime, Page }, previousContext) { const history = recordContextEvents(Runtime, 3); const { frameId } = await Page.navigate({ url: toDataURL("test-page") }); - info("A new page has been loaded"); + info("A new page has been requested"); is( frameId, previousContext.auxData.frameId,