[Sync] Have SyncableService's take ownership of their SyncChangeProcessor.

chrome/browser/extensions/app_notification_manager_sync_unittest.cc

 // 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 "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "chrome/browser/extensions/app_notification.h"
 #include "chrome/browser/extensions/app_notification_manager.h"
@@ skipping 43 matching lines @@
 
   int change_list_size() { return change_map_.size(); }
 
  private:
   // Track the changes received in ProcessSyncChanges.
   std::map<std::string, SyncChange> change_map_;
 
   DISALLOW_COPY_AND_ASSIGN(TestChangeProcessor);
 };
 
+class SyncChangeProcessorDelegate : public SyncChangeProcessor {
+ public:
+  explicit SyncChangeProcessorDelegate(SyncChangeProcessor* recipient)
+      : recipient_(recipient) {
+    DCHECK(recipient_);
+  }

[akalin, 2012/03/21 01:23:19]

virtual destructor

[Nicolas Zea, 2012/03/21 17:38:27]

Done.

+
+  // SyncChangeProcessor implementation.
+  virtual SyncError ProcessSyncChanges(
+      const tracked_objects::Location& from_here,
+      const SyncChangeList& change_list) OVERRIDE {
+    return recipient_->ProcessSyncChanges(from_here, change_list);
+  }
+
+ private:
+  // The recipient of all sync changes.
+  SyncChangeProcessor* recipient_;
+};
+
 }  // namespace
 
 class AppNotificationManagerSyncTest : public testing::Test {
  public:
   AppNotificationManagerSyncTest()
       : ui_thread_(BrowserThread::UI, &ui_loop_),
-        file_thread_(BrowserThread::FILE) { }
+        file_thread_(BrowserThread::FILE),
+        sync_processor_(new TestChangeProcessor),
+        sync_processor_delegate_(new SyncChangeProcessorDelegate(
+            sync_processor_.get())) {}
 
   ~AppNotificationManagerSyncTest() {
     model_ = NULL;
   }
 
   virtual void SetUp() {
     ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI));
     file_thread_.Start();
 
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
@@ skipping 16 matching lines @@
   }
 
   static void WaitForFileThread() {
     BrowserThread::PostTask(BrowserThread::FILE,
                             FROM_HERE,
                             base::Bind(&PostQuitToUIThread));
     MessageLoop::current()->Run();
   }
 
   AppNotificationManager* model() { return model_.get(); }
