Fix one case that can make sync backend stuck in auth error state.

sync/engine/net/server_connection_manager.cc

Index: sync/engine/net/server_connection_manager.cc
diff --git a/sync/engine/net/server_connection_manager.cc b/sync/engine/net/server_connection_manager.cc
index 4927acdaa75630bca9ed3fbee6c25060aa1553f5..a6ee157690d3a1dda92fa10de94352a29140159d 100644
--- a/sync/engine/net/server_connection_manager.cc
+++ b/sync/engine/net/server_connection_manager.cc
@@ -233,6 +233,12 @@ bool ServerConnectionManager::SetAuthToken(const std::string& auth_token) {
     previously_invalidated_token = std::string();
     return true;
   }
+
+  // This could happen in case like server outage/bug. Need to notify sync

[tim (not reviewing), 2013/11/06 19:35:32]

What is server outage/bug?  Either explain the outage, reference the bug#, or
both.

[haitaol1, 2013/11/06 22:37:30]

Done.

+  // frontend again to request new token, otherwise backend will stay in
+  // SYNC_AUTH_ERROR state while fronend thinks everything is fine and takes
+  // no actions.
+  SetServerStatus(HttpResponse::SYNC_AUTH_ERROR);

[tim (not reviewing), 2013/11/06 19:35:32]

I guess the only case that is implicitly in the "else" clause of the above if is
if we call SetAuthToken with an empty token; now we'll always send up a
SYNC_AUTH_ERROR if we did that.  I distinctly recall us changing this a while
back to not always bubble up the error, although it may have been because we
were trying to avoid UI updates. Are you sure that has no implications?

Does your approach actually require tracking previously_invalidated_token?

[haitaol1, 2013/11/06 22:37:30]

Empty token shouldn't be possible because ProfileSyncService::GetCredentials()
overwrites empty token with "credentials_lost".
Without previous token, it takes one trip to sync server to find out if token is
valid or not.

   return false;
 }
 
@@ -252,8 +258,12 @@ void ServerConnectionManager::InvalidateAndClearAuthToken() {
 
 void ServerConnectionManager::SetServerStatus(
     HttpResponse::ServerConnectionCode server_status) {
-  if (server_status_ == server_status)
+  // SYNC_AUTH_ERROR is permanent error. Need to notify observer to take
+  // action externally to resolve.
+  if (server_status != HttpResponse::SYNC_AUTH_ERROR &&
+      server_status_ == server_status) {

[tim (not reviewing), 2013/11/06 19:35:32]

Pavel and Richard should review this part carefully, we've had several bugs in
this area that they have diagnosed.

[haitaol1, 2013/11/06 22:37:30]

acked

[haitaol1, 2013/11/07 15:23:52]

Another option is to leverage CONNECTION_NOT_ATTEMPTED added in 60703002. So
SetAuthToken() always resets status to CONNECTION_NOT_ATTEMPTED at beginning,
then exception can be removed here.

On 2013/11/06 22:37:30, haitaol1 wrote:

     return;
+  }
   server_status_ = server_status;
   NotifyStatusChanged();
 }