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

@@ -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;
   }
 
   /**

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),
     });
   }
 

remote/test/browser/browser_cdp.js

@@ -46,7 +46,7 @@ add_task(async function testCDP({ client }) {
   await Page.navigate({
     url: toDataURL(`<script>console.log("foo")</script>`),
   });
-  info("A new page has been loaded");
+  info("A new page has been requested");
 
   await loadEventFired;
   info("`Page.loadEventFired` fired");

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({

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]

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,

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(

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");
+}

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,

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,

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,