558801 - e10s HTTP: support XMLHttpRequests under e10s

Status: RESOLVED FIXED

(Core Graveyard :: Tracking, defect)

Description

[Jason Duell]

Right now XHR requests fail in the testXUL demo because XHR calls nsIHttpChannel::SetOwner(). That's covered by bug 547051.  There may be other work needed.

Anyone want to own this?

Comment 1

[Jae-Seong Lee-Russo]

Attachment: mOwner patch, 0

Reading the comment6 of bug 547051, this seems sufficient.

Comment 2

[Jason Duell]

Comment on attachment 440136 [details] [diff] [review]
mOwner patch, 0

>diff --git a/netwerk/protocol/http/src/HttpBaseChannel.cpp b/netwerk/protocol/http/src/HttpBaseChannel.cpp
>--- a/netwerk/protocol/http/src/HttpBaseChannel.cpp
>+++ b/netwerk/protocol/http/src/HttpBaseChannel.cpp
>@@ -237,16 +237,32 @@ HttpBaseChannel::GetURI(nsIURI **aURI)
> {
>   NS_ENSURE_ARG_POINTER(aURI);
>   *aURI = mURI;
>   NS_ADDREF(*aURI);
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>+HttpBaseChannel::GetOwner(nsISupports **aOwner)
>+{
>+    NS_ENSURE_ARG_POINTER(aOwner);
>+    *aOwner = mOwner;
>+    NS_IF_ADDREF(*aOwner);
>+    return NS_OK;
>+}
>+
>+NS_IMETHODIMP
>+HttpBaseChannel::SetOwner(nsISupports *aOwner)
>+{
>+    mOwner = aOwner;
>+    return NS_OK;
>+}
>+

Please use 2-space indents in the future if that's what the rest of the file uses!  I'll fix before I check this in.  


>diff --git a/netwerk/test/unit/test_bug558801.js b/netwerk/test/unit/test_bug558801.js

Nice test!  I renamed it "test_xmlhttprequest.js", just to be more informative.

BTW--this patch would have been better submitted to bug 547051--that's the bug that's going to be fixed by this.  As it turns out we may have other issues with XHR once we're in the browser (see bug 559200).

Comment 3

[Jason Duell]

At this point XHR is working in xpcshell, but we suspect bug 559200 may mess it up in fennec.

Comment 4

[Jae-Seong Lee-Russo]

test_xmlhttprequest_wrap.js aborts (test_xmlhttprequest.js works fine).  I think Bug 557605 is related to the assertion failure.

###!!! ASSERTION: wrong context on XPCJSContextStack!: 'cx == topJSContext', file c:/mozilla-build/e10s/js/src/xpconnect/src/xpcquickstubs.cpp, line 1144

(http://mxr.mozilla.org/projects-central/source/electrolysis/js/src/xpconnect/src/xpcquickstubs.cpp#1144)

nsIXMLHttpRequest_SetOnreadystatechange in dom_quickstubs.cpp called xpc_qsAssertContextOK.

Comment 5

[Jason Duell]

Let's keep this bug as a tracking bug only.  I've opened bug 564351 for the XHR stubs issue.   Thanks for noticing, Jae-Seong.

Comment 6

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.

Comment 7

[Jason Duell]

All dependencies fixed.  AFAIK XHR works fine under e10s now.  Reopen if evidence to the contrary emerges.

Yay.