Bug 1045643 - Part 2: Notify the Loop server when the client has local media up and remote media being received, so that it can update the call connection status. r=nperriault, a=lmandel
authorMark Banner <standard8@mozilla.com>
Fri, 29 Aug 2014 11:22:18 +0100
changeset 216681 d820ef3b256d
parent 216680 be539410c211
child 216682 080344c7c80b
child 216684 f4bca655abc9
child 216686 9502b0a5c5f1
push id3872
push userryanvm@gmail.com
push date2014-09-08 19:43 +0000
treeherdermozilla-beta@d820ef3b256d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnperriault, lmandel
bugs1045643
milestone33.0
Bug 1045643 - Part 2: Notify the Loop server when the client has local media up and remote media being received, so that it can update the call connection status. r=nperriault, a=lmandel
browser/components/loop/content/js/conversation.js
browser/components/loop/content/js/conversation.jsx
browser/components/loop/content/shared/js/models.js
browser/components/loop/content/shared/js/views.js
browser/components/loop/content/shared/js/views.jsx
browser/components/loop/content/shared/js/websocket.js
browser/components/loop/standalone/content/js/webapp.js
browser/components/loop/test/desktop-local/conversation_test.js
browser/components/loop/test/shared/views_test.js
browser/components/loop/test/shared/websocket_test.js
browser/components/loop/test/standalone/webapp_test.js
--- a/browser/components/loop/content/js/conversation.js
+++ b/browser/components/loop/content/js/conversation.js
@@ -144,16 +144,19 @@ loop.conversation = (function(OT, mozL10
       this._conversation.set({loopVersion: loopVersion});
       this._conversation.once("accept", () => {
         this.navigate("call/accept", {trigger: true});
       });
       this._conversation.once("decline", () => {
         this.navigate("call/decline", {trigger: true});
       });
       this._conversation.once("call:incoming", this.startCall, this);
+      this._conversation.once("change:publishedStream", this._checkConnected, this);
+      this._conversation.once("change:subscribedStream", this._checkConnected, this);
+
       this._client.requestCallsInfo(loopVersion, (err, sessionData) => {
         if (err) {
           console.error("Failed to get the sessionData", err);
           // XXX Not the ideal response, but bug 1047410 will be replacing
           // this by better "call failed" UI.
           this._notifier.errorL10n("cannot_start_call_session_not_ready");
           return;
         }
@@ -184,16 +187,28 @@ loop.conversation = (function(OT, mozL10
         }));
       }.bind(this), function() {
         this._handleSessionError();
         return;
       }.bind(this));
     },
 
     /**
+     * Checks if the streams have been connected, and notifies the
+     * websocket that the media is now connected.
+     */
+    _checkConnected: function() {
+      // Check we've had both local and remote streams connected before
+      // sending the media up message.
+      if (this._conversation.streamsConnected()) {
+        this._websocket.mediaUp();
+      }
+    },
+
+    /**
      * Accepts an incoming call.
      */
     accept: function() {
       window.navigator.mozLoop.stopAlerting();
       this._websocket.accept();
       this._conversation.incoming();
     },
 
--- a/browser/components/loop/content/js/conversation.jsx
+++ b/browser/components/loop/content/js/conversation.jsx
@@ -144,16 +144,19 @@ loop.conversation = (function(OT, mozL10
       this._conversation.set({loopVersion: loopVersion});
       this._conversation.once("accept", () => {
         this.navigate("call/accept", {trigger: true});
       });
       this._conversation.once("decline", () => {
         this.navigate("call/decline", {trigger: true});
       });
       this._conversation.once("call:incoming", this.startCall, this);
+      this._conversation.once("change:publishedStream", this._checkConnected, this);
+      this._conversation.once("change:subscribedStream", this._checkConnected, this);
+
       this._client.requestCallsInfo(loopVersion, (err, sessionData) => {
         if (err) {
           console.error("Failed to get the sessionData", err);
           // XXX Not the ideal response, but bug 1047410 will be replacing
           // this by better "call failed" UI.
           this._notifier.errorL10n("cannot_start_call_session_not_ready");
           return;
         }
@@ -184,16 +187,28 @@ loop.conversation = (function(OT, mozL10
         }));
       }.bind(this), function() {
         this._handleSessionError();
         return;
       }.bind(this));
     },
 
     /**
+     * Checks if the streams have been connected, and notifies the
+     * websocket that the media is now connected.
+     */
+    _checkConnected: function() {
+      // Check we've had both local and remote streams connected before
+      // sending the media up message.
+      if (this._conversation.streamsConnected()) {
+        this._websocket.mediaUp();
+      }
+    },
+
+    /**
      * Accepts an incoming call.
      */
     accept: function() {
       window.navigator.mozLoop.stopAlerting();
       this._websocket.accept();
       this._conversation.incoming();
     },
 
