2nd attempt at http://codereview.chromium.org/3305003/show

The difference between this patch and the other one is plumbing setup_in_test_mode through the Initialization process, rather than using a separate SetupInTestMode() method, which happens too late to safely stop the syncer thread.  [see syncapi.cc|h for most of the changes.]

---

This patch removes: authenticator.cc, auth_watcher.cc

removes calls to user_settings.cc, removes an authenticate PB request to the server, and moves token storage into the Chrome TokenService. This patch introduces the SigninManager, which is an interim solution for user management prior to moving the system into chrome.

Other changes include removing the dependency on the sync backend to be running while the sync wizard is intially displayed. This means that the backend can be brought up in response to credentials becoming available. The backend now is always provided credentials on startup. If an auth error occurs, it propogates it up via a notification. Some event handlers were removed and streamlined for more straightforward sync system startup.

BUG=51001, 50293, 35158
TEST=Unit tests && Start up sync, log in, log out, run with expired credentials, run with new gaia credentials, run with gaia credentials updated while system is syncing. Try logging in with incorrect username. Trigger CAPTCHA. Try logging out and in repeatedly. Check about:sync works. Try going offline and back online again. Expire gaia credentials and try renewing it with the UI dialog.
Review URL: http://codereview.chromium.org/3342025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@58993 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 82 insertions, 71 deletions

diff --git a/chrome/test/live_sync/live_sync_test.cc b/chrome/test/live_sync/live_sync_test.cc
index 5191855..fcf1e47 100644
--- a/chrome/test/live_sync/live_sync_test.cc
+++ b/chrome/test/live_sync/live_sync_test.cc
@@ -22,6 +22,7 @@
 #include "chrome/test/testing_browser_process.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/escape.h"
+#include "net/base/network_change_notifier.h"
 #include "net/proxy/proxy_config.h"
 #include "net/proxy/proxy_config_service_fixed.h"
 #include "net/proxy/proxy_service.h"
@@ -233,6 +234,10 @@ void LiveSyncTest::SetUpInProcessBrowserTestFixture() {
   // Without this, running the test case on Linux causes an error as we make an
   // external DNS lookup of "ocsp.thawte.com".
   resolver->AllowDirectLookup("*.thawte.com");
+  // The new XMPP cert seems to use crl.geotrust.com on Linux.
+  resolver->AllowDirectLookup("*.geotrust.com");
+  // SSL chain.
+  resolver->AllowDirectLookup("*.gstatic.com");
   mock_host_resolver_override_.reset(
       new net::ScopedDefaultHostResolverProc(resolver));
 }
@@ -260,10 +265,12 @@ void LiveSyncTest::TearDownLocalTestServer() {
   ASSERT_TRUE(test_server_.Stop());
 }
 
