Bug 951188 - [RTSP][V1.3] No error notification when the RTSP link fails to load. r=sworkman, a=1.3+
authorEthan Tseng <ettseng@mozilla.com>
Thu, 30 Jan 2014 10:00:53 +0800
changeset 176140 9b39537be627781478fedf72deb897d39f01c462
parent 176139 0980f4b18b9867dd860ae36580e8698258259430
child 176141 cbda34620583be867d26e74a6362985fb9648ab6
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssworkman, 1
bugs951188
milestone28.0a2
Bug 951188 - [RTSP][V1.3] No error notification when the RTSP link fails to load. r=sworkman, a=1.3+
content/media/RtspMediaResource.cpp
netwerk/protocol/rtsp/rtsp/ARTSPConnection.cpp
netwerk/protocol/rtsp/rtsp/ARTSPConnection.h
netwerk/protocol/rtsp/rtsp/RTSPConnectionHandler.h
netwerk/protocol/rtsp/rtsp/RTSPSource.cpp
--- a/content/media/RtspMediaResource.cpp
+++ b/content/media/RtspMediaResource.cpp
@@ -535,20 +535,19 @@ RtspMediaResource::OnDisconnected(uint8_
   // triggered when media element was destroyed and mDecoder was already
   // shutdown.
   if (!mDecoder) {
     return NS_OK;
   }
 
   if (aReason == NS_ERROR_NOT_INITIALIZED ||
       aReason == NS_ERROR_CONNECTION_REFUSED ||
-      aReason == NS_ERROR_NOT_CONNECTED) {
-
+      aReason == NS_ERROR_NOT_CONNECTED ||
+      aReason == NS_ERROR_NET_TIMEOUT) {
     RTSPMLOG("Error in OnDisconnected 0x%x", aReason);
-
     mDecoder->NetworkError();
     return NS_OK;
   }
 
   // Resetting the decoder and media element when the connection
   // between Rtsp client and server goes down.
   mDecoder->ResetConnectionState();
   return NS_OK;
--- a/netwerk/protocol/rtsp/rtsp/ARTSPConnection.cpp
+++ b/netwerk/protocol/rtsp/rtsp/ARTSPConnection.cpp
@@ -39,26 +39,28 @@
 #include "nsNetCID.h"
 #include "nsIServiceManager.h"
 #include "nsICryptoHash.h"
 
 namespace android {
 
 // static
 const int64_t ARTSPConnection::kSelectTimeoutUs = 1000ll;
+const int64_t ARTSPConnection::kSelectTimeoutRetries = 10000ll;
 
 ARTSPConnection::ARTSPConnection(bool uidValid, uid_t uid)
     : mUIDValid(uidValid),
       mUID(uid),
       mState(DISCONNECTED),
       mAuthType(NONE),
       mSocket(-1),
       mConnectionID(0),
       mNextCSeq(0),
-      mReceiveResponseEventPending(false) {
+      mReceiveResponseEventPending(false),
+      mNumSelectTimeoutRetries(0) {
     MakeUserAgent(&mUserAgent);
 }
 
 ARTSPConnection::~ARTSPConnection() {
     if (mSocket >= 0) {
         LOGE("Connection is still open, closing the socket.");
         if (mUIDValid) {
             HTTPBase::UnRegisterSocketUserTag(mSocket);
@@ -71,18 +73,24 @@ ARTSPConnection::~ARTSPConnection() {
 void ARTSPConnection::connect(const char *url, const sp<AMessage> &reply) {
     sp<AMessage> msg = new AMessage(kWhatConnect, id());
     msg->setString("url", url);
     msg->setMessage("reply", reply);
     msg->post();
 }
 
 void ARTSPConnection::disconnect(const sp<AMessage> &reply) {
+    int32_t result;
     sp<AMessage> msg = new AMessage(kWhatDisconnect, id());
     msg->setMessage("reply", reply);
+    if (reply->findInt32("result", &result)) {
+      msg->setInt32("result", result);
+    } else {
+      msg->setInt32("result", OK);
+    }
     msg->post();
 }
 
 void ARTSPConnection::sendRequest(
         const char *request, const sp<AMessage> &reply) {
     sp<AMessage> msg = new AMessage(kWhatSendRequest, id());
     msg->setString("request", request);
     msg->setMessage("reply", reply);
@@ -280,16 +288,17 @@ void ARTSPConnection::onConnect(const sp
 
     int err = ::connect(
             mSocket, (const struct sockaddr *)&remote, sizeof(remote));
 
     reply->setInt32("server-ip", ntohl(remote.sin_addr.s_addr));
 
     if (err < 0) {
         if (errno == EINPROGRESS) {
+            mNumSelectTimeoutRetries = 0;
             sp<AMessage> msg = new AMessage(kWhatCompleteConnection, id());
             msg->setMessage("reply", reply);
             msg->setInt32("connection-id", mConnectionID);
             msg->post();
             return;
         }
 
         reply->setInt32("result", -errno);
@@ -319,30 +328,32 @@ void ARTSPConnection::performDisconnect(
     mSocket = -1;
 
     flushPendingRequests();
 
     mUser.clear();
     mPass.clear();
     mAuthType = NONE;
     mNonce.clear();
+    mNumSelectTimeoutRetries = 0;
 
     mState = DISCONNECTED;
 }
 
 void ARTSPConnection::onDisconnect(const sp<AMessage> &msg) {
+    int32_t result;
     if (mState == CONNECTED || mState == CONNECTING) {
         performDisconnect();
     }
 
     sp<AMessage> reply;
     CHECK(msg->findMessage("reply", &reply));
+    CHECK(msg->findInt32("result", &result));
 
-    reply->setInt32("result", OK);
-
+    reply->setInt32("result", result);
     reply->post();
 }
 
 void ARTSPConnection::onCompleteConnection(const sp<AMessage> &msg) {
     sp<AMessage> reply;
     CHECK(msg->findMessage("reply", &reply));
 
     int32_t connectionID;
@@ -363,19 +374,29 @@ void ARTSPConnection::onCompleteConnecti
     fd_set ws;
     FD_ZERO(&ws);
     FD_SET(mSocket, &ws);
 
     int res = select(mSocket + 1, NULL, &ws, NULL, &tv);
     CHECK_GE(res, 0);
 
     if (res == 0) {
-        // Timed out. Not yet connected.
-
-        msg->post();
+        // select() timed out. Not yet connected.
+        if (mNumSelectTimeoutRetries < kSelectTimeoutRetries) {
+            mNumSelectTimeoutRetries++;
+            msg->post();
+        } else {
+            // Connection timeout here.
+            // We cannot establish TCP connection, abort the connect
+            // and reply an error to RTSPConnectionHandler.
+            LOGE("Connection timeout. Failed to connect to the server.");
+            mNumSelectTimeoutRetries = 0;
+            reply->setInt32("result", -ETIMEDOUT);
+            reply->post();
+        }
         return;
     }
 
     int err;
     socklen_t optionLen = sizeof(err);
     CHECK_EQ(getsockopt(mSocket, SOL_SOCKET, SO_ERROR, &err, &optionLen), 0);
     CHECK_EQ(optionLen, (socklen_t)sizeof(err));
 
--- a/netwerk/protocol/rtsp/rtsp/ARTSPConnection.h
+++ b/netwerk/protocol/rtsp/rtsp/ARTSPConnection.h
@@ -68,27 +68,29 @@ private:
 
     enum AuthType {
         NONE,
         BASIC,
         DIGEST
     };
 
     static const int64_t kSelectTimeoutUs;
+    static const int64_t kSelectTimeoutRetries;
 
     bool mUIDValid;
     uid_t mUID;
     State mState;
     AString mUser, mPass;
     AuthType mAuthType;
     AString mNonce;
     int mSocket;
     int32_t mConnectionID;
     int32_t mNextCSeq;
     bool mReceiveResponseEventPending;
+    int64_t mNumSelectTimeoutRetries;
 
     KeyedVector<int32_t, sp<AMessage> > mPendingRequests;
 
     sp<AMessage> mObserveBinaryMessage;
 
     AString mUserAgent;
 
     void performDisconnect();
--- a/netwerk/protocol/rtsp/rtsp/RTSPConnectionHandler.h
+++ b/netwerk/protocol/rtsp/rtsp/RTSPConnectionHandler.h
@@ -422,31 +422,37 @@ struct RtspConnectionHandler : public AH
                     request.append(mSessionURL);
                     request.append(" RTSP/1.0\r\n");
                     request.append("Accept: application/sdp\r\n");
                     request.append("\r\n");
 
                     sp<AMessage> reply = new AMessage('desc', id());
                     mConn->sendRequest(request.c_str(), reply);
                 } else {
-                    (new AMessage('disc', id()))->post();
+                    sp<AMessage> reply = new AMessage('disc', id());
+                    reply->setInt32("result", result);
+                    mConn->disconnect(reply);
                 }
                 break;
             }
 
             case 'disc':
             {
                 ++mKeepAliveGeneration;
 
                 int32_t reconnect;
                 if (msg->findInt32("reconnect", &reconnect) && reconnect) {
                     sp<AMessage> reply = new AMessage('conn', id());
                     mConn->connect(mOriginalSessionURL.c_str(), reply);
                 } else {
-                    (new AMessage('quit', id()))->post();
+                    int32_t result;
+                    CHECK(msg->findInt32("result", &result));
+                    sp<AMessage> reply = new AMessage('quit', id());
+                    reply->setInt32("result", result);
+                    reply->post();
                 }
                 break;
             }
 
             case 'desc':
             {
                 int32_t result;
                 CHECK(msg->findInt32("result", &result));
@@ -545,16 +551,17 @@ struct RtspConnectionHandler : public AH
                                 setupTrack(1);
                             }
                         }
                     }
                 }
 
                 if (result != OK) {
                     sp<AMessage> reply = new AMessage('disc', id());
+                    reply->setInt32("result", result);
                     mConn->disconnect(reply);
                 }
                 break;
             }
 
             case 'setu':
             {
                 size_t index;
@@ -676,16 +683,17 @@ struct RtspConnectionHandler : public AH
                     } else {
                       msg->setInt32("isSeekable", 0);
                     }
                     // Notify the Rtsp Controller that we are ready to play.
                     msg->setInt32("what", kWhatConnected);
                     msg->post();
                 } else {
                     sp<AMessage> reply = new AMessage('disc', id());
+                    reply->setInt32("result", result);
                     mConn->disconnect(reply);
                 }
                 break;
             }
             case 'pause':
             {
                 mPausePending = true;
                 LOGI("pause completed");
@@ -718,16 +726,17 @@ struct RtspConnectionHandler : public AH
                         sp<AMessage> timeout = new AMessage('tiou', id());
                         timeout->post(kStartupTimeoutUs);
                         mPausePending = false;
                     }
                 }
 
                 if (result != OK) {
                     sp<AMessage> reply = new AMessage('disc', id());
+                    reply->setInt32("result", result);
                     mConn->disconnect(reply);
                 }
 
                 break;
             }
 
             case 'aliv':
             {
@@ -836,31 +845,34 @@ struct RtspConnectionHandler : public AH
             {
                 int32_t result;
                 CHECK(msg->findInt32("result", &result));
 
                 LOGI("TEARDOWN completed with result %d (%s)",
                      result, strerror(-result));
 
                 sp<AMessage> reply = new AMessage('disc', id());
+                reply->setInt32("result", result);
 
                 int32_t reconnect;
                 if (msg->findInt32("reconnect", &reconnect) && reconnect) {
                     reply->setInt32("reconnect", true);
                 }
 
                 mConn->disconnect(reply);
                 break;
             }
 
             case 'quit':
             {
+                int32_t result;
+                CHECK(msg->findInt32("result", &result));
                 sp<AMessage> msg = mNotify->dup();
                 msg->setInt32("what", kWhatDisconnected);
-                msg->setInt32("result", UNKNOWN_ERROR);
+                msg->setInt32("result", result);
                 msg->post();
                 break;
             }
 
             case 'chek':
             {
                 int32_t generation;
                 CHECK(msg->findInt32("generation", &generation));
--- a/netwerk/protocol/rtsp/rtsp/RTSPSource.cpp
+++ b/netwerk/protocol/rtsp/rtsp/RTSPSource.cpp
@@ -553,33 +553,34 @@ void RTSPSource::onConnected(bool isSeek
 
     mState = CONNECTED;
 }
 
 void RTSPSource::onDisconnected(const sp<AMessage> &msg) {
     status_t err;
     CHECK(msg != NULL);
     CHECK(msg->findInt32("result", &err));
-    CHECK_NE(err, (status_t)OK);
 
-    CHECK(mLooper != NULL);
-    CHECK(mHandler != NULL);
-    mLooper->unregisterHandler(mHandler->id());
-    mHandler.clear();
+    if ((mLooper != NULL) && (mHandler != NULL)) {
+      mLooper->unregisterHandler(mHandler->id());
+      mHandler.clear();
+    }
 
     mState = DISCONNECTED;
     mFinalResult = err;
 
     if (mDisconnectReplyID != 0) {
         finishDisconnectIfPossible();
     }
     if (mListener) {
-      // err is always set to UNKNOWN_ERROR from
-      // Android right now, rename err to NS_ERROR_NET_TIMEOUT.
-      mListener->OnDisconnected(0, NS_ERROR_NET_TIMEOUT);
+      if (err == OK) {
+        mListener->OnDisconnected(0, NS_OK);
+      } else {
+        mListener->OnDisconnected(0, NS_ERROR_NET_TIMEOUT);
+      }
     }
     mAudioTrack = NULL;
     mVideoTrack = NULL;
     mTracks.clear();
 }
 
 void RTSPSource::finishDisconnectIfPossible() {
     if (mState != DISCONNECTED) {