-  TestChangeProcessor* processor() { return &processor_; }
+  TestChangeProcessor* processor() { return sync_processor_.get(); }
+
+  scoped_ptr<SyncChangeProcessor> PassProcessor() {
+    return sync_processor_delegate_.PassAs<SyncChangeProcessor>();

[akalin, 2012/03/21 01:23:19]

Perhaps instead of creating a delegate above, you could instead only create the
TestChangeProcessor when this is called and keep a (non-scoped) pointer to it? 
With comments saying that it's owned by model() and lives as long as it does,
etc.

[Nicolas Zea, 2012/03/21 17:38:27]

I prefer this approach as it lets you verify the behavior after a StopSyncing
call as well. If I create the TestChangeProcessor on demand, it will get
destroyed at StopSyncing time, so we're more limited.

Additionally, this approach lets you have two delegates to the same processor
(which some tests use to verify two syncable services match). Eventually I plan
to pull the SyncChangeProcessorDelegate and a FakeSyncChangeProcessor into their
own class and have all tests use those.

+  }
 
   // Creates a notification whose properties are set from the given integer.
   static AppNotification* CreateNotification(int suffix) {
     return CreateNotification(false, suffix);
   }
   static AppNotification* CreateNotification(bool is_local, int suffix) {
     std::string s = base::IntToString(suffix);
     return CreateNotification(
         is_local, suffix, "guid" + s, "ext" + s, "text" + s, "body" + s,
         "http://www.url" + s + ".com", "link text " + s);
@@ skipping 69 matching lines @@
  protected:
   MessageLoop ui_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread file_thread_;
 
   // We keep two TemplateURLServices to test syncing between them.
   ScopedTempDir temp_dir_;
   scoped_ptr<TestingProfile> profile_;
   scoped_refptr<AppNotificationManager> model_;
 
-  TestChangeProcessor processor_;
+  scoped_ptr<TestChangeProcessor> sync_processor_;
+  scoped_ptr<SyncChangeProcessorDelegate> sync_processor_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(AppNotificationManagerSyncTest);
 };
 
 // Create an AppNotification, convert it to SyncData and convert it back.
 TEST_F(AppNotificationManagerSyncTest, NotificationToSyncDataToNotification) {
   {  // Partial properties set.
     scoped_ptr<AppNotification> notif1(CreateNotificationNoLink(1));
     SyncData sync_data =
         AppNotificationManager::CreateSyncDataFromNotification(*notif1);
@@ skipping 58 matching lines @@
     const AppNotification* notif2 = model()->GetNotification(ext_id, guid);
     ASSERT_TRUE(notif1->Equals(*notif2));
   }
 }
 
 // Model assocation: both models are empty.
 TEST_F(AppNotificationManagerSyncTest, ModelAssocBothEmpty) {
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),  // Empty.
-      processor());
+      PassProcessor());
 
   EXPECT_EQ(0U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   EXPECT_EQ(0, processor()->change_list_size());
 }
 
 // Model assocation: empty sync model and non-empty local model.
 TEST_F(AppNotificationManagerSyncTest, ModelAssocModelEmpty) {
   SyncDataList initial_data;
   initial_data.push_back(CreateSyncData(1));
   initial_data.push_back(CreateSyncData(2));
   initial_data.push_back(CreateSyncData(3));
   initial_data.push_back(CreateSyncData(4));
 
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       initial_data,
-      processor());
+      PassProcessor());
 
   EXPECT_EQ(4U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   // Model should all of the initial sync data.
   for (SyncDataList::const_iterator iter = initial_data.begin();
       iter != initial_data.end(); ++iter) {
     scoped_ptr<AppNotification> notif1(
         AppNotificationManager::CreateNotificationFromSyncData(*iter));
     const std::string& ext_id = notif1->extension_id();
     const std::string& guid = notif1->guid();
     const AppNotification* notif2 = model()->GetNotification(ext_id, guid);
@@ skipping 16 matching lines @@
 
   SyncDataList initial_data;
   initial_data.push_back(CreateSyncData(4));
   initial_data.push_back(CreateSyncData(5));
   initial_data.push_back(CreateSyncData(6));
   initial_data.push_back(CreateSyncData(7));
 
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       initial_data,
-      processor());
+      PassProcessor());
 
   EXPECT_EQ(6U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   for (SyncDataList::const_iterator iter = initial_data.begin();
       iter != initial_data.end(); ++iter) {
     scoped_ptr<AppNotification> notif1(
         AppNotificationManager::CreateNotificationFromSyncData(*iter));
     const std::string& ext_id = notif1->extension_id();
     const std::string& guid = notif1->guid();
     const AppNotification* notif2 = model()->GetNotification(ext_id, guid);
     EXPECT_TRUE(notif2);
@@ skipping 30 matching lines @@
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n1));
   initial_data.push_back(CreateSyncData(6));
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n4));
   initial_data.push_back(CreateSyncData(7));
 
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       initial_data,
-      processor());
+      PassProcessor());
 
   EXPECT_EQ(6U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   for (SyncDataList::const_iterator iter = initial_data.begin();
       iter != initial_data.end(); ++iter) {
     scoped_ptr<AppNotification> notif1(
         AppNotificationManager::CreateNotificationFromSyncData(*iter));
     const std::string& ext_id = notif1->extension_id();
     const std::string& guid = notif1->guid();
     const AppNotification* notif2 = model()->GetNotification(ext_id, guid);
     EXPECT_TRUE(notif2);
@@ skipping 27 matching lines @@
       n1->is_local(), n1->creation_time().ToInternalValue(),
       n1->guid(), n1->extension_id(),
       n1->title() + "_changed", // different title
       n1->body(), n1->link_url().spec(), n1->link_text()));
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n1_a));
 
   SyncError sync_error = model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       initial_data,
