App list sync unit tests

chrome/browser/ui/app_list/app_list_syncable_service_unittest.cc

 // Copyright 2017 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/ui/app_list/app_list_syncable_service.h"
 #include "base/files/scoped_temp_dir.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/app_list_test_util.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/crx_file/id_util.h"
+#include "components/sync/model/attachments/attachment_service_proxy_for_test.h"
+#include "components/sync/model/fake_sync_change_processor.h"
+#include "components/sync/model/sync_error_factory.h"
+#include "components/sync/model/sync_error_factory_mock.h"
+#include "components/sync/protocol/sync.pb.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/constants.h"
 #include "ui/app_list/app_list_item.h"
 #include "ui/app_list/app_list_model.h"
 
+using namespace crx_file::id_util;
+
 namespace {
 
 scoped_refptr<extensions::Extension> MakeApp(
     const std::string& name,
     const std::string& id,
     extensions::Extension::InitFromValueFlags flags) {
   std::string err;
   base::DictionaryValue value;
   value.SetString("name", name);
   value.SetString("version", "0.0");
@@ skipping 11 matching lines @@
   std::string next_app_id = app_id;
   size_t index = next_app_id.length() - 1;
   while (index > 0 && next_app_id[index] == 'p')
     next_app_id[index--] = 'a';
   DCHECK(next_app_id[index] != 'p');
   next_app_id[index]++;
   DCHECK(crx_file::id_util::IdIsValid(next_app_id));
   return next_app_id;
 }
 
+constexpr char kUnset[] = "__unset__";
+constexpr char kDefault[] = "__default__";
+
+// BAD STYLE ALERT: using const instead of constexpr in order to initialize
+// with results of crx_file::id_util::GenerateId().
+const std::string kInvalidOrdinalsId = GenerateId("invalid_ordinals");

[xiyuan, 2017/06/23 17:11:34]

std::string is not allowed for global variable and should not be initialized
with a function call.

[stevenjb, 2017/06/23 19:46:05]

e.g.. these could be members of a helper struct and initialized in the
constructor. The helper can be owned by AppListSyncableServiceTest and passed to
CreateBadAppRemoteData.

Alternatively these can all be constexpr char[] and GenerateId() can be called
where they are used.

[rcui, 2017/06/23 21:28:08]

How about this - defining the constants as functions and keeping the naming
scheme.  Adds extra overhead to the tests, but there's no loop-nesting going on,
so the runtime impact is pretty minimal.

[rcui, 2017/06/23 21:28:41]

This is done in the newest patchset.

+const std::string kEmptyItemNameId = GenerateId("empty_item_name");
+const std::string kEmptyItemNameUnsetId = GenerateId("empty_item_name_unset");
+const std::string kEmptyParentId = GenerateId("empty_parent_id");
+const std::string kEmptyParentUnsetId = GenerateId("empty_parent_id_unset");
+const std::string kEmptyOrdinalsId = GenerateId("empty_ordinals");
+const std::string kEmptyOrdinalsUnsetId = GenerateId("empty_ordinals_unset");
+const std::string kDupeItemId = GenerateId("dupe_item_id");
+const std::string kParentId = GenerateId("parent_id");
+
+syncer::SyncData CreateAppRemoteData(const std::string& id,
+                                     const std::string& name,
+                                     const std::string& parent_id,
+                                     const std::string& item_ordinal,
+                                     const std::string& item_pin_ordinal) {
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::AppListSpecifics* app_list = specifics.mutable_app_list();
+  if (id != kUnset)
+    app_list->set_item_id(id);
+  app_list->set_item_type(sync_pb::AppListSpecifics_AppListItemType_TYPE_APP);
+  if (name != kUnset)
+    app_list->set_item_name(name);
+  if (parent_id != kUnset)
+    app_list->set_parent_id(parent_id);
+  if (item_ordinal != kUnset)
+    app_list->set_item_ordinal(item_ordinal);
+  if (item_pin_ordinal != kUnset)
+    app_list->set_item_pin_ordinal(item_pin_ordinal);
+
+  return syncer::SyncData::CreateRemoteData(
+      std::hash<std::string>{}(id), specifics, base::Time(),
+      syncer::AttachmentIdList(),
+      syncer::AttachmentServiceProxyForTest::Create());
+}
+
+syncer::SyncDataList CreateBadAppRemoteData(const std::string& id) {
+  syncer::SyncDataList sync_list;
+  // Invalid item_ordinal and item_pin_ordinal.
+  sync_list.push_back(CreateAppRemoteData(
+      id == kDefault ? kInvalidOrdinalsId : id, "item_name", kParentId,
+      "$$invalid_ordinal$$", "$$invalid_ordinal$$"));
+  // Empty item name.
+  sync_list.push_back(
+      CreateAppRemoteData(id == kDefault ? kEmptyItemNameId : id, "", kParentId,
+                          "ordinal", "pinordinal"));
+  sync_list.push_back(
+      CreateAppRemoteData(id == kDefault ? kEmptyItemNameUnsetId : id, kUnset,
+                          kParentId, "ordinal", "pinordinal"));
+  // Empty parent ID.
+  sync_list.push_back(CreateAppRemoteData(id == kDefault ? kEmptyParentId : id,
+                                          "item_name", "", "ordinal",
+                                          "pinordinal"));
+  sync_list.push_back(
+      CreateAppRemoteData(id == kDefault ? kEmptyParentUnsetId : id,
+                          "item_name", kUnset, "ordinal", "pinordinal"));
+  // Empty item_ordinal and item_pin_ordinal.
+  sync_list.push_back(CreateAppRemoteData(
+      id == kDefault ? kEmptyOrdinalsId : id, "item_name", kParentId, "", ""));
+  sync_list.push_back(
+      CreateAppRemoteData(id == kDefault ? kEmptyOrdinalsUnsetId : id,
+                          "item_name", kParentId, kUnset, kUnset));
+  // Duplicate item_id.
+  sync_list.push_back(CreateAppRemoteData(id == kDefault ? kDupeItemId : id,
+                                          "item_name", kParentId, "ordinal",
+                                          "pinordinal"));
+  sync_list.push_back(CreateAppRemoteData(id == kDefault ? kDupeItemId : id,
+                                          "item_name_dupe", kParentId,
+                                          "ordinal", "pinordinal"));
+  // Empty item_id.
+  sync_list.push_back(
+      CreateAppRemoteData("", "item_name", kParentId, "ordinal", "pinordinal"));
+  sync_list.push_back(CreateAppRemoteData(kUnset, "item_name", kParentId,
+                                          "ordinal", "pinordinal"));
+  // All fields empty.
+  sync_list.push_back(CreateAppRemoteData("", "", "", "", ""));
+  sync_list.push_back(
+      CreateAppRemoteData(kUnset, kUnset, kUnset, kUnset, kUnset));
+
+  return sync_list;
+}
+
 }  // namespace
 
 class AppListSyncableServiceTest : public AppListTestBase {
  public:
   AppListSyncableServiceTest() = default;
   ~AppListSyncableServiceTest() override = default;
 
   void SetUp() override {
     AppListTestBase::SetUp();
 
     // Make sure we have a Profile Manager.
     DCHECK(temp_dir_.CreateUniqueTempDir());
     TestingBrowserProcess::GetGlobal()->SetProfileManager(
         new ProfileManagerWithoutInit(temp_dir_.GetPath()));
 
     extensions::ExtensionSystem* extension_system =
         extensions::ExtensionSystem::Get(profile_.get());
     DCHECK(extension_system);
     app_list_syncable_service_.reset(
         new app_list::AppListSyncableService(profile_.get(), extension_system));
   }
 
   void TearDown() override { app_list_syncable_service_.reset(); }
 
   app_list::AppListModel* model() {
     return app_list_syncable_service_->GetModel();
   }
 
+  const app_list::AppListSyncableService::SyncItem* GetSyncItem(
+      const std::string& id) const {
+    return app_list_syncable_service_->GetSyncItem(id);
+  }
+
+ protected:
+  app_list::AppListSyncableService* app_list_syncable_service() {
+    return app_list_syncable_service_.get();
+  }
+
  private:
   base::ScopedTempDir temp_dir_;
   std::unique_ptr<app_list::AppListSyncableService> app_list_syncable_service_;
 
   DISALLOW_COPY_AND_ASSIGN(AppListSyncableServiceTest);
 };
 
 TEST_F(AppListSyncableServiceTest, OEMFolderForConflictingPos) {
   // Create a "web store" app.
   const std::string web_store_app_id(extensions::kWebStoreAppId);
@@ skipping 37 matching lines @@
   EXPECT_FALSE(model()->top_level_item_list()->FindItemIndex(oem_app_id,
                                                              &oem_app_index));
   // But OEM folder is.
   EXPECT_TRUE(model()->top_level_item_list()->FindItemIndex(
       app_list::AppListSyncableService::kOemFolderId, &oem_folder_index));
 
   // Ensure right item sequence.
   EXPECT_EQ(some_app_index, web_store_app_index + 1);
   EXPECT_EQ(oem_folder_index, web_store_app_index + 2);
 }
