Sync: Add DeviceInfo's ChangeProcessor

This is the long-awaited change to enable DeviceInfo tracking.

Long ago (r161496) we committed code to support the DeviceInfo type, but
we had to leave this code disabled until the server was ready to support
the new type.  That support has now been added.

This change includes a very special kind of ChangeProcessor named
SyncedDeviceTracker.  It is the only ChangeProcessor that is owned by
the sync thread.  The SyncBackendHost creates and initializes it during
backend intialization and deletes it before the UserShare it references
is destroyed.  As part of its initialization, the SyncedDeviceTracker
will update the DeviceInfo entry for the current device.

The SyncedDeviceTracker could support sending notifications to any
interested listeners when new or update DeviceInfo information arrives,
but this functionality has not been implemented yet.

Also included are:
- Tests for the SyncedDeviceTracker
- A MockTransactionObserver to support these tests
- A cache_guid() accessor on the SyncManager, used to initialize the
  SyncedDeviceTracker
- Remove support for default constructing a DeviceInfo object

BUG=122825


Review URL: https://chromiumcodereview.appspot.com/11360259

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

16 files changed, 162 insertions, 32 deletions

diff --git a/sync/internal_api/base_transaction.cc b/sync/internal_api/base_transaction.cc
index 11a8ca4..cbfb246 100644
--- a/sync/internal_api/base_transaction.cc
+++ b/sync/internal_api/base_transaction.cc
@@ -12,19 +12,20 @@ namespace syncer {
 
 //////////////////////////////////////////////////////////////////////////
 // BaseTransaction member definitions
-BaseTransaction::BaseTransaction(UserShare* share) {
+BaseTransaction::BaseTransaction(UserShare* share)
+    : user_share_(share) {
   DCHECK(share && share->directory.get());
-  directory_ = share->directory.get();
 }
+
 BaseTransaction::~BaseTransaction() {
 }
 
 Cryptographer* BaseTransaction::GetCryptographer() const {
-  return directory_->GetCryptographer(this->GetWrappedTrans());
+  return GetDirectory()->GetCryptographer(this->GetWrappedTrans());
 }
 
 ModelTypeSet BaseTransaction::GetEncryptedTypes() const {
-  return directory_->GetNigoriHandler()->GetEncryptedTypes(
+  return GetDirectory()->GetNigoriHandler()->GetEncryptedTypes(
       this->GetWrappedTrans());
 }
 
diff --git a/sync/internal_api/public/base_transaction.h b/sync/internal_api/public/base_transaction.h
index 242a6b9..7a2ba37 100644
--- a/sync/internal_api/public/base_transaction.h
+++ b/sync/internal_api/public/base_transaction.h
@@ -32,17 +32,25 @@ class BaseTransaction {
   ModelTypeSet GetEncryptedTypes() const;
 
   syncable::Directory* GetDirectory() const {
-    return directory_;
+    if (!user_share_) {
+      return NULL;
+    } else {
+      return user_share_->directory.get();
+    }
+  }
+
+  UserShare* GetUserShare() const {
+    return user_share_;
   }
 
  protected:
   explicit BaseTransaction(UserShare* share);
   virtual ~BaseTransaction();
 
-  BaseTransaction() : directory_(NULL) { }
+  BaseTransaction() : user_share_(NULL) { }
 
  private:
-  syncable::Directory* directory_;
+  UserShare* user_share_;
 
   DISALLOW_COPY_AND_ASSIGN(BaseTransaction);
 };
diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h
index 6ed9eab..b26bd4f 100644
--- a/sync/internal_api/public/sync_manager.h
+++ b/sync/internal_api/public/sync_manager.h
@@ -285,8 +285,6 @@ class SyncManager {
   // |sync_server_and_path| and |sync_server_port| represent the Chrome sync
   // server to use, and |use_ssl| specifies whether to communicate securely;
   // the default is false.
-  // |blocking_task_runner| is a TaskRunner to be used for tasks that
-  // may block on disk I/O.
   // |post_factory| will be owned internally and used to create
   // instances of an HttpPostProvider.
   // |model_safe_worker| ownership is given to the SyncManager.
@@ -307,7 +305,6 @@ class SyncManager {
       const std::string& sync_server_and_path,
       int sync_server_port,
       bool use_ssl,
-      const scoped_refptr<base::TaskRunner>& blocking_task_runner,
       scoped_ptr<HttpPostProviderFactory> post_factory,
       const std::vector<ModelSafeWorker*>& workers,
       ExtensionsActivityMonitor* extensions_activity_monitor,
@@ -409,6 +406,10 @@ class SyncManager {
   // May be called from any thread.
   virtual UserShare* GetUserShare() = 0;
 
+  // Returns the cache_guid of the currently open database.
+  // Requires that the SyncManager be initialized.
+  virtual const std::string cache_guid() = 0;
+
   // Reads the nigori node to determine if any experimental features should
   // be enabled.
   // Note: opens a transaction.  May be called on any thread.
diff --git a/sync/internal_api/public/test/fake_sync_manager.h b/sync/internal_api/public/test/fake_sync_manager.h
index 826753d..88e6360 100644
--- a/sync/internal_api/public/test/fake_sync_manager.h
+++ b/sync/internal_api/public/test/fake_sync_manager.h
@@ -10,6 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
 #include "sync/internal_api/public/sync_manager.h"
+#include "sync/internal_api/public/test/test_user_share.h"
 #include "sync/notifier/invalidator_registrar.h"
 
 namespace base {
@@ -72,7 +73,6 @@ class FakeSyncManager : public SyncManager {
       const std::string& sync_server_and_path,
       int sync_server_port,
       bool use_ssl,
-      const scoped_refptr<base::TaskRunner>& blocking_task_runner,
       scoped_ptr<HttpPostProviderFactory> post_factory,
       const std::vector<ModelSafeWorker*>& workers,
       ExtensionsActivityMonitor* extensions_activity_monitor,
@@ -115,6 +115,7 @@ class FakeSyncManager : public SyncManager {
   virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;
   virtual UserShare* GetUserShare() OVERRIDE;
+  virtual const std::string cache_guid() OVERRIDE;
   virtual bool ReceivedExperiment(Experiments* experiments) OVERRIDE;
   virtual bool HasUnsyncedItems() OVERRIDE;
   virtual SyncEncryptionHandler* GetEncryptionHandler() OVERRIDE;
@@ -149,6 +150,8 @@ class FakeSyncManager : public SyncManager {
 
   scoped_ptr<FakeSyncEncryptionHandler> fake_encryption_handler_;
 
+  TestUserShare test_user_share_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeSyncManager);
 };
 
diff --git a/sync/internal_api/public/test/test_user_share.h b/sync/internal_api/public/test/test_user_share.h
index 507a2ed..6588945 100644
--- a/sync/internal_api/public/test/test_user_share.h
+++ b/sync/internal_api/public/test/test_user_share.h
@@ -39,6 +39,10 @@ namespace syncer {
 class SyncEncryptionHandler;
 class TestDirectorySetterUpper;
 
+namespace syncable {
+  class TestTransactionObserver;
+}
+
 class TestUserShare {
  public:
   TestUserShare();
@@ -61,6 +65,10 @@ class TestUserShare {
   // methods normally handled via the SyncBackendHost
   SyncEncryptionHandler* encryption_handler();
 
+  // Returns the directory's transaction observer.  This transaction observer
+  // has methods which can be helpful when writing test assertions.
+  syncable::TestTransactionObserver* transaction_observer();
+
   // A helper function to pretend to download this type's root node.
   static bool CreateRoot(syncer::ModelType model_type,
                          syncer::UserShare* service);
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index bd528f7..dbb8839 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -47,7 +47,6 @@
 #include "sync/syncable/entry.h"
 #include "sync/syncable/in_memory_directory_backing_store.h"
 #include "sync/syncable/on_disk_directory_backing_store.h"
-#include "sync/util/get_session_name.h"
 
 using base::TimeDelta;
 using sync_pb::GetUpdatesCallerInfo;
@@ -335,7 +334,6 @@ void SyncManagerImpl::Init(
     const std::string& sync_server_and_path,
     int port,
     bool use_ssl,
-    const scoped_refptr<base::TaskRunner>& blocking_task_runner,
     scoped_ptr<HttpPostProviderFactory> post_factory,
     const std::vector<ModelSafeWorker*>& workers,
     ExtensionsActivityMonitor* extensions_activity_monitor,
@@ -357,8 +355,6 @@ void SyncManagerImpl::Init(
 
   weak_handle_this_ = MakeWeakHandle(weak_ptr_factory_.GetWeakPtr());
 
-  blocking_task_runner_ = blocking_task_runner;
-
   change_delegate_ = change_delegate;
 
   invalidator_ = invalidator.Pass();
@@ -1296,6 +1292,11 @@ UserShare* SyncManagerImpl::GetUserShare() {
   return &share_;
 }
 
+const std::string SyncManagerImpl::cache_guid() {
+  DCHECK(initialized_);
+  return directory()->cache_guid();
+}
+
 bool SyncManagerImpl::ReceivedExperiment(Experiments* experiments) {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   ReadNode nigori_node(&trans);
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index 6accdab..de242be 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -68,7 +68,6 @@ class SyncManagerImpl :
       const std::string& sync_server_and_path,
       int sync_server_port,
       bool use_ssl,
-      const scoped_refptr<base::TaskRunner>& blocking_task_runner,
       scoped_ptr<HttpPostProviderFactory> post_factory,
       const std::vector<ModelSafeWorker*>& workers,
       ExtensionsActivityMonitor* extensions_activity_monitor,
@@ -111,6 +110,7 @@ class SyncManagerImpl :
   virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;
   virtual UserShare* GetUserShare() OVERRIDE;
+  virtual const std::string cache_guid() OVERRIDE;
   virtual bool ReceivedExperiment(Experiments* experiments) OVERRIDE;
   virtual bool HasUnsyncedItems() OVERRIDE;
   virtual SyncEncryptionHandler* GetEncryptionHandler() OVERRIDE;
@@ -302,10 +302,6 @@ class SyncManagerImpl :
   // WeakHandle when we construct it.
   WeakHandle<SyncManagerImpl> weak_handle_this_;
 
-  // |blocking_task_runner| is a TaskRunner to be used for tasks that
-  // may block on disk I/O.
-  scoped_refptr<base::TaskRunner> blocking_task_runner_;
-
   // We give a handle to share_ to clients of the API for use when constructing
   // any transaction type.
   UserShare share_;
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 13a0556..e413221 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -830,7 +830,6 @@ class SyncManagerTest : public testing::Test,
     sync_manager_.Init(temp_dir_.path(),
                        WeakHandle<JsEventHandler>(),
                        "bogus", 0, false,
-                       base::MessageLoopProxy::current(),
                        scoped_ptr<HttpPostProviderFactory>(
                            new TestHttpPostProviderFactory()),
                        workers, &extensions_activity_monitor_, this,
diff --git a/sync/internal_api/test/fake_sync_manager.cc b/sync/internal_api/test/fake_sync_manager.cc
index 66b3ac6..0556cc9 100644
--- a/sync/internal_api/test/fake_sync_manager.cc
+++ b/sync/internal_api/test/fake_sync_manager.cc
@@ -20,6 +20,7 @@
 #include "sync/notifier/invalidator.h"
 #include "sync/notifier/invalidator_state.h"
 #include "sync/notifier/object_id_invalidation_map.h"
+#include "sync/syncable/directory.h"
 #include "sync/test/fake_sync_encryption_handler.h"
 
 namespace syncer {
@@ -91,7 +92,6 @@ void FakeSyncManager::Init(
     const std::string& sync_server_and_path,
     int sync_server_port,
     bool use_ssl,
-    const scoped_refptr<base::TaskRunner>& blocking_task_runner,
     scoped_ptr<HttpPostProviderFactory> post_factory,
     const std::vector<ModelSafeWorker*>& workers,
     ExtensionsActivityMonitor* extensions_activity_monitor,
@@ -107,6 +107,14 @@ void FakeSyncManager::Init(
         report_unrecoverable_error_function) {
   sync_task_runner_ = base::ThreadTaskRunnerHandle::Get();
   PurgePartiallySyncedTypes();
+
+  test_user_share_.SetUp();
+  UserShare* share = test_user_share_.user_share();
+  for (ModelTypeSet::Iterator it = initial_sync_ended_types_.First();
+       it.Good(); it.Inc()) {
+    TestUserShare::CreateRoot(it.Get(), share);
+  }
+
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                     OnInitializationComplete(
                         WeakHandle<JsBackend>(),
@@ -186,6 +194,16 @@ void FakeSyncManager::ConfigureSyncer(
            << ModelTypeSetToString(success_types) << ". Cleaning: "
            << ModelTypeSetToString(disabled_types);
 
+  // Update our fake directory by clearing and fake-downloading as necessary.
+  UserShare* share = GetUserShare();
+  share->directory->PurgeEntriesWithTypeIn(disabled_types);
+  for (ModelTypeSet::Iterator it = success_types.First(); it.Good(); it.Inc()) {
+    // We must be careful to not create the same root node twice.
+    if (!initial_sync_ended_types_.Has(it.Get())) {
+      TestUserShare::CreateRoot(it.Get(), share);
+    }
+  }
+
   // Simulate cleaning up disabled types.
   // TODO(sync): consider only cleaning those types that were recently disabled,
   // if this isn't the first cleanup, which more accurately reflects the
@@ -228,10 +246,15 @@ void FakeSyncManager::StopSyncingForShutdown(const base::Closure& callback) {
 
 void FakeSyncManager::ShutdownOnSyncThread() {
   DCHECK(sync_task_runner_->RunsTasksOnCurrentThread());
+  test_user_share_.TearDown();
 }
 
 UserShare* FakeSyncManager::GetUserShare() {
-  return NULL;
+  return test_user_share_.user_share();
+}
+
+const std::string FakeSyncManager::cache_guid() {
+  return test_user_share_.user_share()->directory->cache_guid();
 }
 
 bool FakeSyncManager::ReceivedExperiment(Experiments* experiments) {
diff --git a/sync/internal_api/test/test_user_share.cc b/sync/internal_api/test/test_user_share.cc
index 1533d63..5c7856e 100644
--- a/sync/internal_api/test/test_user_share.cc
+++ b/sync/internal_api/test/test_user_share.cc
@@ -46,6 +46,10 @@ SyncEncryptionHandler* TestUserShare::encryption_handler() {
   return dir_maker_->encryption_handler();
 }
 
+syncable::TestTransactionObserver* TestUserShare::transaction_observer() {
+  return dir_maker_->transaction_observer();
+}
+
 /* static */
 bool TestUserShare::CreateRoot(ModelType model_type, UserShare* user_share) {
   syncer::syncable::Directory* directory = user_share->directory.get();
diff --git a/sync/sync.gyp b/sync/sync.gyp
index de8e734..ad9a684 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -139,8 +139,8 @@
         'syncable/model_type.cc',
         'syncable/mutable_entry.cc',
         'syncable/mutable_entry.h',
-        'syncable/nigori_handler.h',
         'syncable/nigori_handler.cc',
+        'syncable/nigori_handler.h',
         'syncable/nigori_util.cc',
         'syncable/nigori_util.h',
         'syncable/on_disk_directory_backing_store.cc',
@@ -498,6 +498,8 @@
         'test/fake_sync_encryption_handler.cc',
         'test/fake_extensions_activity_monitor.cc',
         'test/fake_extensions_activity_monitor.h',
+        'test/test_transaction_observer.cc',
+        'test/test_transaction_observer.h',
         'test/null_directory_change_delegate.cc',
         'test/null_directory_change_delegate.h',
         'test/null_transaction_observer.cc',
diff --git a/sync/test/engine/test_directory_setter_upper.cc b/sync/test/engine/test_directory_setter_upper.cc
index 9225e57..e302299 100644
--- a/sync/test/engine/test_directory_setter_upper.cc
+++ b/sync/test/engine/test_directory_setter_upper.cc
@@ -10,19 +10,23 @@
 #include "base/string_util.h"
 #include "sync/syncable/directory.h"
 #include "sync/syncable/in_memory_directory_backing_store.h"
+#include "sync/syncable/mutable_entry.h"
 #include "sync/syncable/read_transaction.h"
-#include "sync/test/null_transaction_observer.h"
+#include "sync/syncable/write_transaction.h"
+#include "sync/test/test_transaction_observer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
 
-using syncable::NullTransactionObserver;
-
 TestDirectorySetterUpper::TestDirectorySetterUpper() : name_("Test") {}
 
 TestDirectorySetterUpper::~TestDirectorySetterUpper() {}
 
 void TestDirectorySetterUpper::SetUp() {
+  test_transaction_observer_.reset(new syncable::TestTransactionObserver());
+  WeakHandle<syncable::TransactionObserver> transaction_observer =
+      MakeWeakHandle(test_transaction_observer_->AsWeakPtr());
+
   directory_.reset(new syncable::Directory(
       new syncable::InMemoryDirectoryBackingStore(name_),
       &handler_,
@@ -30,7 +34,7 @@ void TestDirectorySetterUpper::SetUp() {
       &encryption_handler_,
       encryption_handler_.cryptographer()));
   ASSERT_EQ(syncable::OPENED, directory_->Open(
-          name_, &delegate_, NullTransactionObserver()));
+          name_, &delegate_, transaction_observer));
 }
 
 void TestDirectorySetterUpper::TearDown() {
diff --git a/sync/test/engine/test_directory_setter_upper.h b/sync/test/engine/test_directory_setter_upper.h
index a8c5e5e..24372b8 100644
--- a/sync/test/engine/test_directory_setter_upper.h
+++ b/sync/test/engine/test_directory_setter_upper.h
@@ -39,12 +39,13 @@
 #include "sync/util/test_unrecoverable_error_handler.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+namespace syncer {
+
 namespace syncable {
   class Directory;
+  class TestTransactionObserver;
 }
 
-namespace syncer {
-
 class TestDirectorySetterUpper {
  public:
   TestDirectorySetterUpper();
@@ -62,8 +63,13 @@ class TestDirectorySetterUpper {
 
   SyncEncryptionHandler* encryption_handler() { return &encryption_handler_; }
 
+  syncable::TestTransactionObserver* transaction_observer() {
+    return test_transaction_observer_.get();
+  }
+
  private:
   syncable::NullDirectoryChangeDelegate delegate_;
+  scoped_ptr<syncable::TestTransactionObserver> test_transaction_observer_;
   TestUnrecoverableErrorHandler handler_;
 
   void RunInvariantCheck();
diff --git a/sync/test/test_transaction_observer.cc b/sync/test/test_transaction_observer.cc
new file mode 100644
index 0000000..454d986
--- /dev/null
+++ b/sync/test/test_transaction_observer.cc
@@ -0,0 +1,30 @@
+// Copyright (c) 2012 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 "sync/test/test_transaction_observer.h"
+
+namespace syncer {
+
+namespace syncable {
+
+TestTransactionObserver::TestTransactionObserver()
+  : transactions_observed_(0) {
+}
+
+TestTransactionObserver::~TestTransactionObserver() {
+}
+
+int TestTransactionObserver::transactions_observed() {
+  return transactions_observed_;
+}
+
+void TestTransactionObserver::OnTransactionWrite(
+    const ImmutableWriteTransactionInfo& write_transaction_info,
+    ModelTypeSet models_with_changes) {
+  transactions_observed_++;
+}
+
+}  // namespace syncable
+
+}  // namespace syncer
diff --git a/sync/test/test_transaction_observer.h b/sync/test/test_transaction_observer.h
new file mode 100644
index 0000000..b76870c
--- /dev/null
+++ b/sync/test/test_transaction_observer.h
@@ -0,0 +1,45 @@
+// Copyright (c) 2012 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.
+
+#ifndef SYNC_TEST_TEST_TRANSACTION_OBSERVER_H_
+#define SYNC_TEST_TEST_TRANSACTION_OBSERVER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "sync/internal_api/public/base/model_type.h"
+#include "sync/syncable/transaction_observer.h"
+#include "sync/syncable/write_transaction_info.h"
+
+namespace syncer {
+namespace syncable {
+
+// This class acts as a TransactionObserver for the syncable::Directory.
+// It gathers information that is useful for writing test assertions.
+class TestTransactionObserver :
+  public base::SupportsWeakPtr<TestTransactionObserver>,
+  public TransactionObserver {
+ public:
+  TestTransactionObserver();
+  virtual ~TestTransactionObserver();
+  virtual void OnTransactionWrite(
+      const ImmutableWriteTransactionInfo& write_transaction_info,
+      ModelTypeSet models_with_changes) OVERRIDE;
+
+  // Returns the number of transactions observed.
+  //
+  // Transactions are generated only when meaningful changes are made.  For most
+  // testing purposes, you may assume that this counts the number of syncer
+  // nudges generated.
+  int transactions_observed();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestTransactionObserver);
+
+  int transactions_observed_;
+};
+
+}  // namespace syncable
+}  // namespace syncer
+
+
+#endif  // SYNC_TEST_TEST_TRANSACTION_OBSERVER_H_
diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc
index a7ff438..c2dc657 100644
--- a/sync/tools/sync_client.cc
+++ b/sync/tools/sync_client.cc
@@ -340,7 +340,7 @@ int SyncClientMain(int argc, char* argv[]) {
   const char kSyncServerAndPath[] = "clients4.google.com/chrome-sync/dev";
   int kSyncServerPort = 443;
   bool kUseSsl = true;
-  // Used only by RefreshNigori(), so it's okay to leave this as NULL.
+  // Used only by InitialProcessMetadata(), so it's okay to leave this as NULL.
   const scoped_refptr<base::TaskRunner> blocking_task_runner = NULL;
   const char kUserAgent[] = "sync_client";
   // TODO(akalin): Replace this with just the context getter once
@@ -366,7 +366,6 @@ int SyncClientMain(int argc, char* argv[]) {
                     kSyncServerAndPath,
                     kSyncServerPort,
                     kUseSsl,
-                    blocking_task_runner,
                     post_factory.Pass(),
                     workers,
                     extensions_activity_monitor,