observerService.addObserver (http-on-modify-request) fails in e10s child process

RESOLVED FIXED

Status

Developer Documentation
Add-ons
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Anthony Lieuallen, Assigned: billm)

Tracking

unspecified

Firefox Tracking Flags

(e10s+, firefox41 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150306140254

Steps to reproduce:

  var observerService = Components.classes["@mozilla.org/observer-service;1"]
      .getService(Components.interfaces.nsIObserverService);
  observerService.addObserver(requestObserver, "http-on-modify-request", false);

In a content frame script, in Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0, e10s enabled.


Actual results:

[Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIObserverService.addObserver]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: resource://....js :: ....setupReferer :: line 161"  data: no]



Expected results:

The observer should have been attached.  This works as expected with e10s disabled.
Dupe of bug 1108827?
(Reporter)

Comment 3

3 years ago
Comment there says it should be working.
addObserver(http-on-*) fails in the child process because we don't have complete information about cookies there. We could probably provide some sort of notification in the child where you couldn't read or change cookies, but that might be more confusing that not having the notification.

What are you going in the observer?
(Reporter)

Comment 5

3 years ago
Our use case is just setting the Referer header on the request.  Apparently that's the only way it can be done?

Specifically: https://github.com/greasemonkey/greasemonkey/blob/16fb34c8f32f13272ca6bd744d3284d5cc4a7744/modules/xmlhttprequester.js#L152

Comment 6

3 years ago
Docs say that most observers aren't expected to work in the child process: https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Limitations_of_frame_scripts#Observers_in_the_content_process

Maybe just send a message to the parent process to modify the next request with that URL?
(Reporter)

Comment 7

3 years ago
Maybe this should turn into an RFE then?  If I don't happen to know about that particular bit of documentation (times N for all the N features that might work only in the parent/only in the child process), the error message should tell me exactly what's wrong and what to do about it.  Not just "NS_ERROR_NOT_IMPLEMENTED".
https://developer.mozilla.org/en-US/Add-ons/Working_with_multiprocess_Firefox has a link to the "Limitations of chrome scripts" page, but doesn't link to the "Limitations of frame scripts". It should definitely get included there somewhere and maybe the two articles should reference one another. Best case, this N*N would then become a 2.

IHMO, this could already be enough, although an NS_ERROR_NOT_SUPPORTED_IN_CONTENT/CHROME/CHILD sort of thing also looks interesting.
Status: UNCONFIRMED → NEW
Component: Untriaged → Add-ons
Ever confirmed: true
OS: Linux → All
Product: Firefox → Developer Documentation
Hardware: x86_64 → All
Summary: observerService.addObserver (http-on-modify-request) fails in e10s → observerService.addObserver (http-on-modify-request) fails in e10s child process
Version: 36 Branch → unspecified

Updated

3 years ago
Flags: needinfo?(mrbkap)
I've made the updates suggested in comment 8. Let me know if this works for you and I'll close the bug.
Flags: needinfo?(johnp)
LGTM. OTOH I just re-read the bugs and :billm or :mrbkap should have the last word, whether or not there's something else to do to improve the situation code-wise, as they seem to be quite familiar with that code.
Fwiw, I just stumbled upon the bug where the http-on-* observers were apparently removed from the child: Bug 806753.

If not, maybe adding a note that functions that are unsupported in the current context may throw "NS_ERROR_NOT_IMPLEMENTED" could help this turn up in search results more easily... Just a thought though.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(mrbkap)
Flags: needinfo?(johnp)
See Also: → bug 806753
Re-setting needinfo. See comment 11.
(Apparently Bugzilla doesn't support removing a needinfo for a user and adding it again in one comment, but silently discards the re-addition)
Flags: needinfo?(mrbkap)
Created attachment 8623212 [details] [diff] [review]
patch

A warning here seems reasonable. I wasn't able to get the script filename or line number because the code for that seems to be in a different .so that isn't available or something. I don't think it matters that much though. The warning text by itself should still be helpful.

Also, the additional #include is intentional. It looks like all previous uses of nsIScriptError.h already include nsString.h.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Flags: needinfo?(wmccloskey)
Flags: needinfo?(mrbkap)
Attachment #8623212 - Flags: review?(mrbkap)

Updated

3 years ago
Attachment #8623212 - Flags: review?(mrbkap) → review+
I've added a link to "Limitations of frame scripts" to https://developer.mozilla.org/en-US/Add-ons/Working_with_multiprocess_Firefox.
tracking-e10s: --- → +
https://hg.mozilla.org/mozilla-central/rev/fe6e29cd1894
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED

Updated

3 years ago
Blocks: 1176749

Updated

3 years ago
No longer blocks: 1176749
Depends on: 1176749
You need to log in before you can comment on or make changes to this bug.