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

Status: RESOLVED FIXED

(Developer Documentation Graveyard :: Add-ons, defect)

Description

[Anthony Lieuallen]

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.

Comment 1

[Anthony Lieuallen]

Reported to us at https://github.com/greasemonkey/greasemonkey/issues/2103 .

Comment 2

[Johannes Pfrang [:johnp]]

Dupe of bug 1108827?

Comment 3

[Anthony Lieuallen]

Comment there says it should be working.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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?

Comment 5

[Anthony Lieuallen]

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

[The 8472]

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?

Comment 7

[Anthony Lieuallen]

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".

Comment 8

[Johannes Pfrang [:johnp]]

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.

Comment 9

[The 8472]

https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox has both.

Comment 10

[Will Bamberg [:wbamberg]]

I've made the updates suggested in comment 8. Let me know if this works for you and I'll close the bug.

Comment 11

[Johannes Pfrang [: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.

Comment 12

[Johannes Pfrang [:johnp]]

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)

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: 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.

Comment 14

[Eric Shepherd [:sheppy]]

I've added a link to "Limitations of frame scripts" to https://developer.mozilla.org/en-US/Add-ons/Working_with_multiprocess_Firefox.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe6e29cd1894

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fe6e29cd1894