Fix inheritance in Reading List

Make ReadingListModelStorage inherit from syncer::ModelTypeSyncBridge.
ReadingList require the storage to implement USS so add a direct
inheritance to avoid multiple inheritance.

Mark ReadingListStoreDelegate constructor and destructor protected.
ReadingListStoreDelegate is an interface, mark constructor and destructor
protected.

BUG=673169

Review-Url: https://codereview.chromium.org/2568023002
Cr-Commit-Position: refs/heads/master@{#437850}

components/reading_list/ios/BUILD.gn

@@ -15,6 +15,7 @@ source_set("ios") {
     "reading_list_model_impl.cc",
     "reading_list_model_impl.h",
     "reading_list_model_observer.h",
+    "reading_list_model_storage.cc",
     "reading_list_model_storage.h",
     "reading_list_pref_names.cc",
     "reading_list_pref_names.h",

components/reading_list/ios/reading_list_model_impl.cc

@@ -404,7 +404,7 @@ syncer::ModelTypeSyncBridge* ReadingListModelImpl::GetModelTypeSyncBridge() {
   DCHECK(loaded());
   if (!storage_layer_)
     return nullptr;
-  return storage_layer_->GetModelTypeSyncBridge();
+  return storage_layer_.get();
 }
 
 ReadingListModelStorage* ReadingListModelImpl::StorageLayer() {

components/reading_list/ios/reading_list_model_storage.cc

+// Copyright 2016 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/reading_list/ios/reading_list_model_storage.h"
+
+ReadingListModelStorage::ReadingListModelStorage(
+    const ChangeProcessorFactory& change_processor_factory,
+    syncer::ModelType type)
+    : ModelTypeSyncBridge(change_processor_factory, type) {}
+
+ReadingListModelStorage::~ReadingListModelStorage() {}

components/reading_list/ios/reading_list_model_storage.h

@@ -9,6 +9,8 @@
 
 #include "base/macros.h"
 #include "components/reading_list/ios/reading_list_entry.h"
+#include "components/sync/base/model_type.h"
+#include "components/sync/model/model_type_sync_bridge.h"
 
 class ReadingListModel;
 class ReadingListStoreDelegate;
@@ -19,21 +21,20 @@ class ModelTypeSyncBridge;
 
 // Interface for a persistence layer for reading list.
 // All interface methods have to be called on main thread.
-class ReadingListModelStorage {
+class ReadingListModelStorage : public syncer::ModelTypeSyncBridge {
  public:
   class ScopedBatchUpdate;
 
-  ReadingListModelStorage() {}
-  virtual ~ReadingListModelStorage() {}
+  ReadingListModelStorage(
+      const ChangeProcessorFactory& change_processor_factory,
+      syncer::ModelType type);
+  ~ReadingListModelStorage() override;
 
   // Sets the model the Storage is backing.
   // This will trigger store initalization and load persistent entries.
   virtual void SetReadingListModel(ReadingListModel* model,
                                    ReadingListStoreDelegate* delegate) = 0;
 
-  // Returns the class responsible for handling sync messages.
-  virtual syncer::ModelTypeSyncBridge* GetModelTypeSyncBridge() = 0;
-
   // Starts a transaction. All Save/Remove entry will be delayed until the
   // transaction is commited.
   // Multiple transaction can be started at the same time. Commit will happen

components/reading_list/ios/reading_list_model_unittest.mm

@@ -10,6 +10,7 @@
 #include "components/reading_list/ios/reading_list_model_impl.h"
 #include "components/reading_list/ios/reading_list_model_storage.h"
 #include "components/reading_list/ios/reading_list_store_delegate.h"
+#include "components/sync/model/metadata_change_list.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -26,7 +27,10 @@ class TestReadingListStorageObserver {
 class TestReadingListStorage : public ReadingListModelStorage {
  public:
   TestReadingListStorage(TestReadingListStorageObserver* observer)
-      : entries_(new ReadingListStoreDelegate::ReadingListEntries()),
+      : ReadingListModelStorage(
+            base::Bind(&syncer::ModelTypeChangeProcessor::Create),
+            syncer::READING_LIST),
+        entries_(new ReadingListStoreDelegate::ReadingListEntries()),
         observer_(observer) {}
 
   void AddSampleEntries() {
@@ -83,25 +87,62 @@ class TestReadingListStorage : public ReadingListModelStorage {
     delegate->StoreLoaded(std::move(entries_));
   }
 
-  syncer::ModelTypeSyncBridge* GetModelTypeSyncBridge() override {
-    return nullptr;
-  }
-
-  std::unique_ptr<ScopedBatchUpdate> EnsureBatchCreated() override {
-    return std::unique_ptr<ScopedBatchUpdate>();
-  }
-
   // Saves or updates an entry. If the entry is not yet in the database, it is
   // created.
   void SaveEntry(const ReadingListEntry& entry) override {
     observer_->ReadingListDidSaveEntry();
   }
 
-  // Removed an entry from the storage.
+  // Removes an entry from the storage.
   void RemoveEntry(const ReadingListEntry& entry) override {
     observer_->ReadingListDidRemoveEntry();
   }
 
+  std::unique_ptr<ScopedBatchUpdate> EnsureBatchCreated() override {
+    return std::unique_ptr<ScopedBatchUpdate>();
+  }
+
+  // Syncing is not used in this test class.
+  std::unique_ptr<syncer::MetadataChangeList> CreateMetadataChangeList()
+      override {
+    NOTREACHED();
+    return std::unique_ptr<syncer::MetadataChangeList>();
+  }
+
+  syncer::SyncError MergeSyncData(
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      syncer::EntityDataMap entity_data_map) override {
+    NOTREACHED();
+    return syncer::SyncError();
+  }
+
+  syncer::SyncError ApplySyncChanges(
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      syncer::EntityChangeList entity_changes) override {
+    NOTREACHED();
+    return syncer::SyncError();
+  }
+
+  void GetData(StorageKeyList storage_keys, DataCallback callback) override {
+    NOTREACHED();
+    return;
+  }
+
+  void GetAllData(DataCallback callback) override {
+    NOTREACHED();
+    return;
+  }
+
+  std::string GetClientTag(const syncer::EntityData& entity_data) override {
+    NOTREACHED();
+    return "";
+  }
+
+  std::string GetStorageKey(const syncer::EntityData& entity_data) override {
+    NOTREACHED();
+    return "";
+  }
+
  private:
   std::unique_ptr<ReadingListStoreDelegate::ReadingListEntries> entries_;
   TestReadingListStorageObserver* observer_;

components/reading_list/ios/reading_list_store.cc

@@ -20,7 +20,7 @@
 ReadingListStore::ReadingListStore(
     StoreFactoryFunction create_store_callback,
     const ChangeProcessorFactory& change_processor_factory)
-    : ModelTypeSyncBridge(change_processor_factory, syncer::READING_LIST),
+    : ReadingListModelStorage(change_processor_factory, syncer::READING_LIST),
       create_store_callback_(create_store_callback),
       pending_transaction_count_(0) {}
 
@@ -176,10 +176,6 @@ void ReadingListStore::OnStoreCreated(
   return;
 }
 
-syncer::ModelTypeSyncBridge* ReadingListStore::GetModelTypeSyncBridge() {
-  return this;
-}
-
 // Creates an object used to communicate changes in the sync metadata to the
 // model type store.
 std::unique_ptr<syncer::MetadataChangeList>

components/reading_list/ios/reading_list_store.h

@@ -9,7 +9,6 @@
 #include "components/reading_list/ios/reading_list_model_storage.h"
 #include "components/reading_list/ios/reading_list_store_delegate.h"
 #include "components/sync/model/model_type_store.h"
-#include "components/sync/model/model_type_sync_bridge.h"
 
 namespace syncer {
 class MutableDataBatch;
@@ -18,8 +17,7 @@ class MutableDataBatch;
 class ReadingListModel;
 
 // A ReadingListModelStorage storing and syncing data in protobufs.
-class ReadingListStore : public syncer::ModelTypeSyncBridge,
-                         public ReadingListModelStorage,
+class ReadingListStore : public ReadingListModelStorage,
                          public base::NonThreadSafe {
   using StoreFactoryFunction = base::Callback<void(
       const syncer::ModelTypeStore::InitCallback& callback)>;
@@ -38,9 +36,6 @@ class ReadingListStore : public syncer::ModelTypeSyncBridge,
   void SaveEntry(const ReadingListEntry& entry) override;
   void RemoveEntry(const ReadingListEntry& entry) override;
 
-  // ReadingListModelStorage implementation.
-  syncer::ModelTypeSyncBridge* GetModelTypeSyncBridge() override;
-
   // Creates an object used to communicate changes in the sync metadata to the
   // model type store.
   std::unique_ptr<syncer::MetadataChangeList> CreateMetadataChangeList()

components/reading_list/ios/reading_list_store_delegate.h

@@ -14,9 +14,6 @@ class ReadingListEntry;
 // The delegate to handle callbacks from the ReadingListStore.
 class ReadingListStoreDelegate {
  public:
-  ReadingListStoreDelegate() {}
-  virtual ~ReadingListStoreDelegate() {}
-
   using ReadingListEntries = std::map<GURL, ReadingListEntry>;
   // These three methods handle callbacks from a ReadingListStore.
   // This method is called when the local store is loaded. |entries| contains
@@ -36,6 +33,11 @@ class ReadingListStoreDelegate {
 
   // Called to remove an entry to the model.
   virtual void SyncRemoveEntry(const GURL& url) = 0;
+
+ protected:
+  ReadingListStoreDelegate() {}
+  virtual ~ReadingListStoreDelegate() {}
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ReadingListStoreDelegate);
 };