Bug 1740116 - [devtools] Fix fetching extra data about cancelled navigation request. r=nchevobbe

These exception may happen when some pending request from the previous page
are received lated by the netmonitor, which tries to fetch extra data about these
old requests.
On the server side, these requests are destroyed when devtools.netmonitor.persistlog=false.
(which is the default behavior)

Differential Revision: https://phabricator.services.mozilla.com/D132495

devtools/client/netmonitor/src/har/test/browser_net_har_copy_all_as_har.js

@@ -6,6 +6,10 @@
 /**
  * Basic tests for exporting Network panel content into HAR format.
  */
+
+const EXPECTED_REQUEST_HEADER_COUNT = 9;
+const EXPECTED_RESPONSE_HEADER_COUNT = 6;
+
 add_task(async function() {
   // Disable tcp fast open, because it is setting a response header indicator
   // (bug 1352274). TCP Fast Open is not present on all platforms therefore the
@@ -17,7 +21,7 @@ add_task(async function() {
 
   info("Starting test... ");
 
-  let har = await reloadAndCopyAllAsHar(tab, monitor, toolbox);
+  let har = await reloadAndCopyAllAsHar({ tab, monitor, toolbox });
 
   // Check out HAR log
   isnot(har.log, null, "The HAR log must exist");
@@ -33,30 +37,19 @@ add_task(async function() {
   ok("onLoad" in page.pageTimings, "There must be onLoad time");
 
   let entry = har.log.entries[0];
-  ok(entry.time > 0, "Check the total time");
-  is(entry.request.method, "GET", "Check the method");
-  is(entry.request.url, SIMPLE_URL, "Check the URL");
-  is(entry.request.headers.length, 9, "Check number of request headers");
-  is(entry.response.status, 200, "Check response status");
-  is(entry.response.statusText, "OK", "Check response status text");
-  is(entry.response.headers.length, 6, "Check number of response headers");
-  is(
-    entry.response.content.mimeType,
-    "text/html",
-    "Check response content type"
-  );
+  assertNavigationRequestEntry(entry);
   isnot(entry.response.content.text, undefined, "Check response body");
   isnot(entry.timings, undefined, "Check timings");
 
-  // Test response body limit (non zero).
+  info("Test response body limit (non zero).");
   await pushPref("devtools.netmonitor.responseBodyLimit", 10);
-  har = await reloadAndCopyAllAsHar(tab, monitor, toolbox);
+  har = await reloadAndCopyAllAsHar({ tab, monitor, toolbox });
   entry = har.log.entries[0];
   is(entry.response.content.text.length, 10, "Response body must be truncated");
 
-  // Test response body limit (zero).
+  info("Test response body limit (zero).");
   await pushPref("devtools.netmonitor.responseBodyLimit", 0);
-  har = await reloadAndCopyAllAsHar(tab, monitor, toolbox);
+  har = await reloadAndCopyAllAsHar({ tab, monitor, toolbox });
   entry = har.log.entries[0];
   is(
     entry.response.content.text.length,
@@ -64,13 +57,66 @@ add_task(async function() {
     "Response body must not be truncated"
   );
 
+  har = await reloadAndCopyAllAsHar({
+    tab,
+    monitor,
+    toolbox,
+    reloadTwice: true,
+  });
+  is(har.log.entries.length, 2, "There must be two requests");
+  info(
+    "Assert the first navigation request which has been cancelled by the second reload"
+  );
+  // Requests may come out of order, so try to find the bogus cancelled request
+  entry = har.log.entries.find(e => e.response.status == 0);
+  ok(entry, "Found the cancelled request");
+  is(entry.request.method, "GET", "Method is set");
+  is(entry.request.url, SIMPLE_URL, "URL is set");
+  // We always get the following headers:
+  // "Host", "User-agent", "Accept", "Accept-Language", "Accept-Encoding", "Connection"
+  // but are missing the three last headers:
+  // "Upgrade-Insecure-Requests", "Pragma", "Cache-Control"
+  is(entry.request.headers.length, 6, "But headers are partialy populated");
+  is(entry.response.status, 0, "And status is set to 0");
+
+  entry = har.log.entries.find(e => e.response.status != 0);
+  assertNavigationRequestEntry(entry);
+
   return teardown(monitor);
 });
 
+function assertNavigationRequestEntry(entry) {
+  info("Assert that the entry relates to the navigation request");
+  ok(entry.time > 0, "Check the total time");
+  is(entry.request.method, "GET", "Check the method");
+  is(entry.request.url, SIMPLE_URL, "Check the URL");
+  is(
+    entry.request.headers.length,
+    EXPECTED_REQUEST_HEADER_COUNT,
+    "Check number of request headers"
+  );
+  is(entry.response.status, 200, "Check response status");
+  is(entry.response.statusText, "OK", "Check response status text");
+  is(
+    entry.response.headers.length,
+    EXPECTED_RESPONSE_HEADER_COUNT,
+    "Check number of response headers"
+  );
+  is(
+    entry.response.content.mimeType,
+    "text/html",
+    "Check response content type"
+  );
+}
 /**
  * Reload the page and copy all as HAR.
  */
-async function reloadAndCopyAllAsHar(tab, monitor, toolbox) {
+async function reloadAndCopyAllAsHar({
+  tab,
+  monitor,
+  toolbox,
+  reloadTwice = false,
+}) {
   const { connector, store, windowRequire } = monitor.panelWin;
   const Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
   const { HarMenuUtils } = windowRequire(
@@ -87,6 +133,9 @@ async function reloadAndCopyAllAsHar(tab, monitor, toolbox) {
     onDomCompleteResource,
   } = await waitForNextTopLevelDomCompleteResource(toolbox.commands);
 
+  if (reloadTwice) {
+    reloadBrowser();
+  }
   await reloadBrowser();
 
   info("Waiting for network events");

devtools/server/actors/resources/network-events.js

@@ -187,7 +187,13 @@ class NetworkEventWatcher {
         // So do this when navigating a second time, we will navigate from a distinct WindowGlobal
         // and check that this is the top level window global and not an iframe one.
         // So that we avoid clearing the top navigation when an iframe navigates
+        //
+        // Avoid destroying the request if innerWindowId isn't set. This happens when we reload many times in a row.
+        // The previous navigation request will be cancelled and because of that its innerWindowId will be null.
+        // But the frontend will receive it after the navigation begins (after will-navigate) and will display it
+        // and try to fetch extra data about it. So, avoid destroying its NetworkEventActor.
       } else if (
+        child.innerWindowId &&
         child.innerWindowId != innerWindowId &&
         windowGlobal.browsingContext ==
           this.watcherActor.browserElement?.browsingContext