-
 void LiveSyncTest::EnableNetwork(Profile* profile) {
   SetProxyConfig(profile->GetRequestContext(),
                  net::ProxyConfig::CreateDirect());
+  // Bugs: http://crbug.com/53857
+  //       http://crbug.com/53858
+  net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
 }
 
 void LiveSyncTest::DisableNetwork(Profile* profile) {
@@ -272,6 +279,7 @@ void LiveSyncTest::DisableNetwork(Profile* profile) {
   net::ProxyConfig config;
   config.proxy_rules().ParseFromString("http=127.0.0.1:0");
   SetProxyConfig(profile->GetRequestContext(), config);
+  net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
 }
 
 bool LiveSyncTest::EnsureSyncServerConfiguration() {
diff --git a/chrome/test/live_sync/offline_sync_test.cc b/chrome/test/live_sync/offline_sync_test.cc
deleted file mode 100644
index c64df34..0000000
--- a/chrome/test/live_sync/offline_sync_test.cc
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/browser/profile.h"
-#include "chrome/common/net/gaia/google_service_auth_error.h"
-#include "chrome/test/live_sync/live_sync_test.h"
-#include "chrome/test/live_sync/profile_sync_service_test_harness.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-class OfflineSyncTest : public LiveSyncTest {
- public:
-  OfflineSyncTest() : LiveSyncTest(LiveSyncTest::SINGLE_CLIENT) {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(OfflineSyncTest);
-};
-
-IN_PROC_BROWSER_TEST_F(OfflineSyncTest, OfflineStart) {
-  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
-  DisableNetwork(GetProfile(0));
-
-  EXPECT_FALSE(GetClient(0)->SetupSync());
-  EXPECT_EQ(GoogleServiceAuthError::CONNECTION_FAILED,
-            GetClient(0)->service()->GetAuthError().state());
-
-  EnableNetwork(GetProfile(0));
-  EXPECT_TRUE(GetClient(0)->RetryAuthentication());
-}
diff --git a/chrome/test/live_sync/profile_sync_service_test_harness.cc b/chrome/test/live_sync/profile_sync_service_test_harness.cc
index 12448da..b49bf56 100644
--- a/chrome/test/live_sync/profile_sync_service_test_harness.cc
+++ b/chrome/test/live_sync/profile_sync_service_test_harness.cc
@@ -7,9 +7,11 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/net/gaia/token_service.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/sessions/session_state.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/net/gaia/gaia_constants.h"
 #include "chrome/common/net/gaia/google_service_auth_error.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/live_sync/profile_sync_service_test_harness.h"
@@ -84,7 +86,8 @@ bool StateChangeTimeoutEvent::Abort() {
 
 ProfileSyncServiceTestHarness::ProfileSyncServiceTestHarness(Profile* p,
     const std::string& username, const std::string& password, int id)
-    : wait_state_(WAITING_FOR_ON_AUTH_ERROR), profile_(p), service_(NULL),
+    : wait_state_(WAITING_FOR_ON_BACKEND_INITIALIZED),
+      profile_(p), service_(NULL),
       last_status_(kInvalidStatus),
       last_timestamp_(0),
       min_timestamp_needed_(kMinTimestampNeededNone),
@@ -95,15 +98,11 @@ ProfileSyncServiceTestHarness::ProfileSyncServiceTestHarness(Profile* p,
 }
 
 bool ProfileSyncServiceTestHarness::SetupSync() {
-  service_ = profile_->GetProfileSyncService();
-  service_->StartUp();
+  service_ = profile_->GetProfileSyncService("");
   service_->AddObserver(this);
-  return WaitForServiceInit(false);
-}
+  service_->signin_.StartSignIn(username_, password_, "", "");
 
-bool ProfileSyncServiceTestHarness::RetryAuthentication() {
-  wait_state_ = WAITING_FOR_ON_BACKEND_INITIALIZED;
-  return WaitForServiceInit(true);
+  return WaitForServiceInit();
 }
 
 void ProfileSyncServiceTestHarness::SignalStateCompleteWithNextState(
@@ -121,11 +120,6 @@ bool ProfileSyncServiceTestHarness::RunStateChangeMachine() {
   WaitState state = wait_state_;
   ProfileSyncService::Status status(service_->QueryDetailedSyncStatus());
   switch (wait_state_) {
-    case WAITING_FOR_ON_AUTH_ERROR: {
-      LogClientInfo("WAITING_FOR_ON_AUTH_ERROR");
-      SignalStateCompleteWithNextState(WAITING_FOR_ON_BACKEND_INITIALIZED);
-      break;
-    }
     case WAITING_FOR_ON_BACKEND_INITIALIZED: {
       LogClientInfo("WAITING_FOR_ON_BACKEND_INITIALIZED");
       if (service_->GetAuthError().state() != GoogleServiceAuthError::NONE) {
@@ -143,6 +137,21 @@ bool ProfileSyncServiceTestHarness::RunStateChangeMachine() {
       }
       break;
     }
+    case WAITING_FOR_SERVER_REACHABLE: {
+      LogClientInfo("WAITING_FOR_SERVER_REACHABLE");
+      const SyncSessionSnapshot* snap = GetLastSessionSnapshot();
+      if (!status.server_reachable) {
+        break;
+      }
+      if (service()->backend()->HasUnsyncedItems() ||
+          snap->has_more_to_sync || snap->unsynced_count != 0) {
+        SignalStateCompleteWithNextState(WAITING_FOR_SYNC_TO_FINISH);
+        break;
+      }
+      last_timestamp_ = snap->max_local_timestamp;
+      SignalStateCompleteWithNextState(FULLY_SYNCED);
+      break;
+    }
     case WAITING_FOR_SYNC_TO_FINISH: {
       LogClientInfo("WAITING_FOR_SYNC_TO_FINISH");
       const SyncSessionSnapshot* snap = GetLastSessionSnapshot();
@@ -151,7 +160,10 @@ bool ProfileSyncServiceTestHarness::RunStateChangeMachine() {
       // be a sufficient condition for sync to have completed. However, the
       // additional check of snap->unsynced_count is required due to
       // http://crbug.com/48989.
-      if (snap->has_more_to_sync || snap->unsynced_count != 0) {
+      if (service()->backend()->HasUnsyncedItems() ||
+          snap->has_more_to_sync || snap->unsynced_count != 0) {
+        if (!status.server_reachable)
+          SignalStateCompleteWithNextState(WAITING_FOR_SERVER_REACHABLE);
         break;
       }
       EXPECT_LE(last_timestamp_, snap->max_local_timestamp);
@@ -286,18 +298,10 @@ bool ProfileSyncServiceTestHarness::AwaitStatusChangeWithTimeout(
   return timeout_signal->Abort();
 }
 
-bool ProfileSyncServiceTestHarness::WaitForServiceInit(bool is_auth_retry) {
+bool ProfileSyncServiceTestHarness::WaitForServiceInit() {
   LogClientInfo("WaitForServiceInit");
-  if (!is_auth_retry) {
-    // Wait for the OnAuthError() callback.
-    EXPECT_EQ(wait_state_, WAITING_FOR_ON_AUTH_ERROR);
-    EXPECT_TRUE(AwaitStatusChangeWithTimeout(30,
-        "Waiting for the OnAuthError() callback.")) <<
-        "OnAuthError() not seen after 30 seconds.";
-  }
 
-  // Enter GAIA credentials and wait for the OnBackendInitialized() callback.
-  service_->backend()->Authenticate(username_, password_, std::string());
+  // Wait for the OnBackendInitialized() callback.
   EXPECT_EQ(wait_state_, WAITING_FOR_ON_BACKEND_INITIALIZED);
   EXPECT_TRUE(AwaitStatusChangeWithTimeout(30,
       "Waiting for OnBackendInitialized().")) <<
@@ -307,15 +311,13 @@ bool ProfileSyncServiceTestHarness::WaitForServiceInit(bool is_auth_retry) {
     return false;
   }
 
-  // Choose datatypes to be synced. Note: This is unnecessary on Chrome OS.
-  if (!browser_defaults::kBootstrapSyncAuthentication) {
-    syncable::ModelTypeSet set;
-    for (int i = syncable::FIRST_REAL_MODEL_TYPE;
-        i < syncable::MODEL_TYPE_COUNT; ++i) {
-      set.insert(syncable::ModelTypeFromInt(i));
-    }
-    service_->OnUserChoseDatatypes(true, set);
+  // Choose datatypes to be synced.
+  syncable::ModelTypeSet set;
+  for (int i = syncable::FIRST_REAL_MODEL_TYPE;
+      i < syncable::MODEL_TYPE_COUNT; ++i) {
+    set.insert(syncable::ModelTypeFromInt(i));
   }
+  service_->OnUserChoseDatatypes(true, set);
 
   // Wait for notifications_enabled to be set to true.
   EXPECT_EQ(wait_state_, WAITING_FOR_NOTIFICATIONS_ENABLED);
@@ -329,7 +331,10 @@ bool ProfileSyncServiceTestHarness::WaitForServiceInit(bool is_auth_retry) {
 const SyncSessionSnapshot*
     ProfileSyncServiceTestHarness::GetLastSessionSnapshot() const {
   EXPECT_FALSE(service_ == NULL) << "Sync service has not yet been set up.";
-  return service_->backend()->GetLastSessionSnapshot();
+  if (service_->backend()) {
+    return service_->backend()->GetLastSessionSnapshot();
+  }
+  return NULL;
 }
 
 void ProfileSyncServiceTestHarness::LogClientInfo(std::string message) {
diff --git a/chrome/test/live_sync/profile_sync_service_test_harness.h b/chrome/test/live_sync/profile_sync_service_test_harness.h
index 3b32930..34cfde3 100644
--- a/chrome/test/live_sync/profile_sync_service_test_harness.h
+++ b/chrome/test/live_sync/profile_sync_service_test_harness.h
@@ -31,10 +31,6 @@ class ProfileSyncServiceTestHarness : public ProfileSyncServiceObserver {
   // authenticated, and we are ready to process changes.
   bool SetupSync();
 
-  // Retries a sync setup when the previous attempt was aborted by an
-  // authentication failure.
-  bool RetryAuthentication();
-
   // ProfileSyncServiceObserver implementation.
   virtual void OnStateChanged();
 
@@ -79,16 +75,16 @@ class ProfileSyncServiceTestHarness : public ProfileSyncServiceObserver {
   friend class StateChangeTimeoutEvent;
 
   enum WaitState {
-    // The sync client awaits the OnAuthError() callback.
-    WAITING_FOR_ON_AUTH_ERROR = 0,
     // The sync client awaits the OnBackendInitialized() callback.
-    WAITING_FOR_ON_BACKEND_INITIALIZED,
+    WAITING_FOR_ON_BACKEND_INITIALIZED = 0,
     // The sync client is waiting for notifications_enabled to become true.
     WAITING_FOR_NOTIFICATIONS_ENABLED,
     // The sync client is waiting for an ongoing sync cycle to complete.
     WAITING_FOR_SYNC_TO_FINISH,
     // The sync client anticipates incoming updates leading to a new sync cycle.
     WAITING_FOR_UPDATES,
+    // The sync client is waiting for server_reachable to become true.
+    WAITING_FOR_SERVER_REACHABLE,
     // The sync client is fully synced and there are no pending updates.
     FULLY_SYNCED,
     // An authentication error has occurred.
@@ -110,7 +106,7 @@ class ProfileSyncServiceTestHarness : public ProfileSyncServiceObserver {
 
   // Returns true if the service initialized correctly. Set is_auth_retry to
   // true when calling this method second time after an authentication failure.
-  bool WaitForServiceInit(bool is_auth_retry);
+  bool WaitForServiceInit();
 
   // Logs message with relevant info about client's sync state (if available).
   void LogClientInfo(std::string message);
diff --git a/chrome/test/live_sync/single_client_live_bookmarks_sync_test.cc b/chrome/test/live_sync/single_client_live_bookmarks_sync_test.cc
index c4cc585..bc66917 100644
--- a/chrome/test/live_sync/single_client_live_bookmarks_sync_test.cc
+++ b/chrome/test/live_sync/single_client_live_bookmarks_sync_test.cc
@@ -4,6 +4,37 @@
 
 #include "chrome/test/live_sync/live_bookmarks_sync_test.h"
 
+// TODO(rsimha): Debug the flakiness of this with fred. http://crbug.com/53858
+IN_PROC_BROWSER_TEST_F(SingleClientLiveBookmarksSyncTest,
+                       FAILS_OfflineToOnline) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+
+  DisableNetwork(GetProfile(0));
+
+  BookmarkModel* bm = GetBookmarkModel(0);
+  BookmarkModelVerifier* v = verifier_helper();
+
+  const BookmarkNode* top = v->AddGroup(bm, bm->other_node(), 0, L"top");
+  v->SetTitle(bm, top, L"title");
+
+
+  EXPECT_TRUE(GetClient(0)->AwaitSyncCycleCompletion("Offline state change."));
+  {
+    ProfileSyncService::Status status(
+        GetClient(0)->service()->QueryDetailedSyncStatus());
+    EXPECT_EQ(status.summary, ProfileSyncService::Status::OFFLINE_UNSYNCED);
+  }
+
+  EnableNetwork(GetProfile(0));
+  {
+    EXPECT_TRUE(GetClient(0)->AwaitSyncCycleCompletion("Commit changes."));
+    ProfileSyncService::Status status(
+        GetClient(0)->service()->QueryDetailedSyncStatus());
+    EXPECT_EQ(status.summary, ProfileSyncService::Status::READY);
+    v->ExpectMatch(bm);
+  }
+}
+
 IN_PROC_BROWSER_TEST_F(SingleClientLiveBookmarksSyncTest, Sanity) {
   ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
   BookmarkModel* bm = GetBookmarkModel(0);