Skip to content

Commit

Permalink
browser(firefox): fix request frame attribution (#3657)
Browse files Browse the repository at this point in the history
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?
  • Loading branch information
JoelEinbinder authored Aug 29, 2020
1 parent 6a93cb9 commit abb50a7
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 10 deletions.
4 changes: 2 additions & 2 deletions browser_patches/firefox/BUILD_NUMBER
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
1167
Changed: [email protected] Tue Aug 25 14:46:17 PDT 2020
1168
Changed: [email protected] Thu 27 Aug 2020 03:42:20 AM PDT
2 changes: 1 addition & 1 deletion browser_patches/firefox/juggler/NetworkObserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ class NetworkRequest {
navigationId: this.navigationId,
cause: causeTypeToString(causeType),
internalCause: causeTypeToString(internalCauseType),
}, this.httpChannel.channelId);
}, this.httpChannel.channelId + ':' + internalCauseType);
}

_sendOnResponse(fromCache) {
Expand Down
11 changes: 8 additions & 3 deletions browser_patches/firefox/juggler/content/NetworkMonitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,21 @@ 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) {
// Accessing loadContext.associatedWindow sometimes throws.
}
}

requestDetails(channelId) {
return this._requestDetails.get(channelId) || null;
requestDetails(channelKey) {
return this._requestDetails.get(channelKey) || null;
}

dispose() {
Expand Down
4 changes: 2 additions & 2 deletions browser_patches/firefox/juggler/content/PageAgent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}) {
Expand Down
4 changes: 2 additions & 2 deletions browser_patches/firefox/juggler/protocol/NetworkHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit abb50a7

Please sign in to comment.