--- a/browser/components/loop/content/shared/js/models.js
+++ b/browser/components/loop/content/shared/js/models.js
@@ -22,19 +22,23 @@ loop.shared.models = (function() {
                                // is the version received from the push
                                // notification and is used by the server to
                                // determine the pending calls
       sessionId:    undefined, // OT session id
       sessionToken: undefined, // OT session token
       apiKey:       undefined,  // OT api key
       callId:       undefined,     // The callId on the server
       progressURL:  undefined,     // The websocket url to use for progress
-      websocketToken: undefined    // The token to use for websocket auth, this is
+      websocketToken: undefined,   // The token to use for websocket auth, this is
                                    // stored as a hex string which is what the server
                                    // requires.
+      subscribedStream: false,     // Used to indicate that a stream has been
+                                   // subscribed to
+      publishedStream: false       // Used to indicate that a stream has been
+                                   // published
     },
 
     /**
      * SDK object.
      * @type {OT}
      */
     sdk: undefined,
 
@@ -173,16 +177,49 @@ loop.shared.models = (function() {
      */
     endSession: function() {
       this.session.disconnect();
       this.set("ongoing", false)
           .once("session:ended", this.stopListening, this);
     },
 
     /**
+     * Publishes a local stream.
+     *
+     * @param {Publisher} publisher The publisher object to publish
+     *                              to the session.
+     */
+    publish: function(publisher) {
+      this.session.publish(publisher);
+      this.set("publishedStream", true);
+    },
+
+    /**
+     * Subscribes to a remote stream.
+     *
+     * @param {Stream} stream The remote stream to subscribe to.
+     * @param {DOMElement} element The element to display the stream in.
+     * @param {Object} config The display properties to set on the stream as
+     *                        documented in:
+     * https://tokbox.com/opentok/libraries/client/js/reference/Session.html#subscribe
+     */
+    subscribe: function(stream, element, config) {
+      this.session.subscribe(stream, element, config);
+      this.set("subscribedStream", true);
+    },
+
+    /**
+     * Returns true if a stream has been published and a stream has been
+     * subscribed to.
+     */
+    streamsConnected: function() {
+      return this.get("publishedStream") && this.get("subscribedStream");
+    },
+
+    /**
      * Clears current pending call timer, if any.
      */
     _clearPendingCallTimer: function() {
       if (this._pendingCallTimer) {
         clearTimeout(this._pendingCallTimer);
       }
     },
 
--- a/browser/components/loop/content/shared/js/views.js
+++ b/browser/components/loop/content/shared/js/views.js
@@ -250,23 +250,17 @@ loop.shared.views = (function(_, OT, l10
      *      element.
      *
      * http://tokbox.com/opentok/libraries/client/js/reference/StreamEvent.html
      *
      * @param  {StreamEvent} event
      */
     _streamCreated: function(event) {
       var incoming = this.getDOMNode().querySelector(".incoming");
-      event.streams.forEach(function(stream) {
-        if (stream.connection.connectionId !==
-            this.props.model.session.connection.connectionId) {
-          this.props.model.session.subscribe(stream, incoming,
-                                             this.publisherConfig);
-        }
-      }, this);
+      this.props.model.subscribe(event.stream, incoming, this.publisherConfig);
     },
 
     /**
      * Publishes remote streams available once a session is connected.
      *
      * http://tokbox.com/opentok/libraries/client/js/reference/SessionConnectEvent.html
      *
      * @param  {SessionConnectEvent} event
@@ -293,17 +287,17 @@ loop.shared.views = (function(_, OT, l10
 
       this.listenTo(this.publisher, "streamDestroyed", function() {
         this.setState({
           audio: {enabled: false},
           video: {enabled: false}
         });
       }.bind(this));
 
-      this.props.model.session.publish(this.publisher);
+      this.props.model.publish(this.publisher);
     },
 
     /**
      * Toggles streaming status for a given stream type.
      *
      * @param  {String}  type     Stream type ("audio" or "video").
      * @param  {Boolean} enabled  Enabled stream flag.
      */
--- a/browser/components/loop/content/shared/js/views.jsx
+++ b/browser/components/loop/content/shared/js/views.jsx
@@ -250,23 +250,17 @@ loop.shared.views = (function(_, OT, l10
      *      element.
      *
      * http://tokbox.com/opentok/libraries/client/js/reference/StreamEvent.html
      *
      * @param  {StreamEvent} event
      */
     _streamCreated: function(event) {
       var incoming = this.getDOMNode().querySelector(".incoming");
-      event.streams.forEach(function(stream) {
-        if (stream.connection.connectionId !==
-            this.props.model.session.connection.connectionId) {
-          this.props.model.session.subscribe(stream, incoming,
-                                             this.publisherConfig);
-        }
-      }, this);
+      this.props.model.subscribe(event.stream, incoming, this.publisherConfig);
     },
 
     /**
      * Publishes remote streams available once a session is connected.
      *
      * http://tokbox.com/opentok/libraries/client/js/reference/SessionConnectEvent.html
      *
      * @param  {SessionConnectEvent} event
@@ -293,17 +287,17 @@ loop.shared.views = (function(_, OT, l10
 
       this.listenTo(this.publisher, "streamDestroyed", function() {
         this.setState({
           audio: {enabled: false},
           video: {enabled: false}
         });
       }.bind(this));
 
-      this.props.model.session.publish(this.publisher);
+      this.props.model.publish(this.publisher);
     },
 
     /**
      * Toggles streaming status for a given stream type.
      *
      * @param  {String}  type     Stream type ("audio" or "video").
      * @param  {Boolean} enabled  Enabled stream flag.
      */
--- a/browser/components/loop/content/shared/js/websocket.js
+++ b/browser/components/loop/content/shared/js/websocket.js
@@ -133,16 +133,27 @@ loop.CallConnectionWebSocket = (function
     accept: function() {
       this._send({
         messageType: "action",
         event: "accept"
       });
     },
 
     /**
+     * Notifies the server that the outgoing media is up, and the
+     * incoming media is being received.
+     */
+    mediaUp: function() {
+      this._send({
+        messageType: "action",
+        event: "media-up"
+      });
+    },
+
+    /**
      * Sends data on the websocket.
      *
      * @param {Object} data The data to send.
      */
     _send: function(data) {
       this._log("WS Sending", data);
 
       this.socket.send(JSON.stringify(data));
--- a/browser/components/loop/standalone/content/js/webapp.js
+++ b/browser/components/loop/standalone/content/js/webapp.js
@@ -186,16 +186,28 @@ loop.webapp = (function($, _, OT) {
         this._notifier.errorL10n("cannot_start_call_session_not_ready");
         return;
       }.bind(this));
 
       this._websocket.on("progress", this._handleWebSocketProgress, this);
     },
 
     /**
+     * Checks if the streams have been connected, and notifies the
+     * websocket that the media is now connected.
+     */
+    _checkConnected: function() {
+      // Check we've had both local and remote streams connected before
+      // sending the media up message.
+      if (this._conversation.streamsConnected()) {
+        this._websocket.mediaUp();
+      }
+    },
+
+    /**
      * Used to receive websocket progress and to determine how to handle
      * it if appropraite.
      */
     _handleWebSocketProgress: function(progressData) {
       if (progressData.state === "terminated") {
         // XXX Before adding more states here, the basic protocol messages to the
         // server need implementing on both the standalone and desktop side.
         // These are covered by bug 1045643, but also check the dependencies on
@@ -266,16 +278,18 @@ loop.webapp = (function($, _, OT) {
       this._conversation.set("loopToken", loopToken);
 
       var startView = new StartConversationView({
         model: this._conversation,
         notifier: this._notifier,
         client: this._client
       });
       this._conversation.once("call:outgoing:setup", this.setupOutgoingCall, this);
+      this._conversation.once("change:publishedStream", this._checkConnected, this);
+      this._conversation.once("change:subscribedStream", this._checkConnected, this);
       this.loadView(startView);
     },
 
     /**
      * Loads conversation establishment view.
      *
      */
     loadConversation: function(loopToken) {
--- a/browser/components/loop/test/desktop-local/conversation_test.js
+++ b/browser/components/loop/test/desktop-local/conversation_test.js
@@ -447,16 +447,59 @@ describe("loop.conversation", function()
 
       it("should navigate to call/{token} when network disconnects",
         function() {
           conversation.trigger("session:network-disconnected");
 
           sinon.assert.calledOnce(router.navigate);
           sinon.assert.calledWith(router.navigate, "call/ended");
         });
+
+      describe("Published and Subscribed Streams", function() {
+        beforeEach(function() {
+          router._websocket = {
+            mediaUp: sinon.spy()
+          };
+          router.incoming("fakeVersion");
+        });
+
+        describe("publishStream", function() {
+          it("should not notify the websocket if only one stream is up",
+            function() {
+              conversation.set("publishedStream", true);
+
+              sinon.assert.notCalled(router._websocket.mediaUp);
+            });
+
+          it("should notify the websocket that media is up if both streams" +
+             "are connected", function() {
+              conversation.set("subscribedStream", true);
+              conversation.set("publishedStream", true);
+
+              sinon.assert.calledOnce(router._websocket.mediaUp);
+            });
+        });
+
+        describe("subscribedStream", function() {
+          it("should not notify the websocket if only one stream is up",
+            function() {
+              conversation.set("subscribedStream", true);
+
+              sinon.assert.notCalled(router._websocket.mediaUp);
+            });
+
+          it("should notify the websocket that media is up if both streams" +
+             "are connected", function() {
+              conversation.set("publishedStream", true);
+              conversation.set("subscribedStream", true);
+
+              sinon.assert.calledOnce(router._websocket.mediaUp);
+            });
+        });
+      });
     });
   });
 
   describe("EndedCallView", function() {
     describe("#closeWindow", function() {
       it("should close the conversation window", function() {
         sandbox.stub(window, "close");
         var view = new loop.conversation.EndedCallView();
--- a/browser/components/loop/test/shared/views_test.js
+++ b/browser/components/loop/test/shared/views_test.js
@@ -328,25 +328,24 @@ describe("loop.shared.views", function()
 
       describe("Model events", function() {
         it("should start streaming on session:connected", function() {
           model.trigger("session:connected");
 
           sinon.assert.calledOnce(fakeSDK.initPublisher);
         });
 
-        it("should publish remote streams on session:stream-created",
+        it("should publish remote stream on session:stream-created",
           function() {
             var s1 = {connection: {connectionId: 42}};
-            var s2 = {connection: {connectionId: 43}};
 
-            model.trigger("session:stream-created", {streams: [s1, s2]});
+            model.trigger("session:stream-created", {stream: s1});
 
             sinon.assert.calledOnce(fakeSession.subscribe);
-            sinon.assert.calledWith(fakeSession.subscribe, s2);
+            sinon.assert.calledWith(fakeSession.subscribe, s1);
           });
 
         it("should unpublish local stream on session:ended", function() {
           comp.startPublishing();
 
           model.trigger("session:ended");
 
           sinon.assert.calledOnce(fakeSession.unpublish);
--- a/browser/components/loop/test/shared/websocket_test.js
+++ b/browser/components/loop/test/shared/websocket_test.js
@@ -157,16 +157,30 @@ describe("loop.CallConnectionWebSocket",
         sinon.assert.calledOnce(dummySocket.send);
         sinon.assert.calledWithExactly(dummySocket.send, JSON.stringify({
           messageType: "action",
           event: "accept"
         }));
       });
     });
 
+    describe("#mediaUp", function() {
+      it("should send a media-up message to the server", function() {
+        callWebSocket.promiseConnect();
+
+        callWebSocket.mediaUp();
+
+        sinon.assert.calledOnce(dummySocket.send);
+        sinon.assert.calledWithExactly(dummySocket.send, JSON.stringify({
+          messageType: "action",
+          event: "media-up"
+        }));
+      });
+    });
+
     describe("Events", function() {
       beforeEach(function() {
         sandbox.stub(callWebSocket, "trigger");
 
         callWebSocket.promiseConnect();
       });
 
       describe("Progress", function() {
--- a/browser/components/loop/test/standalone/webapp_test.js
+++ b/browser/components/loop/test/standalone/webapp_test.js
@@ -392,16 +392,59 @@ describe("loop.webapp", function() {
       it("should navigate to call/{token} when network disconnects",
         function() {
           conversation.trigger("session:network-disconnected");
 
           sinon.assert.calledOnce(router.navigate);
           sinon.assert.calledWithMatch(router.navigate, "call/fakeToken");
         });
 
+      describe("Published and Subscribed Streams", function() {
+        beforeEach(function() {
+          router._websocket = {
+            mediaUp: sinon.spy()
+          };
+          router.initiate();
+        });
+
+        describe("publishStream", function() {
+          it("should not notify the websocket if only one stream is up",
+            function() {
+              conversation.set("publishedStream", true);
+
+              sinon.assert.notCalled(router._websocket.mediaUp);
+            });
+
+          it("should notify the websocket that media is up if both streams" +
+             "are connected", function() {
+              conversation.set("subscribedStream", true);
+              conversation.set("publishedStream", true);
+
+              sinon.assert.calledOnce(router._websocket.mediaUp);
+            });
+        });
+
+        describe("subscribedStream", function() {
+          it("should not notify the websocket if only one stream is up",
+            function() {
+              conversation.set("subscribedStream", true);
+
+              sinon.assert.notCalled(router._websocket.mediaUp);
+            });
+
+          it("should notify the websocket that media is up if both streams" +
+             "are connected", function() {
+              conversation.set("publishedStream", true);
+              conversation.set("subscribedStream", true);
+
+              sinon.assert.calledOnce(router._websocket.mediaUp);
+            });
+        });
+      });
+
       describe("#setupOutgoingCall", function() {
         beforeEach(function() {
           router.initiate();
         });
 
         describe("No loop token", function() {
           it("should navigate to home", function() {
             conversation.setupOutgoingCall();