Bug 771318 - Code cleanup before actual fix. r=mcmanus

netwerk/protocol/websocket/WebSocketChannel.cpp

@@ -222,7 +222,7 @@ public:
   }
 
   // returns true if channel connects immediately, or false if it's delayed
-  bool DelayOrBegin(WebSocketChannel *ws)
+  void DelayOrBegin(WebSocketChannel *ws)
   {
     if (!mDelaysDisabled) {
       PRUint32 failIndex = 0;
@@ -244,7 +244,7 @@ public:
               LOG(("WebSocket: delaying websocket [this=%p] by %lu ms",
                    ws, (unsigned long)remainingDelay));
               ws->mConnecting = CONNECTING_DELAYED;
-              return false;
+              return;
             }
           }
           // if timer fails (which is very unlikely), drop down to BeginOpen call
@@ -257,11 +257,7 @@ public:
 
     // Delays disabled, or no previous failure, or we're reconnecting after scheduled
     // delay interval has passed: connect.
-    //
-    ws->mConnecting = CONNECTING_IN_PROGRESS;
-    // If BeginOpen fails, it calls AbortSession, which calls OnStopSession,
-    // which will ensure any remaining queued connection(s) are scheduled
-    return ws->BeginOpen();
+    ws->BeginOpen();
   }
 
   // Remove() also deletes all expired entries as it iterates: better for
@@ -324,7 +320,7 @@ public:
 
   // Determine if we will open connection immediately (returns true), or
   // delay/queue the connection (returns false)
-  bool ConditionallyConnect(WebSocketChannel *ws)
+  void ConditionallyConnect(WebSocketChannel *ws)
   {
     NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread");
     NS_ABORT_IF_FALSE(ws->mConnecting == NOT_CONNECTING, "opening state");
@@ -339,13 +335,12 @@ public:
 
     if (found) {
       ws->mConnecting = CONNECTING_QUEUED;
-      return false;
+    } else {
+      mFailures.DelayOrBegin(ws);
     }
-
-    return mFailures.DelayOrBegin(ws);
   }
 
-  bool OnConnected(WebSocketChannel *aChannel)
+  void OnConnected(WebSocketChannel *aChannel)
   {
     NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread");
     NS_ABORT_IF_FALSE(aChannel->mConnecting == CONNECTING_IN_PROGRESS,
@@ -362,12 +357,12 @@ public:
     // Check for queued connections to same host.
     // Note: still need to check for failures, since next websocket with same
     // host may have different port
-    return ConnectNext(aChannel->mAddress);
+    ConnectNext(aChannel->mAddress);
   }
 
   // Called every time a websocket channel ends its session (including going away
   // w/o ever successfully creating a connection)
-  bool OnStopSession(WebSocketChannel *aChannel, nsresult aReason)
+  void OnStopSession(WebSocketChannel *aChannel, nsresult aReason)
   {
     NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread");
 
@@ -397,13 +392,13 @@ public:
 
       bool wasNotQueued = (aChannel->mConnecting != CONNECTING_QUEUED);
       aChannel->mConnecting = NOT_CONNECTING;
-      if (wasNotQueued)
-        return ConnectNext(aChannel->mAddress);
+      if (wasNotQueued) {
+        ConnectNext(aChannel->mAddress);
+      }
     }
-    return false;
   }
 
-  bool ConnectNext(nsCString &hostName)
+  void ConnectNext(nsCString &hostName)
   {
     PRInt32 index = IndexOf(hostName);
     if (index >= 0) {
@@ -413,10 +408,8 @@ public:
                         "transaction not queued but in queue");
       LOG(("WebSocket: ConnectNext: found channel [this=%p] in queue", chan));
 
-      return mFailures.DelayOrBegin(chan);
+      mFailures.DelayOrBegin(chan);
     }
-
-    return false;
   }
 
   void IncrementSessionCount()
@@ -1020,7 +1013,7 @@ WebSocketChannel::Shutdown()
   sWebSocketAdmissions = nsnull;
 }
 
