Closed
Bug 1081879
Opened 10 years ago
Closed 9 years ago
[e10s] Can not get nsIHttpChannel in nsIWebProgressListener.onStateChange
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
People
(Reporter: pzhang, Assigned: billm)
References
Details
Attachments
(1 file)
19.23 KB,
patch
|
mconley
:
review+
|
Details | Diff | Splinter Review |
We can't get nsIHttpChannel in the onStateChange callback of a web progress listener, considering the following codes: var myListener = { QueryInterface: XPCOMUtils.generateQI(["nsIWebProgressListener", "nsISupportsWeakReference"]), onStateChange: function(aWebProgress, aRequest, aFlag, aStatus) { // |aRequest instanceof Ci.nsIHttpChannel| is evaluated as true in *non-e10s* Nightly // but false in *e10s* Nightly. if (aRequest instanceof Ci.nsIHttpChannel) { alert('Got httpChannel in onStateChange: ' + aRequest.URI.spec); } }, onLocationChange: function(aProgress, aRequest, aURI) {}, // For definitions of the remaining functions see related documentation onProgressChange: function(aWebProgress, aRequest, curSelf, maxSelf, curTot, maxTot) {}, onStatusChange: function(aWebProgress, aRequest, aStatus, aMessage) {}, onSecurityChange: function(aWebProgress, aRequest, aState) {} } gBrowser.addProgressListener(myListener); on *non-e10s* Nightly, |aRequest instanceof Ci.nsIHttpChannel| is evaluates as |true|, but on *non-e10s* Nightly, it's evaluated as |false|.
Comment 1•10 years ago
|
||
Which process does the web progress listener live in, chrome or content?
Updated•10 years ago
|
Flags: needinfo?(pzhang)
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #1) > Which process does the web progress listener live in, chrome or content? Hi Josh, it's in the chrome process. When e10s is enabled, |aRequest| is an instance of |Ci.nsIChannel|, and i tried to |QueryInterface| Ci.nsIHttpChannel on |aRequest|, it failed.
Flags: needinfo?(pzhang) → needinfo?(josh)
Updated•10 years ago
|
tracking-e10s:
--- → m6+
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → wmccloskey
Comment 3•9 years ago
|
||
Sorry for letting this sit; Bill appears to be on the case, now.
Flags: needinfo?(josh)
Assignee | ||
Comment 4•9 years ago
|
||
This patch adds shims around gBrowser.addProgressListener and addTabsProgressListener. Add-on listeners will be passed CPOWs for the nsIWebProgress and nsIRequest arguments. (Right now we just pass in fake objects that are close enough to fool our frontend code.) Additionally, listeners are run synchronously. That is, the content process blocks waiting for them to finish. That way add-ons have a chance to examine the state of the content process using CPOWs.
Attachment #8589312 -
Flags: review?(mconley)
Comment 6•9 years ago
|
||
Comment on attachment 8589312 [details] [diff] [review] patch Review of attachment 8589312 [details] [diff] [review]: ----------------------------------------------------------------- So this looks OK, and should, as far as I can tell, give old shimmed add-ons what they need. I think it's particularly clever how you expose the nsIRequest CPOWs _just_ for add-ons! ::: toolkit/components/addoncompat/RemoteAddonsChild.jsm @@ +547,5 @@ > } > } > }, > + > + useSyncWebProgress() { Nit - might as well use a getter here, since you don't take arguments. ::: toolkit/components/addoncompat/tests/addon/bootstrap.js @@ +552,5 @@ > then(testObserver). > then(testSandbox). > then(testAddonContent). > + then(testAboutModuleRegistration). > + then(testProgressListener); Just a thought - if we wanted to not taint the blame of the last entry every time we add a new one, we could take this opportunity to put this at the end: .then(testProgressListener) .then(Promise.resolve()); If you want - just something I thought of.
Attachment #8589312 -
Flags: review?(mconley) → review+
Reporter | ||
Comment 7•9 years ago
|
||
I tried this patch, the interface `Ci.nsIHttpChannel` could be queried on the parameter `aRequest` now, thanks. But error occured when visiting headers on the nsIHttpChannel object: [object CPOW [Exception... "It's illegal to pass a CPOW to native code arg 0 [nsIHttpChannel.visitRequestHeaders]" nsresult: "0x80570036 (NS_ERROR_XPC_CANT_PASS_CPOW_TO_NATIVE)" location: "JS frame :: chrome://global/content/browser-child.js :: _send :: line 105" data: no]] and here is the code: gBrowser.addTabsProgressListener({ onStateChange: function(aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) { if (!(aStateFlags & (Ci.nsIWebProgressListener.STATE_TRANSFERRING | Ci.nsIWebProgressListener.STATE_IS_DOCUMENT))) { return; } let httpChannel = aRequest.QueryInterface(Ci.nsIHttpChannel); httpChannel.visitRequestHeaders({ visitHeader: function(aHeader, aValue) { console.log(aHeader + ': ' + aValue); } }); } });
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e158ed184326
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment 10•9 years ago
|
||
A quick heads up that the tabbrowser addProgressListener() still produces a request that cannot be QueryInterface'd to a channel (with e10s enabled and the addon declared multiprocess-compatible). I hesitate to even mention it because I don't think we should provide extra shims or synchronous messaging in this case. The tabbrowser progress listener behaviour cannot easily be replaced by a frame script and I think it should remain available even to multiprocess-compatible code without any baggage. Possibly the request parameter should even be made null in that case since it isn't reliable. Note that the web progress DOMWindow property is still shimmed even for addons marked compatible, I think intentionally, but this object may become invalid when accessed asynchronously from the tabbrowser listener. Bug 1118880 describes this situation. Again, I don't think it should be changed, but either documented or possible set null.
You need to log in
before you can comment on or make changes to this bug.
Description
•