Add Store+Sync to reading list.

ios/chrome/browser/reading_list/reading_list_store.h

+// 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.
+
+#ifndef IOS_CHROME_BROWSER_READING_LIST_READING_LIST_STORE_H_
+#define IOS_CHROME_BROWSER_READING_LIST_READING_LIST_STORE_H_
+
+#include "components/leveldb_proto/proto_database.h"
+#include "components/sync/model/model_type_service.h"
+#include "components/sync/model/model_type_store.h"
+#include "ios/chrome/browser/reading_list/reading_list_model_storage.h"
+
+using ReadingListDB =
+    leveldb_proto::ProtoDatabase<reading_list::ReadingListLocal>;
+
+namespace syncer {
+class ModelTypeService;
+class MutableDataBatch;
+}
+
+typedef base::Callback<void(
+    const syncer::ModelTypeStore::InitCallback& callback)>
+    StoreFactoryFunction;
+
+// A ReadingListModelStorage storing data in protobufs.
+class ReadingListStore : public ReadingListModelStorage,
+                         public syncer::ModelTypeService {
+  using EntryVector = std::vector<reading_list::ReadingListLocal>;
+
+ public:
+  ReadingListStore(std::unique_ptr<ReadingListDB> database,
+                   const base::FilePath& database_dir,
+                   StoreFactoryFunction create_store_callback);
+  ~ReadingListStore() override;
+
+  // ReadingListModelStorage implementation
+  void SetReadingListModel(ReadingListModelImpl* model) override;
+  void BeginTransaction() override;
+  void CommitTransaction() override;
+  void SaveEntry(const ReadingListEntry& entry,
+                 bool read,
+                 bool from_sync) override;
+  void RemoveEntry(const ReadingListEntry& entry, bool from_sync) override;
+
+  // ReadingListModelStorage implementation.
+  syncer::ModelTypeService* GetModelTypeService() override;
+
+  // Creates an object used to communicate changes in the sync metadata to the
+  // model type store.
+  std::unique_ptr<syncer::MetadataChangeList> CreateMetadataChangeList()
+      override;
+
+  // Perform the initial merge between local and sync data. This should only be
+  // called when a data type is first enabled to start syncing, and there is no
+  // sync metadata. Best effort should be made to match local and sync data. The
+  // keys in the |entity_data_map| will have been created via GetClientTag(...),
+  // and if a local and sync data should match/merge but disagree on tags, the
+  // service should use the sync data's tag. Any local pieces of data that are
+  // not present in sync should immediately be Put(...) to the processor before
+  // returning. The same MetadataChangeList that was passed into this function
+  // can be passed to Put(...) calls. Delete(...) can also be called but should
+  // not be needed for most model types. Durable storage writes, if not able to
+  // combine all change atomically, should save the metadata after the data
+  // changes, so that this merge will be re-driven by sync if is not completely
+  // saved during the current run.
+  syncer::SyncError MergeSyncData(
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      syncer::EntityDataMap entity_data_map) override;
+
+  // Apply changes from the sync server locally.
+  // Please note that |entity_changes| might have fewer entries than
+  // |metadata_change_list| in case when some of the data changes are filtered
+  // out, or even be empty in case when a commit confirmation is processed and
+  // only the metadata needs to persisted.
+  syncer::SyncError ApplySyncChanges(
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      syncer::EntityChangeList entity_changes) override;
+
+  // Asynchronously retrieve the corresponding sync data for |storage_keys|.
+  void GetData(StorageKeyList storage_keys, DataCallback callback) override;
+
+  // Asynchronously retrieve all of the local sync data.
+  void GetAllData(DataCallback callback) override;
+
+  // Get or generate a client tag for |entity_data|. This must be the same tag
+  // that was/would have been generated in the SyncableService/Directory world
+  // for backward compatibility with pre-USS clients. The only time this
+  // theoretically needs to be called is on the creation of local data, however
+  // it is also used to verify the hash of remote data. If a data type was never
+  // launched pre-USS, then method does not need to be different from
+  // GetStorageKey().
+  std::string GetClientTag(const syncer::EntityData& entity_data) override;
+
+  // Get or generate a storage key for |entity_data|. This will only ever be
+  // called once when first encountering a remote entity. Local changes will
+  // provide their storage keys directly to Put instead of using this method.
+  // Theoretically this function doesn't need to be stable across multiple calls
+  // on the same or different clients, but to keep things simple, it probably
+  // should be.
+  std::string GetStorageKey(const syncer::EntityData& entity_data) override;
+
+  // Methods used as callbacks given to DataTypeStore.
+  void OnStoreCreated(syncer::ModelTypeStore::Result result,
+                      std::unique_ptr<syncer::ModelTypeStore> store);
+
+ private:
+  void OnDatabaseInit(bool success);
+  void OnDatabaseLoad(
+      syncer::ModelTypeStore::Result result,
+      std::unique_ptr<syncer::ModelTypeStore::RecordList> entries);
+  void OnDatabaseSave(syncer::ModelTypeStore::Result result);
+  void OnReadAllMetadata(
+      syncer::ModelTypeStore::Result result,
+      std::unique_ptr<syncer::ModelTypeStore::RecordList> metadata_records,
+      const std::string& global_metadata);
+
+  void AddEntryToBatch(syncer::MutableDataBatch* batch,
+                       const ReadingListEntry& entry,
+                       bool read);
+
+  std::unique_ptr<syncer::ModelTypeStore> store_;

[skym, 2016/10/28 16:30:56]

Hmm. This concerns me. It seems like |model_| and |store_| both have durable
storage. This is exactly the scenario we're trying to stop with USS.

We want both metadata and real data to be stored in the same persistent storage,
such that you can guarantee they are either atomically stored to disk, or
they're stored in a strict order such that crashing in the middle does not leave
us in a bad state. (On local change it should be metadata -> real data, on
remote change it should be real data -> metadata)

https://docs.google.com/document/d/1QvfB9XTonsnnJf1PiPuT0nwaGObql5G04C6ccw2LA...

I'd much prefer a solution with a custom MetadataChangeList implementation that
batches changes that are going to eventually make their way into
reading_list_model_storage_defaults.mm's |backend_| somehow.

Or, completely remove reading_list_model_storage_defaults.mm and use the
syncer::ModelTypeStore leveldb as your model's storage.

[skym, 2016/11/01 16:40:23]

Pavel has informed me that I was confused. You are in fact deleting
reading_list_model_storage_defaults.mm! Awesome. Taking another look at things.

+  bool database_loaded_;
+  ReadingListModelImpl* model_;
+  StoreFactoryFunction create_store_callback_;
+  bool has_metadata_loaded_ = false;
+
+  int pending_transaction_;
+  std::unique_ptr<syncer::ModelTypeStore::WriteBatch> batch_;
+};
+
+#endif  // IOS_CHROME_BROWSER_READING_LIST_READING_LIST_STORE_H_