537787 - e10s: Remote WebSockets

Status: VERIFIED FIXED

(Core :: Networking: WebSockets, defect)

Description

[Jason Duell]

This is not a must-have (or even a strong want-have) for the next fennec release, but at some point we'll want to make web sockets work under e10s.  We'll want a separate IPDL protocol (PWebSocket) for it.

Hopefully it won't be much work, as the trickier stuff (like authentication) should be similar enough to whatever we cook up for HTTP, and web socket messages should map pretty straightforwardly onto IPDL msgs or shmem.

Comment 3

[Paul Rouget [:paul]]

I think it's a must-have, at least for parity with Fx4 Desktop.

Comment 4

[Lars Gunther]

I'm confused. I saw Paul demo using Web Sockets to control a presentation in another computer just a couple of weeks ago. Did e10s disable that feature?

Comment 5

[Paul Rouget [:paul]]

@Lars: Yes:

Error: docShell: Not Remotable
Source File: chrome://global/content/bindings/browser.xml
Firefox can't establish a connection to the server at ws://.../

Comment 6

[Josh Matthews [:jdm]]

I'll see what I can do with this.

Comment 7

[Manuel Serrano]

Sorry for this naive comment. I don't really understand all the details of this
discussion (for instance, I'm not really sure of what e10s is) so if my remark is
stupid, please, just ignore it.

If there is no plan to integrate correctly the websockets for the next stable
Fennec version, could it be possible to disable them by default (network.websocket.enabled set to false)? This will give a change to applications that need server pushes to switch smoothly to another implementation method such multipart XHRs.

Comment 8

[Manuel Serrano]

Just another comment that might interest you. If you are looking for incentive
for adding websockets to Fennec you might find interesting watching this video.
Go to the time position 7:03 and you will see the demo. Of course, everything is implemented with Firefox and Fennec (this is acknowledged at the end of the talk).
Thanks to all of you.

http://www.college-de-france.fr/default/EN/all/cha_inf2009/Cours_et_seminaire_du_20_janvi.jsp

Comment 9

[Jason Duell]

> If there is no plan to integrate correctly the websockets for the next stable
Fennec version, could it be possible to disable them by default
(network.websocket.enabled set to false)?

Yes--we should check this in now, in case there's any fennec release before we ship e10 websockets.

Comment 10

[Matt Brubeck (:mbrubeck)]

(In reply to comment #7)
> If there is no plan to integrate correctly the websockets for the next stable
> Fennec version, could it be possible to disable them by default
> (network.websocket.enabled set to false)?

Even with this pref change, sites still seem to think that Fennec supports web sockets.  The window.WebSocket property still exists.  Is this a bug?

Comment 11

[Josh Matthews [:jdm]]

I'm inclined to think that window.WebSocket existing is not a bug.  For one thing, that's controlled by the DOMClassInfo stuff as far as I can tell, and I don't believe that's easy to hide based on a pref.  For another, we check the pref during websocket initialization and throw a NS_ERROR_DOM_SECURITY_ERROR - I have no idea if this is to spec and people should be catching this and simply aren't.

Comment 12

[Stuart Parmenter]

Just for clarity, this is a blocker for release.

Comment 13

[Andy Green]

> This is not a must-have (or even a strong want-have)

At the moment there is no websockets + canvas solution on Android.  When there is one, it'll extend realtime network-aware UIs that would previously have needed a native applet / app to Android.  There are people who "strongly want to have" it ;-) and I hope it'll be possible to ship with it.

Comment 14

[Josh Matthews [:jdm]]

I'm working on the implementation right now, and the bug is marked as blocking the final release of mobile Firefox.  There is no need for advocacy.

Comment 15

[Josh Matthews [:jdm]]

Attachment: WIP

This patch allows sites like http://websockets.org/echo.html and http://mrdoob.com/projects/multiuserpad to work seemingly correctly.  There are still problems with the implementation (for example, cookies don't work yet), but I think it's the right architecture.

Comment 16

[Josh Matthews [:jdm]]

Attachment: WIP v2

Rebased on top of bug 574897. Cookies should now work, and theoretically GetInterface for an auth prompt on the channel should also work.  I'm stumped about what to do with the nsILoadContext block that bug 574897 added in GetInterface, however.

Comment 17

[Doug Turner (:dougt)]

jdm, should someone be looking at this wip patch?

Comment 18

[Josh Matthews [:jdm]]

Comment on attachment 492835 [details] [diff] [review]
WIP v2

I suppose it wouldn't hurt if jduell or anybody else knowledgeable wanted to look over it.

Comment 19

[Honza Bambas (:mayhemer)]

I could take a look, if anyone is going to do this sooner, please drop a note.

Comment 20

[Josh Matthews [:jdm]]

Attachment: Add cross-process websocket support.

Comment 21

[Josh Matthews [:jdm]]

Now passes all cookie tests in bug 574897 with the help of SpecialPowers.

Comment 22

[Josh Matthews [:jdm]]

Attachment: Add cross-process websocket support.

Comment 23

[Josh Matthews [:jdm]]

Comment on attachment 495639 [details] [diff] [review]
Add cross-process websocket support.

Alright, I think this is good to go.  I've verified that no objects leak, random browsing of websockets-enabled websites works flawlessly, all websockets tests in the tree pass, and I've got a comfortable mental model of how the various proxy and concrete objects on the parent and child interact.  Honza, if you're not the right person for this then please reassign.

Comment 24

[Josh Matthews [:jdm]]

Honza, looks like this can wait until bug 616733 is resolved one way or the other.

Comment 25

[Brad Lassey [:blassey] (use needinfo?)]

As I understand it, firefox is pref'ing this off for 4.0, so this shouldn't block the release anymore. If my understanding is incorrect, please leave a comment. 

Also, to be clear, I don't see any reason for this not to land if the patches are ready.

Comment 26

[Doug Turner (:dougt)]

honza any update on this review?

Comment 27

[Josh Matthews [:jdm]]

Comment on attachment 495639 [details] [diff] [review]
Add cross-process websocket support.

This review isn't worth Honza's time. The websocket design has changed significantly in the new implementation, so I (or someone else) needs to sit down an reevaluate how best to remote them. In fact, I think I'll try to do that during the train rides I'll be taking in the next few days.

Comment 28

[Josh Matthews [:jdm]]

Hooray, it looks like remoting the new websockets patch should be significantly easier.  We'll want a proxy listener object in the parent process that relays to the child, and some kind of proxy protocol handler in the child that relays the initial AsyncOpen to the parent, I think. I haven't sorted out all the details, but I'm hopeful. I'd be happy to put together a patch when I start full time at the end of May.

Comment 29

[Jason Duell]

I think we're going to want to move on this sooner than the end of May, assuming that biesi will get a chance to review the main websockets patches before then, and that we want to land e10s support concurrently (I don't care personally about landing them at the same time, but I seem to remember other people feeling it was a requirement).

jdm:  please unassign yourself if you don't think you can get to this in a few weeks or less, and we'll find someone to do it.

Comment 30

[Josh Matthews [:jdm]]

Turns out that I've spent the majority of the past couple days on trains, so I'll be able to put up a proof-of-concept semi-patch in the near future. We can see where to go from there.

Comment 31

[Josh Matthews [:jdm]]

Attachment: Make websockets work across processes.

This patch is conceptually complete. I've manually put together a diff based off of the most recent proposed websockets patch, so it may or may not apply to any given tree. It also conceptually builds, as I've gone to fairly painstaking trouble of reading over all my code changes. If someone wants to take it any run with it from here, that'd be swell.

Comment 32

[Josh Matthews [:jdm]]

Attachment: Remote websockets.

I got access to a real tree, so this is actually a patch that can be applied. I also moved the IPC classes into new files, because the originals were long and complicated enough already.

Comment 33

[Josh Matthews [:jdm]]

Attachment: Remote websockets.

Now buildable, thanks to a few hours spent in the Paris office.

Comment 34

[Josh Matthews [:jdm]]

Attachment: Pqtch

Comment 35

[Josh Matthews [:jdm]]

The attached patch represents the sum of my work. I'm not sure how much else I'll be able to do before I return on the 29th, so I'm unassigning myself. My work was at the point where I was attempting to test fennec against updated websocket tests to see what was still broken, last I recall.

Comment 36

[Josh Matthews [:jdm]]

Attachment: Remote websockets.

Psyke! Here's a patch that passes all the websocket tests in the tree. The only remaining issues are that I haven't added any new logging, and the mochitest currently leaks like a bucket with no bottom.

Comment 37

[Josh Matthews [:jdm]]

Attachment: Use SpecialPowers in tests

You can never have too many GC methods in SpecialPowers.

Comment 38

[Josh Matthews [:jdm]]

Attachment: Remote websockets.

Comment 39

[Josh Matthews [:jdm]]

Attachment: Update websocket test to use SpecialPowers.

Comment 40

[Josh Matthews [:jdm]]

Comment on attachment 535997 [details] [diff] [review]
Remote websockets.

This patch is the best one to date! Now with fewer leaks, fewer crashes and more logging. All mochitests also pass with flying colours. Jason, do you feel like taking this?

Comment 41

[Jason Duell]

Comment on attachment 535997 [details] [diff] [review]
Remote websockets.

jdm already discussed much of this on IRC, but I've found some new things.   I'll want to have a look at the next patch too.

Test cases: I haven't looked at the mochitest(s), but let's make sure we cover
- bogus URI in websocket constructor
- valid but refused/unavailable URI
- sync XHR in onmessage() (have server close WS: make sure readyState doesn't change to CLOSED during onmessage()).
- test that involves an HTTP redirect (both failed and succeeded redirects would be good)

>diff --git a/netwerk/build/nsNetModule.cpp b/netwerk/build/nsNetModule.cpp
>
>+static BaseWebSocketHandler*
>+WebSocketHandlerConstructor(bool aSecure)
>+{
>+  if (XRE_GetProcessType() != GeckoProcessType_Default) {
>+      return new nsWebSocketHandlerChild(aSecure);
>+  }

Use IsNeckoChild()?  Are you sure you want the check to be !GeckoProcessType_Default, or is ==GeckoProcessType_Content good enough?  We're only supporting tab processes, right?  In which case I'd think IsNeckoChild() should work fine.  

>diff --git a/netwerk/protocol/websocket/PWebSocket.ipdl b/netwerk/protocol/websocket/PWebSocket.ipdl
>new file mode 100644

Your new files seem to be using DOS line-endings (I see lots of ^M in vim).  Convert to unix (dos2unix is one util for doing that).

Also, not all of your new files have emacs/vim modelines.

>+  // Forwarded methods corresponding to methods on nsIWebSocketProtocolHandler

s/nsIWebSocketProtocolHandler/nsIWebSocketListener/

>diff --git a/netwerk/protocol/websocket/nsWebSocketHandler.cpp b/netwerk/protocol/websocket/nsWebSocketHandler.cpp

>+#include "mozilla/net/NeckoChild.h"

See comments on logging below.  Also may need to remove #include of prlog.h below this.

> #if defined(PR_LOGGING)
>-static PRLogModuleInfo *webSocketLog = nsnull;
>+PRLogModuleInfo *webSocketLog = nsnull;
> #endif
> #define LOG(args) PR_LOG(webSocketLog, PR_LOG_DEBUG, args)

Hmm, we're going to want logging on when !DEBUG, which means we need to have FORCE_PR_LOG on AFICT.  Look at how nsHttp.h does it in lines 43-61.  I'm surprised we don't run into an error re-#defining LOG here--it's probably a warning?  Anyway, I think you'll want to centralize this in BaseWebChannel.h.  Make sure to compile an opt build and make sure that logging works in non-debug mode. 

Our logging is a real mess.

>diff --git a/netwerk/protocol/websocket/nsWebSocketHandler.h b/netwerk/protocol/websocket/nsWebSocketHandler.h
> 
>+class BaseWebSocketHandler : public nsIWebSocketProtocol,

As mentioned on IRC, rename "BaseWebSocketChannel" (per bug 664860) and move into its own .h/.cpp files.

>+public:
>+    BaseWebSocketHandler();
>+
>+    NS_DECL_NSIPROTOCOLHANDLER
>+
>+    NS_SCRIPTABLE NS_IMETHOD QueryInterface(const nsIID & uuid, void **result NS_OUTPARAM) = 0;
>+    NS_IMETHOD_(nsrefcnt ) AddRef(void) = 0;
>+    NS_IMETHOD_(nsrefcnt ) Release(void) = 0;

As mentioned on IRC, I don't think you need to declare methods you're inheriting from interfaces unless you're actually defining them, so these can be eliminated.

>+
>+    NS_SCRIPTABLE NS_IMETHOD GetOriginalURI(nsIURI **aOriginalURI);
>+    NS_SCRIPTABLE NS_IMETHOD GetURI(nsIURI **aURI);
>+    NS_SCRIPTABLE NS_IMETHOD GetNotificationCallbacks(nsIInterfaceRequestor **aNotificationCallbacks);
>+    NS_SCRIPTABLE NS_IMETHOD SetNotificationCallbacks(nsIInterfaceRequestor *aNotificationCallbacks);

80 columns max, please.  Maybe break after NS_IMETHOD?  I'm not used to having NS_SCRIPTABLE in the decls, which makes lines (and even the column of the 1st param) very long.  We can probably scrap the NS_SCRIPTABLE decls--we don't use them in necko except a few scattered places, and they're just for dehydra analysis.  

>+    NS_SCRIPTABLE NS_IMETHOD GetSecurityInfo(nsISupports **aSecurityInfo) = 0;
>+
>+    NS_SCRIPTABLE NS_IMETHOD AsyncOpen(nsIURI *aURI,
>+                         const nsACString &aOrigin,
>+                         nsIWebSocketListener *aListener,
>+                         nsISupports *aContext) = 0;
>+    NS_SCRIPTABLE NS_IMETHOD Close() = 0;
>+    NS_SCRIPTABLE NS_IMETHOD SendMsg(const nsACString &aMsg) = 0;
>+    NS_SCRIPTABLE NS_IMETHOD SendBinaryMsg(const nsACString &aMsg) = 0;

I think these pure virtual decls can also be eliminated.

>+protected:
>+    nsCOMPtr<nsIURI>                mOriginalURI;
>+    nsCOMPtr<nsIURI>                mURI;
>+    nsCOMPtr<nsIWebSocketListener>  mListener;
>+    nsCOMPtr<nsISupports>           mContext;
>+    nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
>+    nsCOMPtr<nsILoadGroup>          mLoadGroup;
>+
>+    nsCString                       mProtocol;
>+    nsCString                       mOrigin;
>+
>+    PRBool  mEncrypted;

indent mEncrypted to same level as other data members.

> public:
>-    nsWebSocketSSLHandler() {nsWebSocketHandler::mEncrypted = PR_TRUE;}
>+    nsWebSocketSSLHandler() {BaseWebSocketHandler::mEncrypted = PR_TRUE;}

Put space between { and BaseWebSocketHandler, and btw ; and }

>diff --git a/netwerk/protocol/websocket/nsWebSocketHandlerIPC.cpp b/netwerk/protocol/websocket/nsWebSocketHandlerIPC.cpp

per IRC, split each class into its own file, and name WebSocketChannelChild|Parent.  And we need ChannelEventQueue for all incoming IPDL traffic to child.

>+bool
>+nsWebSocketHandlerParent::RecvDeleteSelf()
>+{
>+  LOG(("nsWebSocketHandlerParent::RecvDeleteSelf() %p\n", this));
>+  mProtocol = nsnull;
>+  mAuthProvider = nsnull;
>+  return Send__delete__(this);
>+}

Send__delete__ should be guarded with mIPCOpen.  The child process could conceivably drop dead at any point.

>+
>+bool
>+nsWebSocketHandlerParent::RecvAsyncOpen(const IPC::URI& aURI,
>+                                        const nsCString& aOrigin,
>+                                        const nsCString& aProtocol,
>+                                        const bool& aSecure)
>+{
>+  LOG(("nsWebSocketHandlerParent::RecvAsyncOpen() %p\n", this));
>+  nsresult rv;
>+  if (aSecure) {
>+    mProtocol =
>+        do_CreateInstance("@mozilla.org/network/protocol;1?name=wss", &rv);
>+  } else {
>+    mProtocol =
>+        do_CreateInstance("@mozilla.org/network/protocol;1?name=ws", &rv);

indent do_CreateInstance by 2, not 4

>+bool
>+nsWebSocketHandlerParent::CancelEarly()
>+{
>+  LOG(("nsWebSocketHandlerParent::CancelEarly() %p\n", this));
>+  return SendAsyncOpenFailed();
>+}

Also guard this with mIPCOpen.

>diff --git a/netwerk/protocol/websocket/nsWebSocketHandlerIPC.h b/netwerk/protocol/websocket/nsWebSocketHandlerIPC.h
>+
>+class nsWebSocketHandlerParent : public PWebSocketParent,
>+                                 public nsIWebSocketListener,
>+                                 public nsIInterfaceRequestor
>+{
>+private:
>+  bool RecvAsyncOpen(const IPC::URI& aURI,
>+                     const nsCString& aOrigin,
>+                     const nsCString& aProtocol,
>+                     const bool& aSecure);
>+  bool RecvClose();
>+  bool RecvSendMsg(const nsCString& aMsg);
>+  bool RecvSendBinaryMsg(const nsCString& aMsg);
>+  bool RecvDeleteSelf();
>+  bool CancelEarly();
>+
>+  void ActorDestroy(ActorDestroyReason why);
>+
>+  nsCOMPtr<nsIAuthPromptProvider> mAuthProvider;
>+  nsCOMPtr<nsIWebSocketProtocol> mProtocol;

In anticipation of bug 664860 (and to avoid confusion with the Base class' String mProtocol field), let's rename mProtocol -> mChannel.

Comment 42

[Josh Matthews [:jdm]]

(In reply to comment #41)

>>+  // Forwarded methods corresponding to methods on nsIWebSocketProtocolHandler
>
>s/nsIWebSocketProtocolHandler/nsIWebSocketListener/

Nope, these are from the ProtocolHandler (which is actually the channel).

> >diff --git a/netwerk/protocol/websocket/nsWebSocketHandler.cpp b/netwerk/protocol/websocket/nsWebSocketHandler.cpp
> 
> >+#include "mozilla/net/NeckoChild.h"
> 
> See comments on logging below.  Also may need to remove #include of prlog.h
> below this.
> 
> > #if defined(PR_LOGGING)
> >-static PRLogModuleInfo *webSocketLog = nsnull;
> >+PRLogModuleInfo *webSocketLog = nsnull;
> > #endif
> > #define LOG(args) PR_LOG(webSocketLog, PR_LOG_DEBUG, args)
> 
> Hmm, we're going to want logging on when !DEBUG, which means we need to have
> FORCE_PR_LOG on AFICT.  Look at how nsHttp.h does it in lines 43-61.  I'm
> surprised we don't run into an error re-#defining LOG here--it's probably a
> warning?  Anyway, I think you'll want to centralize this in
> BaseWebChannel.h.  Make sure to compile an opt build and make sure that
> logging works in non-debug mode. 
> 
> Our logging is a real mess.

I really want to keep these sorts of things out of header files, because it just perpetuates the mess. What I've done in my updated patch is add LOG defines per file as needed, store webSocketLog in BaseWebSocketChannel.cpp, and add extern definitions to the other consumers, which is an acceptable duplication/cleanliness tradeoff in my opinion.
 
> >diff --git a/netwerk/protocol/websocket/nsWebSocketHandler.h b/netwerk/protocol/websocket/nsWebSocketHandler.h
> >+public:
> >+    BaseWebSocketHandler();
> >+
> >+    NS_DECL_NSIPROTOCOLHANDLER
> >+
> >+    NS_SCRIPTABLE NS_IMETHOD QueryInterface(const nsIID & uuid, void **result NS_OUTPARAM) = 0;
> >+    NS_IMETHOD_(nsrefcnt ) AddRef(void) = 0;
> >+    NS_IMETHOD_(nsrefcnt ) Release(void) = 0;
> 
> As mentioned on IRC, I don't think you need to declare methods you're
> inheriting from interfaces unless you're actually defining them, so these
> can be eliminated.

Turns out we refuse to link without these here. It might be an ambiguity thing since it's inheriting from multiple nsISupports bases?
 
> >+bool
> >+nsWebSocketHandlerParent::RecvDeleteSelf()
> >+{
> >+  LOG(("nsWebSocketHandlerParent::RecvDeleteSelf() %p\n", this));
> >+  mProtocol = nsnull;
> >+  mAuthProvider = nsnull;
> >+  return Send__delete__(this);
> >+}
> 
> Send__delete__ should be guarded with mIPCOpen.  The child process could
> conceivably drop dead at any point.

Well, any point is a bit much. I mean, I guess if nulling out a member triggered a destructor which spun the event loop... (ok, I'll fix it.)

Comment 43

[Josh Matthews [:jdm]]

Attachment: Remote websockets.

Haven't done anything further with tests, but this makes all the other requested changes.

Comment 44

[Josh Matthews [:jdm]]

From test_websocket.html comments:

>Test cases: I haven't looked at the mochitest(s), but let's make sure we cover
>- bogus URI in websocket constructor

 *  1. client tries to connect to a http scheme location;
 *  4. client tries to connect using a relative url;

>- valid but refused/unavailable URI

 *  3. client tries to connect to an non-existent ws server;
 *  5. client uses an invalid protocol value;

>- sync XHR in onmessage() (have server close WS: make sure readyState doesn't change to CLOSED during onmessage()).
>- test that involves an HTTP redirect (both failed and succeeded redirects would be good)

I'll see what I can do about these last two.

Comment 45

[Josh Matthews [:jdm]]

>Hmm, we're going to want logging on when !DEBUG, which means we need to have >FORCE_PR_LOG on AFICT.  Look at how nsHttp.h does it in lines 43-61.  I'm >surprised we don't run into an error re-#defining LOG here--it's probably a >warning?  Anyway, I think you'll want to centralize this in BaseWebChannel.h.  >Make sure to compile an opt build and make sure that logging works in non-debug >mode. 

NSPR logging is not part of opt builds.

Comment 46

[Josh Matthews [:jdm]]

Attachment: Test to make sure websocket callbacks don't reenter.

Reentrancy test.

Comment 47

[Josh Matthews [:jdm]]

Attachment: Test to make sure websocket callbacks don't reenter.

Comment 48

[Josh Matthews [:jdm]]

Attachment: Remote websockets.

Now with working nspr logging.

Comment 49

[Josh Matthews [:jdm]]

Attachment: Test to make sure websocket callbacks don't reenter.

Test updated to pass with new protocol restrictions. I also confirmed that it does in fact fail without the IPDL buffering present.

Comment 50

[Jason Duell]

Attachment: DON'T LAND: Re-entrancy test, with change of wording

Nice test.  So good, in fact, that it breaks non-e10s WS.  (did you check?  It breaks for me, at least, with 

  TEST-UNEXPECTED-FAIL |  | onclose called before onmessage!  (I changed wording--yours was wrong?)

  TEST-UNEXPECTED-FAIL |  | readystate should not be CLOSED - didn't expect 3, but got it
TEST-UNEXPECTED-FAIL |  | got a close callback before finishing onmessage

Not checking in--I'll move to a new bug.

Comment 51

[Jason Duell]

Comment on attachment 543566 [details] [diff] [review]
DON'T LAND: Re-entrancy test, with change of wording

Moved to bug 668948

Comment 52

[Jason Duell]

Attachment: interdiff of changes to main patch

Comment 53

[Jason Duell]

Attachment: Remote Websockets v4

Looks good.  The level of error/state checking is less in the e10s code than the mainline, but that looks like a good thing--the nsWebsocket.cpp code does enough checking that many of the things nsWebSocketHandler.cpp is checking for are not possible.  Should cleanup all that logic at some point.  Added some of those checks in e10s in form of assertions.

Running through try one last time, then will land.

Comment 54

[Jason Duell]

http://hg.mozilla.org/integration/mozilla-inbound/rev/4a894d4dc4da

I'm landing the main patch separately from the SpecialPowers patch, as with it I'm occasionally seeing 

 SpecialPowers is not defined at 
 http://mochi.test:8888/tests/Harness_sanity/file_SpecialPowersFrame1.html:11

mainly only on windows debug.  I don't see any reason why it could be causing that, but just in case..

Comment 55

[Jason Duell]

Comment on attachment 535998 [details] [diff] [review]
Update websocket test to use SpecialPowers.

Special powers patch landed as

http://hg.mozilla.org/integration/mozilla-inbound/rev/7a747adc8303

Comment 56

[Josh Matthews [:jdm]]

Adding dev-doc-needed for the SpecialPowers API addition.

Comment 57

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/4a894d4dc4da
http://hg.mozilla.org/mozilla-central/rev/7a747adc8303

Comment 58

[Dão Gottwald [:dao]]

(In reply to comment #55)
> Comment on attachment 535998 [details] [diff] [review] [review]
> Update websocket test to use SpecialPowers.
> 
> Special powers patch landed as
> 
> http://hg.mozilla.org/integration/mozilla-inbound/rev/7a747adc8303

Please check your commits (hg out) before pushing stuff.

Comment 59

[Andreea Pod]

I tried http://websockets.org/echo.html and http://mrdoob.com/projects/multiuserpad with browser.tabs.remote=true and the pages work. With browser.tabs.remote=false they don't work, is this the expected result?

Mozilla /5.0 (Android;Linux armv7l;rv:7.0a2) Gecko/20110804 Firefox/7.0a2 Fennec/7.0a2
Device: LG Opyimus 2X

Comment 60

[Eric Shepherd [:sheppy]]

From a developer standpoint, is the key issue here that WebSockets now work on mobile?

Comment 61

[Josh Matthews [:jdm]]

Yes. The problem with testing this is that there are not many public tests that exercise the more recent websocket protocol, plus virtually no tests make use of the prefixed version.

Comment 62

[Eric Shepherd [:sheppy]]

OK, the compatibility tables for WebSockets have been updated.

Comment 63

[Martijn Wargers (dead)]

Note that there are virtually no demos online on the web that work in Mozilla, because WebSocket was prefixed with "Moz" in bug 659324.

Comment 64

[Martijn Wargers (dead)]

It turns out there is one site that supports MozWebSocket, that is http://websocketstest.com/ . I used that for a litmus test.
https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=26364

Comment 65

[Camelia Urian]

Mozilla /5.0 (Android;Linux armv7l;rv:7.0) Gecko/20110830 Firefox/7.0 Fennec/7.0
Device: Motorola DROID2

Verified site: http://websocketstest.com/ and every thing was reported as working on that page, except for the 'HTTP Proxy' part.

Marking bug as verified fixed.