-bool
+void
 WebSocketChannel::BeginOpen()
 {
   LOG(("WebSocketChannel::BeginOpen() %p\n", this));
@@ -1028,25 +1021,30 @@ WebSocketChannel::BeginOpen()
 
   nsresult rv;
 
+  // Important that we set CONNECTING_IN_PROGRESS before any call to
+  // AbortSession here: ensures that any remaining queued connection(s) are
+  // scheduled in OnStopSession
+  mConnecting = CONNECTING_IN_PROGRESS;
+
   if (mRedirectCallback) {
     LOG(("WebSocketChannel::BeginOpen: Resuming Redirect\n"));
     rv = mRedirectCallback->OnRedirectVerifyCallback(NS_OK);
     mRedirectCallback = nsnull;
-    return false;
+    return;
   }
 
   nsCOMPtr<nsIChannel> localChannel = do_QueryInterface(mChannel, &rv);
   if (NS_FAILED(rv)) {
     LOG(("WebSocketChannel::BeginOpen: cannot async open\n"));
     AbortSession(NS_ERROR_UNEXPECTED);
-    return false;
+    return;
   }
 
   rv = localChannel->AsyncOpen(this, mHttpChannel);
   if (NS_FAILED(rv)) {
     LOG(("WebSocketChannel::BeginOpen: cannot async open\n"));
     AbortSession(NS_ERROR_CONNECTION_REFUSED);
-    return false;
+    return;
   }
   mChannelWasOpened = 1;
 
@@ -1054,7 +1052,7 @@ WebSocketChannel::BeginOpen()
   if (NS_FAILED(rv)) {
     LOG(("WebSocketChannel::BeginOpen: cannot create open timer\n"));
     AbortSession(NS_ERROR_UNEXPECTED);
-    return false;
+    return;
   }
 
   rv = mOpenTimer->InitWithCallback(this, mOpenTimeout,
@@ -1062,10 +1060,8 @@ WebSocketChannel::BeginOpen()
   if (NS_FAILED(rv)) {
     LOG(("WebSocketChannel::BeginOpen: cannot initialize open timer\n"));
     AbortSession(NS_ERROR_UNEXPECTED);
-    return false;
+    return;
   }
-
-  return true;
 }
 
 bool
@@ -2213,11 +2209,8 @@ WebSocketChannel::OnLookupComplete(nsICancelable *aRequest,
       LOG(("WebSocketChannel::OnLookupComplete: Failed GetNextAddr\n"));
   }
 
-  if (sWebSocketAdmissions->ConditionallyConnect(this)) {
-    LOG(("WebSocketChannel::OnLookupComplete: Proceeding with Open\n"));
-  } else {
-    LOG(("WebSocketChannel::OnLookupComplete: Deferring Open\n"));
-  }
+  LOG(("WebSocket OnLookupComplete: Proceeding to ConditionallyConnect\n"));
+  sWebSocketAdmissions->ConditionallyConnect(this);
 
   return NS_OK;
 }
@@ -2395,10 +2388,7 @@ WebSocketChannel::Notify(nsITimer *timer)
 
     mReconnectDelayTimer = nsnull;
     LOG(("WebSocketChannel: connecting [this=%p] after reconnect delay", this));
-    // - if BeginOpen fails, it calls AbortSession, which calls OnStopSession,
-    //   which will ensure any remaining queued connection(s) are scheduled
-    mConnecting = CONNECTING_IN_PROGRESS;
-    this->BeginOpen();
+    BeginOpen();
   } else if (timer == mPingTimer) {
     NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread,
                       "not socket thread");

netwerk/protocol/websocket/WebSocketChannel.h

@@ -126,7 +126,7 @@ private:
   void GeneratePong(PRUint8 *payload, PRUint32 len);
   void GeneratePing();
 
-  bool     BeginOpen();
+  void     BeginOpen();
   nsresult HandleExtensions();
   nsresult SetupRequest();
   nsresult ApplyForAdmission();