Delete BookmarkModelAssociator and BookmarkChangeProcessor

These were part of the old bookmark sync implementation and not used
anymore.

With this, the ModelAssociator interface becomes unused and is removed
too.

profile_sync_service_bookmark_unittest.cc basically contained unit tests
for the two removed classes, also based on low-level directory code.
These are not relevant anymore and so are also removed.

Bug: 933756
Change-Id: I616f38ab9170d3d3fdae5eac427c8eda1c0ab658
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863808
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707790}

chrome/browser/sync/test/integration/bookmarks_helper.cc

@@ -46,11 +46,11 @@
 #include "components/history/core/browser/history_types.h"
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/test/fake_server/entity_builder_factory.h"
-#include "components/sync_bookmarks/bookmark_change_processor.h"
 #include "content/public/test/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/models/tree_node_iterator.h"
+#include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image_skia.h"
 
 using bookmarks::BookmarkModel;
@@ -58,6 +58,44 @@ using bookmarks::BookmarkNode;
 
 namespace {
 
+void ApplyBookmarkFavicon(
+    const BookmarkNode* bookmark_node,
+    favicon::FaviconService* favicon_service,
+    const GURL& icon_url,
+    const scoped_refptr<base::RefCountedMemory>& bitmap_data) {
+  // Some tests use no services.
+  if (favicon_service == nullptr)
+    return;
+
+  favicon_service->AddPageNoVisitForBookmark(bookmark_node->url(),
+                                             bookmark_node->GetTitle());
+
+  GURL icon_url_to_use = icon_url;
+
+  if (icon_url.is_empty()) {
+    if (bitmap_data->size() == 0) {
+      // Empty icon URL and no bitmap data means no icon mapping.
+      favicon_service->DeleteFaviconMappings({bookmark_node->url()},
+                                             favicon_base::IconType::kFavicon);
+      return;
+    } else {
+      // Ancient clients (prior to M25) may not be syncing the favicon URL. If
+      // the icon URL is not synced, use the page URL as a fake icon URL as it
+      // is guaranteed to be unique.
+      icon_url_to_use = bookmark_node->url();
+    }
+  }
+
+  // The client may have cached the favicon at 2x. Use MergeFavicon() as not to
+  // overwrite the cached 2x favicon bitmap. Sync favicons are always
+  // gfx::kFaviconSize in width and height. Store the favicon into history
+  // as such.
+  gfx::Size pixel_size(gfx::kFaviconSize, gfx::kFaviconSize);
+  favicon_service->MergeFavicon(bookmark_node->url(), icon_url_to_use,
+                                favicon_base::IconType::kFavicon, bitmap_data,
+                                pixel_size);
+}
+
 // History task which signals an event.
 class SignalEventTask : public history::HistoryDBTask {
  public:
@@ -248,14 +286,13 @@ void SetFaviconImpl(Profile* profile,
   if (favicon_source == bookmarks_helper::FROM_UI) {
     favicon_service->SetFavicons({node->url()}, icon_url,
                                  favicon_base::IconType::kFavicon, image);
-    } else {
-      sync_bookmarks::BookmarkChangeProcessor::ApplyBookmarkFavicon(
-          node, favicon_service, icon_url, image.As1xPNGBytes());
-    }
+  } else {
+    ApplyBookmarkFavicon(node, favicon_service, icon_url, image.As1xPNGBytes());
+  }
 
-    // Wait for the favicon for |node| to be invalidated.
-    observer.WaitForSetFavicon();
-    model->GetFavicon(node);
+  // Wait for the favicon for |node| to be invalidated.
+  observer.WaitForSetFavicon();
+  model->GetFavicon(node);
 }
 
 // Expires the favicon for |profile| and |node|. |profile| may be
@@ -293,7 +330,7 @@ void DeleteFaviconMappingsImpl(Profile* profile,
     favicon_service->DeleteFaviconMappings({node->url()},
                                            favicon_base::IconType::kFavicon);
   } else {
-    sync_bookmarks::BookmarkChangeProcessor::ApplyBookmarkFavicon(
+    ApplyBookmarkFavicon(
         node, favicon_service, /*icon_url=*/GURL(),
         scoped_refptr<base::RefCountedString>(new base::RefCountedString()));
   }

components/browser_sync/BUILD.gn

@@ -44,9 +44,7 @@ static_library("browser_sync") {
 source_set("unit_tests") {
   testonly = true
 
-  sources = [
-    "profile_sync_service_bookmark_unittest.cc",
-  ]
+  sources = []
 
   deps = [
     ":browser_sync",

components/sync/driver/BUILD.gn

@@ -43,7 +43,6 @@ jumbo_static_library("driver") {
     "glue/sync_engine_impl.h",
     "model_association_manager.cc",
     "model_association_manager.h",
-    "model_associator.h",
     "model_type_controller.cc",
     "model_type_controller.h",
     "non_ui_syncable_service_based_model_type_controller.cc",
@@ -170,8 +169,6 @@ static_library("test_support") {
     "fake_sync_service.h",
     "mock_sync_service.cc",
     "mock_sync_service.h",
-    "model_associator_mock.cc",
-    "model_associator_mock.h",
     "profile_sync_service_bundle.cc",
     "profile_sync_service_bundle.h",
     "sync_api_component_factory_mock.cc",

components/sync/driver/model_associator.h

-// Copyright 2014 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 COMPONENTS_SYNC_DRIVER_MODEL_ASSOCIATOR_H_
-#define COMPONENTS_SYNC_DRIVER_MODEL_ASSOCIATOR_H_
-
-#include <stdint.h>
-
-#include "base/synchronization/lock.h"
-#include "components/sync/base/model_type.h"
-#include "components/sync/model/sync_error.h"
-
-namespace syncer {
-
-class SyncMergeResult;
-
-// This represents the fundamental operations used for model association that
-// are common to all ModelAssociators and do not depend on types of the models
-// being associated.
-class AssociatorInterface {
- public:
-  virtual ~AssociatorInterface() {}
-
-  // Iterates through both the sync and the chrome model looking for
-  // matched pairs of items. After successful completion, the models
-  // should be identical and corresponding. Returns true on
-  // success. On failure of this step, we should abort the sync
-  // operation and report an error to the user.
-  virtual SyncError AssociateModels(SyncMergeResult* local_merge_result,
-                                    SyncMergeResult* syncer_merge_result) = 0;
-
-  // Clears all the associations between the chrome and sync models.
-  virtual SyncError DisassociateModels() = 0;
-
-  // The has_nodes out parameter is set to true if the sync model has
-  // nodes other than the permanent tagged nodes.  The method may
-  // return false if an error occurred.
-  virtual bool SyncModelHasUserCreatedNodes(bool* has_nodes) = 0;
-
-  // Calling this method while AssociateModels() is in progress will
-  // cause the method to exit early with a "false" return value.  This
-  // is useful for aborting model associations for shutdown.  This
-  // method is only implemented for model associators that are invoked
-  // off the main thread.
-  virtual void AbortAssociation() = 0;
-
-  // Returns whether the datatype is ready for encryption/decryption if the
-  // sync service requires it.
-  // TODO(zea): This should be implemented automatically for each datatype, see
-  // http://crbug.com/76232.
-  virtual bool CryptoReadyIfNecessary() = 0;
-};
-
-}  // namespace syncer
-
-#endif  // COMPONENTS_SYNC_DRIVER_MODEL_ASSOCIATOR_H_

components/sync/driver/model_associator_mock.cc

-// Copyright 2014 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 "components/sync/driver/model_associator_mock.h"
-
-namespace syncer {
-
-ModelAssociatorMock::ModelAssociatorMock() {}
-
-ModelAssociatorMock::~ModelAssociatorMock() {}
-
-}  // namespace syncer

components/sync/driver/model_associator_mock.h

-// Copyright 2014 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 COMPONENTS_SYNC_DRIVER_MODEL_ASSOCIATOR_MOCK_H__
-#define COMPONENTS_SYNC_DRIVER_MODEL_ASSOCIATOR_MOCK_H__
-
-#include "base/location.h"
-#include "components/sync/driver/model_associator.h"
-#include "components/sync/model/sync_error.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace syncer {
-
-ACTION_P(SetSyncError, type) {
-  arg0->Reset(FROM_HERE, "test", type);
-}
-
-class ModelAssociatorMock : public AssociatorInterface {
- public:
-  ModelAssociatorMock();
-  ~ModelAssociatorMock() override;
-
-  MOCK_METHOD2(AssociateModels, SyncError(SyncMergeResult*, SyncMergeResult*));
-  MOCK_METHOD0(DisassociateModels, SyncError());
-  MOCK_METHOD1(SyncModelHasUserCreatedNodes, bool(bool* has_nodes));
-  MOCK_METHOD0(AbortAssociation, void());
-  MOCK_METHOD0(CryptoReadyIfNecessary, bool());
-};
-
-}  // namespace syncer
-
-#endif  // COMPONENTS_SYNC_DRIVER_MODEL_ASSOCIATOR_MOCK_H__

components/sync_bookmarks/BUILD.gn

@@ -6,12 +6,8 @@ import("//build/config/features.gni")
 
 static_library("sync_bookmarks") {
   sources = [
-    "bookmark_change_processor.cc",
-    "bookmark_change_processor.h",
     "bookmark_local_changes_builder.cc",
     "bookmark_local_changes_builder.h",
-    "bookmark_model_associator.cc",
-    "bookmark_model_associator.h",
     "bookmark_model_merger.cc",
     "bookmark_model_merger.h",
     "bookmark_model_observer_impl.cc",