From abb50a79bdce1b862e4e540ec8575d6d26a29df6 Mon Sep 17 00:00:00 2001
From: Joel Einbinder <joel.einbinder@gmail.com>
Date: Fri, 28 Aug 2020 17:55:05 -0700
Subject: [PATCH] browser(firefox): fix request frame attribution (#3657)

Firefox will sometimes send multiple requests with the same http channel id. When a frame is loaded, the favicon is requested in the parent frame, but with the same channel id. This can cause the document request to report the wrong frame, causing the test 'should capture iframe navigation request' to fail. It fails consistently on my computer.

This patch adds the content policy type into the http channelId to better distinguish requests. Maybe there is something better we can do? It looks like we use channelId has request ids, so there might be more bugs with these favicon requests in playwright?
---
 browser_patches/firefox/BUILD_NUMBER                  |  4 ++--
 browser_patches/firefox/juggler/NetworkObserver.js    |  2 +-
 .../firefox/juggler/content/NetworkMonitor.js         | 11 ++++++++---
 browser_patches/firefox/juggler/content/PageAgent.js  |  4 ++--
 .../firefox/juggler/protocol/NetworkHandler.js        |  4 ++--
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER
index 48294aca41..5b3ada0456 100644
--- a/browser_patches/firefox/BUILD_NUMBER
+++ b/browser_patches/firefox/BUILD_NUMBER
@@ -1,2 +1,2 @@
-1167
-Changed: yurys@chromium.org Tue Aug 25 14:46:17 PDT 2020
+1168
+Changed: joel.einbinder@gmail.com Thu 27 Aug 2020 03:42:20 AM PDT
diff --git a/browser_patches/firefox/juggler/NetworkObserver.js b/browser_patches/firefox/juggler/NetworkObserver.js
index 551fcae75b..03cab787ab 100644
--- a/browser_patches/firefox/juggler/NetworkObserver.js
+++ b/browser_patches/firefox/juggler/NetworkObserver.js
@@ -492,7 +492,7 @@ class NetworkRequest {
       navigationId: this.navigationId,
       cause: causeTypeToString(causeType),
       internalCause: causeTypeToString(internalCauseType),
-    }, this.httpChannel.channelId);
+    }, this.httpChannel.channelId + ':' + internalCauseType);
   }
 
   _sendOnResponse(fromCache) {
diff --git a/browser_patches/firefox/juggler/content/NetworkMonitor.js b/browser_patches/firefox/juggler/content/NetworkMonitor.js
index fdb5e2899c..6cec455d59 100644
--- a/browser_patches/firefox/juggler/content/NetworkMonitor.js
+++ b/browser_patches/firefox/juggler/content/NetworkMonitor.js
@@ -33,7 +33,12 @@ class NetworkMonitor {
       const frame = this._frameTree.frameForDocShell(window.docShell);
       if (!frame)
         return;
-      this._requestDetails.set(httpChannel.channelId, {
+      const typeId = httpChannel.loadInfo ? httpChannel.loadInfo.internalContentPolicyType : Ci.nsIContentPolicy.TYPE_OTHER;
+      // Channel ids are not unique. We combine them with the typeId
+      // to better distinguish requests. For example, favicon requests
+      // have the same channel id as their associated document request.
+      const channelKey = httpChannel.channelId + ':' + typeId;
+      this._requestDetails.set(channelKey, {
         frameId: frame.id(),
       });
     } catch (e) {
@@ -41,8 +46,8 @@ class NetworkMonitor {
     }
   }
 
-  requestDetails(channelId) {
-    return this._requestDetails.get(channelId) || null;
+  requestDetails(channelKey) {
+    return this._requestDetails.get(channelKey) || null;
   }
 
   dispose() {
diff --git a/browser_patches/firefox/juggler/content/PageAgent.js b/browser_patches/firefox/juggler/content/PageAgent.js
index 5d12f4c24c..124df808d4 100644
--- a/browser_patches/firefox/juggler/content/PageAgent.js
+++ b/browser_patches/firefox/juggler/content/PageAgent.js
@@ -170,8 +170,8 @@ class PageAgent {
     this._dataTransfer = null;
   }
 
-  _requestDetails({channelId}) {
-    return this._networkMonitor.requestDetails(channelId);
+  _requestDetails({channelKey}) {
+    return this._networkMonitor.requestDetails(channelKey);
   }
 
   async _setEmulatedMedia({type, colorScheme}) {
diff --git a/browser_patches/firefox/juggler/protocol/NetworkHandler.js b/browser_patches/firefox/juggler/protocol/NetworkHandler.js
index 85e69d6b70..9df5817f3c 100644
--- a/browser_patches/firefox/juggler/protocol/NetworkHandler.js
+++ b/browser_patches/firefox/juggler/protocol/NetworkHandler.js
@@ -114,13 +114,13 @@ class NetworkHandler {
       this._httpActivity.delete(activity._id);
   }
 
-  async _onRequest(eventDetails, channelId) {
+  async _onRequest(eventDetails, channelKey) {
     let pendingRequestCallback;
     let pendingRequestPromise = new Promise(x => pendingRequestCallback = x);
     this._pendingRequstWillBeSentEvents.add(pendingRequestPromise);
     let details = null;
     try {
-      details = await this._contentPage.send('requestDetails', {channelId});
+      details = await this._contentPage.send('requestDetails', {channelKey});
     } catch (e) {
       pendingRequestCallback();
       this._pendingRequstWillBeSentEvents.delete(pendingRequestPromise);