Bug 1201805 - [Presentation WebAPI] Fix collaboration issues with control channel. Part 2 - Adjust the timing to send offer. r=smaug

dom/presentation/PresentationSessionInfo.cpp

@@ -344,18 +344,21 @@ PresentationSessionInfo::NotifyData(const nsACString& aData)
  * Implementation of PresentationRequesterInfo
  *
  * During presentation session establishment, the sender expects the following
- * after trying to establish the control channel: (The order between step 2 and
- * 3 is not guaranteed.)
+ * after trying to establish the control channel: (The order between step 3 and
+ * 4 is not guaranteed.)
  * 1. |Init| is called to open a socket |mServerSocket| for data transport
- *    channel and send the offer to the receiver via the control channel.
- * 2.1 |OnSocketAccepted| of |nsIServerSocketListener| is called to indicate the
+ *    channel.
+ * 2. |NotifyOpened| of |nsIPresentationControlChannelListener| is called to
+ *    indicate the control channel is ready to use. Then send the offer to the
+ *    receiver via the control channel.
+ * 3.1 |OnSocketAccepted| of |nsIServerSocketListener| is called to indicate the
  *     data transport channel is connected. Then initialize |mTransport|.
- * 2.2 |NotifyTransportReady| of |nsIPresentationSessionTransportCallback| is
+ * 3.2 |NotifyTransportReady| of |nsIPresentationSessionTransportCallback| is
  *     called.
- * 3. |OnAnswer| of |nsIPresentationControlChannelListener| is called to
+ * 4. |OnAnswer| of |nsIPresentationControlChannelListener| is called to
  *    indicate the receiver is ready. Close the control channel since it's no
  *    longer needed.
- * 4. Once both step 2 and 3 are done, the presentation session is ready to use.
+ * 5. Once both step 3 and 4 are done, the presentation session is ready to use.
  *    So notify the listener of CONNECTED state.
  */
 
@@ -385,26 +388,6 @@ PresentationRequesterInfo::Init(nsIPresentationControlChannel* aControlChannel)
     return rv;
   }
 
-  // Prepare and send the offer.
-  int32_t port;
-  rv = mServerSocket->GetPort(&port);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  nsCString address;
-  rv = GetAddress(address);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  nsRefPtr<PresentationChannelDescription> description =
-    new PresentationChannelDescription(address, static_cast<uint16_t>(port));
-  rv = mControlChannel->SendOffer(description);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
   return NS_OK;
 }
 
@@ -496,8 +479,22 @@ PresentationRequesterInfo::OnAnswer(nsIPresentationChannelDescription* aDescript
 NS_IMETHODIMP
 PresentationRequesterInfo::NotifyOpened()
 {
-  // Do nothing and wait for receiver to be ready.
-  return NS_OK;
+  // Prepare and send the offer.
+  int32_t port;
+  nsresult rv = mServerSocket->GetPort(&port);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  nsCString address;
+  rv = GetAddress(address);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
+
+  nsRefPtr<PresentationChannelDescription> description =
+    new PresentationChannelDescription(address, static_cast<uint16_t>(port));
+  return mControlChannel->SendOffer(description);
 }
 
 NS_IMETHODIMP
@@ -666,24 +663,19 @@ PresentationResponderInfo::InitTransportAndSendAnswer()
   // description for the answer, which is not actually checked at requester side.
   nsCOMPtr<nsINetAddr> selfAddr;
   rv = mTransport->GetSelfAddress(getter_AddRefs(selfAddr));
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
+  NS_WARN_IF(NS_FAILED(rv));
 
   nsCString address;
-  selfAddr->GetAddress(address);
-  uint16_t port;
-  selfAddr->GetPort(&port);
+  uint16_t port = 0;
+  if (NS_SUCCEEDED(rv)) {
+    selfAddr->GetAddress(address);
+    selfAddr->GetPort(&port);
+  }
   nsCOMPtr<nsIPresentationChannelDescription> description =
     new PresentationChannelDescription(address, port);
 
-  rv = mControlChannel->SendAnswer(description);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
-  }
-
-  return NS_OK;
- }
+  return mControlChannel->SendAnswer(description);
+}
 
 nsresult
 PresentationResponderInfo::UntrackFromService()

dom/presentation/tests/mochitest/PresentationSessionChromeScript.js

@@ -152,6 +152,10 @@ const mockedControlChannel = {
     sendAsyncMessage('answer-received');
     this._listener.QueryInterface(Ci.nsIPresentationControlChannelListener).onAnswer(mockedChannelDescription);
   },