+
+TEST_F(AppListSyncableServiceTest, InitialMerge) {
+  const std::string kItemId1 = GenerateId("item_id1");
+  const std::string kItemId2 = GenerateId("item_id2");
+
+  syncer::SyncDataList sync_list;
+  sync_list.push_back(CreateAppRemoteData(kItemId1, "item_name1",
+                                          GenerateId("parent_id1"), "ordinal",
+                                          "pinordinal"));
+  sync_list.push_back(CreateAppRemoteData(kItemId2, "item_name2",
+                                          GenerateId("parent_id2"), "ordinal",
+                                          "pinordinal"));
+
+  app_list_syncable_service()->MergeDataAndStartSyncing(
+      syncer::APP_LIST, sync_list,
+      base::MakeUnique<syncer::FakeSyncChangeProcessor>(),
+      base::MakeUnique<syncer::SyncErrorFactoryMock>());
+  content::RunAllBlockingPoolTasksUntilIdle();
+
+  ASSERT_TRUE(GetSyncItem(kItemId1));
+  EXPECT_EQ("item_name1", GetSyncItem(kItemId1)->item_name);
+  EXPECT_EQ(GenerateId("parent_id1"), GetSyncItem(kItemId1)->parent_id);
+  EXPECT_EQ("ordinal", GetSyncItem(kItemId1)->item_ordinal.ToDebugString());
+  EXPECT_EQ("pinordinal",
+            GetSyncItem(kItemId1)->item_pin_ordinal.ToDebugString());
+
+  ASSERT_TRUE(GetSyncItem(kItemId2));
+  EXPECT_EQ("item_name2", GetSyncItem(kItemId2)->item_name);
+  EXPECT_EQ(GenerateId("parent_id2"), GetSyncItem(kItemId2)->parent_id);
+  EXPECT_EQ("ordinal", GetSyncItem(kItemId2)->item_ordinal.ToDebugString());
+  EXPECT_EQ("pinordinal",
+            GetSyncItem(kItemId2)->item_pin_ordinal.ToDebugString());
+}
+
+TEST_F(AppListSyncableServiceTest, InitialMerge_BadData) {
+  syncer::SyncDataList sync_list = CreateBadAppRemoteData(kDefault);
+
+  app_list_syncable_service()->MergeDataAndStartSyncing(
+      syncer::APP_LIST, sync_list,
+      base::MakeUnique<syncer::FakeSyncChangeProcessor>(),
+      base::MakeUnique<syncer::SyncErrorFactoryMock>());
+  content::RunAllBlockingPoolTasksUntilIdle();
+
+  // Invalid item_ordinal and item_pin_ordinal.
+  ASSERT_TRUE(GetSyncItem(kInvalidOrdinalsId));
+  EXPECT_EQ("n", GetSyncItem(kInvalidOrdinalsId)->item_ordinal.ToDebugString());
+  EXPECT_EQ("INVALID[$$invalid_ordinal$$]",
+            GetSyncItem(kInvalidOrdinalsId)->item_pin_ordinal.ToDebugString());
+
+  // Empty item name.
+  ASSERT_TRUE(GetSyncItem(kEmptyItemNameId));
+  EXPECT_EQ("", GetSyncItem(kEmptyItemNameId)->item_name);
+  EXPECT_TRUE(GetSyncItem(kEmptyItemNameUnsetId));
+  EXPECT_EQ("", GetSyncItem(kEmptyItemNameUnsetId)->item_name);
+
+  // Empty parent ID.
+  ASSERT_TRUE(GetSyncItem(kEmptyParentId));
+  EXPECT_EQ("", GetSyncItem(kEmptyParentId)->parent_id);
+  EXPECT_TRUE(GetSyncItem(kEmptyParentUnsetId));
+  EXPECT_EQ("", GetSyncItem(kEmptyParentUnsetId)->parent_id);
+
+  // Empty item_ordinal and item_pin_ordinal.
+  ASSERT_TRUE(GetSyncItem(kEmptyOrdinalsId));
+  EXPECT_EQ("n", GetSyncItem(kEmptyOrdinalsId)->item_ordinal.ToDebugString());
+  EXPECT_EQ("INVALID[]",
+            GetSyncItem(kEmptyOrdinalsId)->item_pin_ordinal.ToDebugString());
+  ASSERT_TRUE(GetSyncItem(kEmptyOrdinalsUnsetId));
+  EXPECT_EQ("n",
+            GetSyncItem(kEmptyOrdinalsUnsetId)->item_ordinal.ToDebugString());
+  EXPECT_EQ(
+      "INVALID[]",
+      GetSyncItem(kEmptyOrdinalsUnsetId)->item_pin_ordinal.ToDebugString());
+
+  // Duplicate item_id overrides previous.
+  ASSERT_TRUE(GetSyncItem(kDupeItemId));
+  EXPECT_EQ("item_name_dupe", GetSyncItem(kDupeItemId)->item_name);
+}
+
+TEST_F(AppListSyncableServiceTest, InitialMergeAndUpdate) {
+  const std::string kItemId1 = GenerateId("item_id1");
+  const std::string kItemId2 = GenerateId("item_id2");
+
+  syncer::SyncDataList sync_list;
+  sync_list.push_back(CreateAppRemoteData(kItemId1, "item_name1", kParentId,
+                                          "ordinal", "pinordinal"));
+  sync_list.push_back(CreateAppRemoteData(kItemId2, "item_name2", kParentId,
+                                          "ordinal", "pinordinal"));
+
+  app_list_syncable_service()->MergeDataAndStartSyncing(
+      syncer::APP_LIST, sync_list,
+      base::MakeUnique<syncer::FakeSyncChangeProcessor>(),
+      base::MakeUnique<syncer::SyncErrorFactoryMock>());
+  content::RunAllBlockingPoolTasksUntilIdle();
+
+  ASSERT_TRUE(GetSyncItem(kItemId1));
+  ASSERT_TRUE(GetSyncItem(kItemId2));
+
+  syncer::SyncChangeList change_list;
+  change_list.push_back(syncer::SyncChange(
+      FROM_HERE, syncer::SyncChange::ACTION_UPDATE,
+      CreateAppRemoteData(kItemId1, "item_name1x", GenerateId("parent_id1x"),
+                          "ordinalx", "pinordinalx")));
+  change_list.push_back(syncer::SyncChange(
+      FROM_HERE, syncer::SyncChange::ACTION_UPDATE,
+      CreateAppRemoteData(kItemId2, "item_name2x", GenerateId("parent_id2x"),
+                          "ordinalx", "pinordinalx")));
+
+  app_list_syncable_service()->ProcessSyncChanges(tracked_objects::Location(),
+                                                  change_list);
+  content::RunAllBlockingPoolTasksUntilIdle();
+
+  ASSERT_TRUE(GetSyncItem(kItemId1));
+  EXPECT_EQ("item_name1x", GetSyncItem(kItemId1)->item_name);
+  EXPECT_EQ(GenerateId("parent_id1x"), GetSyncItem(kItemId1)->parent_id);
+  EXPECT_EQ("ordinalx", GetSyncItem(kItemId1)->item_ordinal.ToDebugString());
+  EXPECT_EQ("pinordinalx",
+            GetSyncItem(kItemId1)->item_pin_ordinal.ToDebugString());
+
+  ASSERT_TRUE(GetSyncItem(kItemId2));
+  EXPECT_EQ("item_name2x", GetSyncItem(kItemId2)->item_name);
+  EXPECT_EQ(GenerateId("parent_id2x"), GetSyncItem(kItemId2)->parent_id);
+  EXPECT_EQ("ordinalx", GetSyncItem(kItemId2)->item_ordinal.ToDebugString());
+  EXPECT_EQ("pinordinalx",
+            GetSyncItem(kItemId2)->item_pin_ordinal.ToDebugString());
+}
+
+TEST_F(AppListSyncableServiceTest, InitialMergeAndUpdate_BadData) {
+  const std::string kItemId = GenerateId("item_id");
+
+  syncer::SyncDataList sync_list;
+  sync_list.push_back(CreateAppRemoteData(kItemId, "item_name", kParentId,
+                                          "ordinal", "pinordinal"));
+
+  app_list_syncable_service()->MergeDataAndStartSyncing(
+      syncer::APP_LIST, sync_list,
+      base::MakeUnique<syncer::FakeSyncChangeProcessor>(),
+      base::MakeUnique<syncer::SyncErrorFactoryMock>());
+  content::RunAllBlockingPoolTasksUntilIdle();
+
+  ASSERT_TRUE(GetSyncItem(kItemId));
+
+  syncer::SyncChangeList change_list;
+  syncer::SyncDataList update_list = CreateBadAppRemoteData(kItemId);
+  for (syncer::SyncDataList::const_iterator iter = update_list.begin();
+       iter != update_list.end(); ++iter) {
+    change_list.push_back(syncer::SyncChange(
+        FROM_HERE, syncer::SyncChange::ACTION_UPDATE, *iter));
+  }
+
+  // Validate items with bad data are processed without crashing.
+  app_list_syncable_service()->ProcessSyncChanges(tracked_objects::Location(),
+                                                  change_list);
+  content::RunAllBlockingPoolTasksUntilIdle();
+
+  ASSERT_TRUE(GetSyncItem(kItemId));
+}