[Sync] Fix bug where DeviceInfo is missing a backup timestamp

Remove last_synced_time_ member variable from ProfileSyncService.  This
variable acted as a cache of the underlying pref.  However, it was not
properly cleared in ProfileSyncService::DisableForUser when the
associated pref was cleared.  This caused a bug where device info may
not have a backup timestamp.

BUG=427096

Review URL: https://codereview.chromium.org/660023003

Cr-Commit-Position: refs/heads/master@{#301481}

chrome/browser/sync/profile_sync_service.cc

@@ -311,8 +311,6 @@ void ProfileSyncService::Initialize() {
 
   TrySyncDatatypePrefRecovery();
 
-  last_synced_time_ = sync_prefs_.GetLastSyncedTime();
-
 #if defined(OS_CHROMEOS)
   std::string bootstrap_token = sync_prefs_.GetEncryptionBootstrapToken();
   if (bootstrap_token.empty()) {
@@ -904,8 +902,7 @@ void ProfileSyncService::SetSyncSetupCompleted() {
 }
 
 void ProfileSyncService::UpdateLastSyncedTime() {
-  last_synced_time_ = base::Time::Now();
-  sync_prefs_.SetLastSyncedTime(last_synced_time_);
+  sync_prefs_.SetLastSyncedTime(base::Time::Now());
 }
 
 void ProfileSyncService::NotifyObservers() {
@@ -1028,9 +1025,9 @@ void ProfileSyncService::PostBackendInitialization() {
   ConsumeCachedPassphraseIfPossible();
 
   // The very first time the backend initializes is effectively the first time
-  // we can say we successfully "synced".  last_synced_time_ will only be null
-  // in this case, because the pref wasn't restored on StartUp.
-  if (last_synced_time_.is_null()) {
+  // we can say we successfully "synced".  LastSyncedTime will only be null in
+  // this case, because the pref wasn't restored on StartUp.
+  if (sync_prefs_.GetLastSyncedTime().is_null()) {
     UpdateLastSyncedTime();
   }
 
@@ -1707,16 +1704,18 @@ bool ProfileSyncService::IsPassphraseRequiredForDecryption() const {
 }
 
 base::string16 ProfileSyncService::GetLastSyncedTimeString() const {
-  if (last_synced_time_.is_null())
+  const base::Time last_synced_time = sync_prefs_.GetLastSyncedTime();
+  if (last_synced_time.is_null())
     return l10n_util::GetStringUTF16(IDS_SYNC_TIME_NEVER);
 
-  base::TimeDelta last_synced = base::Time::Now() - last_synced_time_;
+  base::TimeDelta time_since_last_sync = base::Time::Now() - last_synced_time;
 
-  if (last_synced < base::TimeDelta::FromMinutes(1))
+  if (time_since_last_sync < base::TimeDelta::FromMinutes(1))
     return l10n_util::GetStringUTF16(IDS_SYNC_TIME_JUST_NOW);
 
   return ui::TimeFormat::Simple(ui::TimeFormat::FORMAT_ELAPSED,
-                                ui::TimeFormat::LENGTH_SHORT, last_synced);
+                                ui::TimeFormat::LENGTH_SHORT,
+                                time_since_last_sync);
 }
 
 void ProfileSyncService::UpdateSelectedTypesHistogram(
@@ -2673,10 +2672,11 @@ void ProfileSyncService::CheckSyncBackupIfNeeded() {
   DCHECK_EQ(backend_mode_, SYNC);
 
 #if defined(ENABLE_PRE_SYNC_BACKUP)
+  const base::Time last_synced_time = sync_prefs_.GetLastSyncedTime();
   // Check backup once a day.
   if (!last_backup_time_ &&
-      (last_synced_time_.is_null() ||
-          base::Time::Now() - last_synced_time_ >=
+      (last_synced_time.is_null() ||
+          base::Time::Now() - last_synced_time >=
               base::TimeDelta::FromDays(1))) {
     // If sync thread is set, need to serialize check on sync thread after
     // closing backup DB.

chrome/browser/sync/profile_sync_service.h

@@ -987,10 +987,6 @@ class ProfileSyncService : public ProfileSyncServiceBase,
   // This specifies where to find the sync server.
   const GURL sync_service_url_;
 
-  // The last time we detected a successful transition from SYNCING state.
-  // Our backend notifies us whenever we should take a new snapshot.
-  base::Time last_synced_time_;
-
   // The time that OnConfigureStart is called. This member is zero if
   // OnConfigureStart has not yet been called, and is reset to zero once
   // OnConfigureDone is called.

chrome/browser/sync/profile_sync_service_unittest.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/sync/supervised_user_signin_manager_wrapper.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
@@ -35,6 +36,7 @@
 #include "google_apis/gaia/gaia_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace content {
 class BrowserContext;
@@ -618,5 +620,24 @@ TEST_F(ProfileSyncServiceTest, GetSyncServiceURL) {
             ProfileSyncService::GetSyncServiceURL(command_line).spec());
 }
 
+// Verify that LastSyncedTime is cleared when the user signs out.
+TEST_F(ProfileSyncServiceTest, ClearLastSyncedTimeOnSignOut) {
+  IssueTestTokens();
+  CreateService(AUTO_START);
+  ExpectDataTypeManagerCreation(1);
+  ExpectSyncBackendHostCreation(1);
+  InitializeForNthSync();
+  EXPECT_TRUE(service()->SyncActive());
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SYNC_TIME_JUST_NOW),
+            service()->GetLastSyncedTimeString());
+
+  // Sign out.
+  service()->DisableForUser();
+  PumpLoop();
+
+  EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SYNC_TIME_NEVER),
+            service()->GetLastSyncedTimeString());
+}
+
 }  // namespace
 }  // namespace browser_sync