839490 - NS_ERROR_NOT_IMPLEMENTED errors in nsIObserverService.addObserver in the B2G mochitest run

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Martijn Wargers (dead)]

There are a bunch of test failures in B2G mochitest run (when they are not in the exclude list), like in content/base/test/test_CSP.html.
This triggers "NS_ERROR_NOT_IMPLEMENTED: Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIObserverService.addObserver] at http://mochi.test:8888/tests/content/base/test/test_CSP.html:49".

Jlebar explained this to me:
Basically these kinds of errors come from this part of the code:
http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsObserverService.cpp#106
106     if (mozilla::net::IsNeckoChild() && !strncmp(aTopic, "http-on-", 8)) {
107       return NS_ERROR_NOT_IMPLEMENTED;
108     }

This isn't implemented for the child content process yet, and it might very well never will, because it's not clear how this should behave well in the child process.

So that means, these tests need to remain in the exclude list.

Comment 1

[Jason Duell]

My understanding is that for now anything that uses http-on-XYZ observers (like CSPs) is supposed to live on the parent only.  So yes, disable these tests in e10s.

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Jason is right that these things need to live in the parent process.

BUT, I disagree about disabling the tests. CSP is a critical part of B2G security so it should be tested if at all possible.

It seems like the problem is that the test is written wrongly.

The part that adds the http-on-* observers needs to run in the parent process. That is likely to be a very common pattern. Is there an example of a mochitest that has code that needs to run in both process that we could use as a model?

Otherwise, we can see how the http-on-modify-request notification is used in the test:

    if (topic === "http-on-modify-request") {
      //these things were allowed by CSP
      var uri = subject.QueryInterface(Components.interfaces.nsIHttpChannel).URI;
      if (!testpat.test(uri.asciiSpec)) return;
      var testid = testpat.exec(uri.asciiSpec)[1];

      window.testResult(testid,
                        /_good/.test(testid),
                        uri.asciiSpec + " allowed by csp");
    }

So, an alternate way of fixing these tests would be to modify it so that we verify that the load succeeded in a different way.

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Brian: I believe this test was created before b2g, so I am not sure it should be deemed "written *wrongly*". We should update it to run in the parent, but given the mochitest harness as it is now, it's not very clear when that should happen and when it should not.  We should call it out somewhere pretty darn clearly if some observer topics are off-limits in certain situations.

Comment 4

[Martijn Wargers (dead)]

Ok, bug 94528 is fixed, but I see there are some instances that still need to have the same treatment as that bug.
I guess we can use this bug to convert those instances:
http://mxr.mozilla.org/mozilla-central/search?string=%22http-on-modify-request
content/base/test/test_bug650386_redirect_301.html
content/base/test/test_bug650386_redirect_302.html
content/base/test/test_bug650386_redirect_303.html
content/base/test/test_bug650386_redirect_307.html
content/base/test/test_bug717511.html
content/xml/document/test/test_bug392338.html
extensions/cookie/test/file_testloadflags.js