+  simulateNotifyOpened: function() {
+    sendAsyncMessage('control-channel-opened');
+    this._listener.QueryInterface(Ci.nsIPresentationControlChannelListener).notifyOpened();
+  },
 };
 
 const mockedDevice = {
@@ -369,6 +373,10 @@ addMessageListener('trigger-incoming-transport', function() {
   mockedServerSocket.simulateOnSocketAccepted(mockedServerSocket, mockedSocketTransport);
 });
 
+addMessageListener('trigger-control-channel-open', function(reason) {
+  mockedControlChannel.simulateNotifyOpened();
+});
+
 addMessageListener('trigger-control-channel-close', function(reason) {
   mockedControlChannel.close(reason);
 });

dom/presentation/tests/mochitest/test_presentation_sender.html

@@ -41,6 +41,12 @@ function testStartSession() {
     gScript.addMessageListener('control-channel-established', function controlChannelEstablishedHandler() {
       gScript.removeMessageListener('control-channel-established', controlChannelEstablishedHandler);
       info("A control channel is established.");
+      gScript.sendAsyncMessage('trigger-control-channel-open');
+    });
+
+    gScript.addMessageListener('control-channel-opened', function controlChannelOpenedHandler(aReason) {
+      gScript.removeMessageListener('control-channel-opened', controlChannelOpenedHandler);
+      info("The control channel is opened.");
     });
 
     gScript.addMessageListener('control-channel-closed', function controlChannelClosedHandler(aReason) {

dom/presentation/tests/mochitest/test_presentation_sender_disconnect.html

@@ -41,6 +41,12 @@ function testStartSession() {
     gScript.addMessageListener('control-channel-established', function controlChannelEstablishedHandler() {
       gScript.removeMessageListener('control-channel-established', controlChannelEstablishedHandler);
       info("A control channel is established.");
+      gScript.sendAsyncMessage('trigger-control-channel-open');
+    });
+
+    gScript.addMessageListener('control-channel-opened', function controlChannelOpenedHandler(aReason) {
+      gScript.removeMessageListener('control-channel-opened', controlChannelOpenedHandler);
+      info("The control channel is opened.");
     });
 
     gScript.addMessageListener('control-channel-closed', function controlChannelClosedHandler(aReason) {

dom/presentation/tests/mochitest/test_presentation_sender_start_session_error.html

@@ -76,6 +76,12 @@ function testStartSessionUnexpectedControlChannelCloseBeforeDataTransportInit()
     gScript.addMessageListener('control-channel-established', function controlChannelEstablishedHandler() {
       gScript.removeMessageListener('control-channel-established', controlChannelEstablishedHandler);
       info("A control channel is established.");
+      gScript.sendAsyncMessage('trigger-control-channel-open');
+    });
+
+    gScript.addMessageListener('control-channel-opened', function controlChannelOpenedHandler(aReason) {
+      gScript.removeMessageListener('control-channel-opened', controlChannelOpenedHandler);
+      info("The control channel is opened.");
     });
 
     gScript.addMessageListener('control-channel-closed', function controlChannelClosedHandler(aReason) {
@@ -113,6 +119,12 @@ function testStartSessionUnexpectedControlChannelCloseBeforeDataTransportReady()
     gScript.addMessageListener('control-channel-established', function controlChannelEstablishedHandler() {
       gScript.removeMessageListener('control-channel-established', controlChannelEstablishedHandler);
       info("A control channel is established.");
+      gScript.sendAsyncMessage('trigger-control-channel-open');
+    });
+
+    gScript.addMessageListener('control-channel-opened', function controlChannelOpenedHandler(aReason) {
+      gScript.removeMessageListener('control-channel-opened', controlChannelOpenedHandler);
+      info("The control channel is opened.");
     });
 
     gScript.addMessageListener('control-channel-closed', function controlChannelClosedHandler(aReason) {
@@ -161,6 +173,12 @@ function testStartSessionUnexpectedDataTransportClose() {
     gScript.addMessageListener('control-channel-established', function controlChannelEstablishedHandler() {
       gScript.removeMessageListener('control-channel-established', controlChannelEstablishedHandler);
       info("A control channel is established.");
+      gScript.sendAsyncMessage('trigger-control-channel-open');
+    });
+
+    gScript.addMessageListener('control-channel-opened', function controlChannelOpenedHandler(aReason) {
+      gScript.removeMessageListener('control-channel-opened', controlChannelOpenedHandler);
+      info("The control channel is opened.");
     });
 
     gScript.addMessageListener('control-channel-closed', function controlChannelClosedHandler(aReason) {