-      processor());
+      PassProcessor());
 
   EXPECT_TRUE(sync_error.IsSet());
   EXPECT_EQ(syncable::APP_NOTIFICATIONS, sync_error.type());
   EXPECT_EQ(0, processor()->change_list_size());
 }
 
 // When an item in sync matches with a local-only item in model, an error
 // should be returned.
 TEST_F(AppNotificationManagerSyncTest, ModelAssocBothNonEmptyMatchesLocal) {
   AppNotification* n1 = CreateNotification(1);
   model()->Add(n1);
   AppNotification* n2 = CreateNotification(true, 2);
   model()->Add(n2);
 
   SyncDataList initial_data;
   initial_data.push_back(CreateSyncData(1));
   scoped_ptr<AppNotification> n2_a(CreateNotification(2));
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n2_a));
 
   SyncError sync_error = model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       initial_data,
-      processor());
+      PassProcessor());
 
   EXPECT_TRUE(sync_error.IsSet());
   EXPECT_EQ(syncable::APP_NOTIFICATIONS, sync_error.type());
   EXPECT_EQ(0, processor()->change_list_size());
 }
 
 // Process sync changes when model is empty.
 TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesEmptyModel) {
   // We initially have no data.
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
   // Set up a bunch of ADDs.
   SyncChangeList changes;
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(1)));
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(2)));
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(3)));
 
   model()->ProcessSyncChanges(FROM_HERE, changes);
 
   EXPECT_EQ(3U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   EXPECT_EQ(0, processor()->change_list_size());
 }
 
 // Process sync changes when model is not empty.
 TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesNonEmptyModel) {
   AppNotification* n1 = CreateNotification(1);
   model()->Add(n1);
   AppNotification* n2 = CreateNotification(2);
   model()->Add(n2);
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
   // Some adds and some deletes.
   SyncChangeList changes;
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(3)));
   changes.push_back(CreateSyncChange(SyncChange::ACTION_DELETE, n1->Copy()));
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(4)));
 
   model()->ProcessSyncChanges(FROM_HERE, changes);
 
   EXPECT_EQ(3U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   EXPECT_EQ(2, processor()->change_list_size());
 }
 
 // Process sync changes should ignore a bad ADD.
 TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesIgnoreBadAdd) {
   AppNotification* n1 = CreateNotification(1);
   model()->Add(n1);
   AppNotification* n2 = CreateNotification(2);
   model()->Add(n2);
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
   // Some adds and some deletes.
   SyncChangeList changes;
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(1)));
 
   SyncError error = model()->ProcessSyncChanges(FROM_HERE, changes);
   EXPECT_FALSE(error.IsSet());
 
   EXPECT_EQ(2U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   EXPECT_EQ(2, processor()->change_list_size());
 }
 
 // Process sync changes should ignore a bad DELETE.
 TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesIgnoreBadDelete) {
   AppNotification* n1 = CreateNotification(1);
   model()->Add(n1);
   AppNotification* n2 = CreateNotification(2);
   model()->Add(n2);
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
   // Some adds and some deletes.
   SyncChangeList changes;
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_DELETE, CreateNotification(3)));
 
   SyncError error = model()->ProcessSyncChanges(FROM_HERE, changes);
   EXPECT_FALSE(error.IsSet());
 
   EXPECT_EQ(2U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   EXPECT_EQ(2, processor()->change_list_size());
 }
 
 // Process sync changes should ignore bad UPDATEs.
 TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesIgnoreBadUpdates) {
   AppNotification* n1 = CreateNotification(1);
   model()->Add(n1);
   AppNotification* n2 = CreateNotification(2);
   model()->Add(n2);
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
   // Some adds and some deletes.
   SyncChangeList changes;
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_UPDATE, CreateNotification(3)));
   AppNotification* n2_changed = n2->Copy();
   n2_changed->set_link_text(n2_changed->link_text() + "-changed");
   changes.push_back(CreateSyncChange(
       SyncChange::ACTION_UPDATE, n2_changed));
 
   SyncError error = model()->ProcessSyncChanges(FROM_HERE, changes);
   EXPECT_FALSE(error.IsSet());
 
   EXPECT_EQ(2U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
   EXPECT_EQ(2, processor()->change_list_size());
 }
 
 // Process over 15 changes when model is not empty.
 TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesEmptyModelWithMax) {
   const std::string& ext_id = "e1";
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
   for (unsigned int i = 0;
        i < AppNotificationManager::kMaxNotificationPerApp * 2; i++) {
     SyncChangeList changes;
     changes.push_back(CreateSyncChange(
       SyncChange::ACTION_ADD, CreateNotification(false, i, ext_id)));
     model()->ProcessSyncChanges(FROM_HERE, changes);
     if (i < AppNotificationManager::kMaxNotificationPerApp) {
       EXPECT_EQ(i + 1,
                 model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size());
     } else {
@@ skipping 14 matching lines @@
   SyncChangeList changes;
 
   SyncError sync_error = model()->ProcessSyncChanges(FROM_HERE, changes);
   EXPECT_TRUE(sync_error.IsSet());
   EXPECT_EQ(syncable::APP_NOTIFICATIONS, sync_error.type());
   EXPECT_EQ(0, processor()->change_list_size());
 }
 
 // Stop syncing sets state correctly.
 TEST_F(AppNotificationManagerSyncTest, StopSyncing) {
-  EXPECT_FALSE(model()->sync_processor_);
+  EXPECT_FALSE(model()->sync_processor_.get());
   EXPECT_FALSE(model()->models_associated_);
 
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
-  EXPECT_TRUE(model()->sync_processor_);
+  EXPECT_TRUE(model()->sync_processor_.get());
   EXPECT_TRUE(model()->models_associated_);
 
   model()->StopSyncing(syncable::APP_NOTIFICATIONS);
-  EXPECT_FALSE(model()->sync_processor_);
+  EXPECT_FALSE(model()->sync_processor_.get());
   EXPECT_FALSE(model()->models_associated_);
 }
 
 // Adds get pushed to sync but local only are skipped.
 TEST_F(AppNotificationManagerSyncTest, AddsGetsSynced) {
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       SyncDataList(),
-      processor());
+      PassProcessor());
 
   AppNotification* n1 = CreateNotification(1);
   model()->Add(n1);
   AppNotification* n2 = CreateNotification(2);
   model()->Add(n2);
   AppNotification* n3 = CreateNotification(true, 2);
   model()->Add(n3);
 
   EXPECT_EQ(2, processor()->change_list_size());
   EXPECT_TRUE(processor()->ContainsGuid(n1->guid()));
@@ skipping 16 matching lines @@
       AppNotificationManager::CreateSyncDataFromNotification(*n1));
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n2));
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n3));
   initial_data.push_back(
       AppNotificationManager::CreateSyncDataFromNotification(*n4));
   model()->MergeDataAndStartSyncing(
       syncable::APP_NOTIFICATIONS,
       initial_data,
-      processor());
+      PassProcessor());
 
   model()->ClearAll(ext_id);
 
   EXPECT_EQ(3, processor()->change_list_size());
   EXPECT_TRUE(processor()->ContainsGuid(n1->guid()));
   SyncChange c1 = processor()->GetChangeByGuid(n1->guid());
   AssertSyncChange(c1, SyncChange::ACTION_DELETE, *n1);
   SyncChange c2 = processor()->GetChangeByGuid(n2->guid());
   AssertSyncChange(c2, SyncChange::ACTION_DELETE, *n2);
   SyncChange c3 = processor()->GetChangeByGuid(n3->guid());
   AssertSyncChange(c3, SyncChange::ACTION_DELETE, *n3);
 }