chrome.webRequest.onCompleted never fires

RESOLVED FIXED in Firefox 44

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: mao, Assigned: billm)

Tracking

(Blocks 1 bug)

unspecified
mozilla44
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

(Whiteboard: [webRequest])

Attachments

(4 attachments)

Posted file oncompleted-0.1.xpi
See the attached sample XPI.
Furthermore, we should either implement onErrorOccurred or at least call onCompleted also when the request is aborted by an error (you can use a global nsIWebProgressListener to track that).
Whiteboard: [webRequest]
OK, I've figured out what's going on.

At the time when onStopRequest is called on our channel listener, causing "onStop" and eventually "onCompleted" to be fired, HttpObserverManager.getLoadContext(channel) returns null, and therefore the "browser" variable is set to null:

http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/addons/WebRequest.jsm#250

This causes the wrapper of the "onCompleted" event callback to return prematurely because the tab id could not be retrieved:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/ext-webRequest.js#24

We need either to retrieve this info in some other way or to omit it from onCompleted's details.

Any ETA for a fix? 
I've got a demo in 2 days (which does not depend on the tab id being available in onCompleted, BTW) and this would really help.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(amckay)
On a side note, outerWindowId is available at that time, so in the worst case (when browser is unavailable) we could always return tabId as a lazy property to be initialized on demand by iterating across the available browsers until we match the outer window id (maybe starting with the focused one as a tentative optimization).
Posted patch patchSplinter Review
The problem here, as Giorgio pointed out, is that the load context is null when we get the onStopRequest notification in e10s (in non-e10s it's still valid). That's presumably some leak prevention thing we do (none of the networking code is cycle collected).

This patch just keeps the load context alive by saving it in the StartStopListener. I was a bit concerned about leaks here, but the networking code seems to null out the listener for us later.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(amckay)
Attachment #8672844 - Flags: review?(dtownsend)
Oh, this patch also fixes an unrelated bug I noticed where runChannelListener doesn't return true at the end (signalling that the load wasn't blocked).
This patch adds a test for the issue where we weren't returning true. The bug manifests in us not calling runChannelListener("afterModify") in some cases, which leads to onSendHeaders not firing.
Attachment #8672845 - Flags: review?(dtownsend)
Here's a test for the onCompleted issue. I had to refactor the test a bit.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8672846 - Flags: review?(dtownsend)
Attachment #8672844 - Flags: review?(dtownsend) → review+
Attachment #8672845 - Flags: review?(dtownsend) → review+
Attachment #8672846 - Flags: review?(dtownsend) → review+
Last two commits were actually for bug 1209983.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.