Skip to content

Commit

Permalink
Refactor to adopt OnceCallback in USS ModelTypeStore API
Browse files Browse the repository at this point in the history
This is a manually generated patch to update a widely used class in USS,
which prevents new datatypes from properly adopting OnceCallback in
their internals.

No behavioral differences.

Bug: 714018
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If5c63f0c180e077980a74a9dd60def7b9171def4
Reviewed-on: https://chromium-review.googlesource.com/897865
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534978}
  • Loading branch information
Mikel Astiz authored and Commit Bot committed Feb 7, 2018
1 parent 0519cfc commit 6bd0344
Show file tree
Hide file tree
Showing 24 changed files with 231 additions and 221 deletions.
11 changes: 6 additions & 5 deletions chrome/browser/chromeos/printing/printers_sync_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ std::unique_ptr<EntityData> CopyToEntityData(
class PrintersSyncBridge::StoreProxy {
public:
StoreProxy(PrintersSyncBridge* owner,
const syncer::ModelTypeStoreFactory& callback)
syncer::OnceModelTypeStoreFactory callback)
: owner_(owner), weak_ptr_factory_(this) {
callback.Run(syncer::PRINTERS, base::Bind(&StoreProxy::OnStoreCreated,
weak_ptr_factory_.GetWeakPtr()));
std::move(callback).Run(syncer::PRINTERS,
base::BindOnce(&StoreProxy::OnStoreCreated,
weak_ptr_factory_.GetWeakPtr()));
}

// Returns true if the store has been initialized.
Expand Down Expand Up @@ -147,12 +148,12 @@ class PrintersSyncBridge::StoreProxy {
};

PrintersSyncBridge::PrintersSyncBridge(
const syncer::ModelTypeStoreFactory& callback,
syncer::OnceModelTypeStoreFactory callback,
const base::RepeatingClosure& error_callback)
: ModelTypeSyncBridge(base::BindRepeating(&ModelTypeChangeProcessor::Create,
error_callback),
syncer::PRINTERS),
store_delegate_(std::make_unique<StoreProxy>(this, callback)),
store_delegate_(std::make_unique<StoreProxy>(this, std::move(callback))),
observers_(new base::ObserverListThreadSafe<Observer>()) {}

PrintersSyncBridge::~PrintersSyncBridge() {}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/printing/printers_sync_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace chromeos {
// Sync Service for printers.
class PrintersSyncBridge : public syncer::ModelTypeSyncBridge {
public:
PrintersSyncBridge(const syncer::ModelTypeStoreFactory& callback,
PrintersSyncBridge(syncer::OnceModelTypeStoreFactory callback,
const base::RepeatingClosure& error_callback);
~PrintersSyncBridge() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@ SyncedPrintersManager* SyncedPrintersManagerFactory::BuildServiceInstanceFor(
content::BrowserContext* browser_context) const {
Profile* profile = Profile::FromBrowserContext(browser_context);

const syncer::ModelTypeStoreFactory& store_factory =
syncer::OnceModelTypeStoreFactory store_factory =
browser_sync::ProfileSyncService::GetModelTypeStoreFactory(
profile->GetPath());

std::unique_ptr<PrintersSyncBridge> sync_bridge =
std::make_unique<PrintersSyncBridge>(
store_factory, base::BindRepeating(base::IgnoreResult(
&base::debug::DumpWithoutCrashing)));
std::move(store_factory), base::BindRepeating(base::IgnoreResult(
&base::debug::DumpWithoutCrashing)));

return SyncedPrintersManager::Create(profile, std::move(sync_bridge))
.release();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/sync/user_event_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ KeyedService* UserEventServiceFactory::BuildServiceInstanceFor(
return new syncer::NoOpUserEventService();
}

syncer::ModelTypeStoreFactory store_factory =
syncer::OnceModelTypeStoreFactory store_factory =
browser_sync::ProfileSyncService::GetModelTypeStoreFactory(
profile->GetPath());
syncer::ModelTypeSyncBridge::ChangeProcessorFactory processor_factory =
Expand Down
6 changes: 3 additions & 3 deletions components/browser_sync/profile_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1623,13 +1623,13 @@ void ProfileSyncService::SetPlatformSyncAllowedProvider(
}

// static
syncer::ModelTypeStoreFactory ProfileSyncService::GetModelTypeStoreFactory(
const base::FilePath& base_path) {
syncer::RepeatingModelTypeStoreFactory
ProfileSyncService::GetModelTypeStoreFactory(const base::FilePath& base_path) {
// TODO(skym): Verify using AsUTF8Unsafe is okay here. Should work as long
// as the Local State file is guaranteed to be UTF-8.
const std::string path =
FormatSharedModelTypeStorePath(base_path).AsUTF8Unsafe();
return base::Bind(&ModelTypeStore::CreateStore, path);
return base::BindRepeating(&ModelTypeStore::CreateStore, path);
}

void ProfileSyncService::ConfigureDataTypeManager() {
Expand Down
6 changes: 3 additions & 3 deletions components/browser_sync/profile_sync_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ class ProfileSyncService : public syncer::SyncServiceBase,
scoped_refptr<net::URLRequestContextGetter> url_request_context;
std::string debug_identifier;
version_info::Channel channel = version_info::Channel::UNKNOWN;
syncer::ModelTypeStoreFactory model_type_store_factory;
syncer::RepeatingModelTypeStoreFactory model_type_store_factory;

private:
DISALLOW_COPY_AND_ASSIGN(InitParams);
Expand Down Expand Up @@ -558,7 +558,7 @@ class ProfileSyncService : public syncer::SyncServiceBase,

// Returns a function that will create a ModelTypeStore that shares
// the sync LevelDB backend. |base_path| should be set to profile path.
static syncer::ModelTypeStoreFactory GetModelTypeStoreFactory(
static syncer::RepeatingModelTypeStoreFactory GetModelTypeStoreFactory(
const base::FilePath& base_path);

// Needed to test whether the directory is deleted properly.
Expand Down Expand Up @@ -885,7 +885,7 @@ class ProfileSyncService : public syncer::SyncServiceBase,
// sync bridges created by the ProfileSyncService. The default factory
// creates an on disk leveldb-backed ModelTypeStore; one might override this
// default to, e.g., use an in-memory db for unit tests.
syncer::ModelTypeStoreFactory model_type_store_factory_;
syncer::RepeatingModelTypeStoreFactory model_type_store_factory_;

// This weak factory invalidates its issued pointers when Sync is disabled.
base::WeakPtrFactory<ProfileSyncService> sync_enabled_weak_factory_;
Expand Down
3 changes: 2 additions & 1 deletion components/reading_list/core/reading_list_model_storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef COMPONENTS_READING_LIST_CORE_READING_LIST_MODEL_STORAGE_H_
#define COMPONENTS_READING_LIST_CORE_READING_LIST_MODEL_STORAGE_H_

#include <memory>
#include <vector>

#include "base/macros.h"
Expand Down Expand Up @@ -37,7 +38,7 @@ class ReadingListModelStorage : public syncer::ModelTypeSyncBridge {
// Sets the model the Storage is backing.
// This will trigger store initalization and load persistent entries.
// Pass the |clock| from the |model| to ensure synchroization when loading
// entries.
// entries. Must be called no more than once.
virtual void SetReadingListModel(ReadingListModel* model,
ReadingListStoreDelegate* delegate,
base::Clock* clock) = 0;
Expand Down
11 changes: 6 additions & 5 deletions components/reading_list/core/reading_list_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@
#include "components/sync/protocol/model_type_state.pb.h"

ReadingListStore::ReadingListStore(
StoreFactoryFunction create_store_callback,
syncer::OnceModelTypeStoreFactory create_store_callback,
const ChangeProcessorFactory& change_processor_factory)
: ReadingListModelStorage(change_processor_factory, syncer::READING_LIST),
create_store_callback_(create_store_callback),
create_store_callback_(std::move(create_store_callback)),
pending_transaction_count_(0) {}

ReadingListStore::~ReadingListStore() {
Expand All @@ -40,9 +40,10 @@ void ReadingListStore::SetReadingListModel(ReadingListModel* model,
model_ = model;
delegate_ = delegate;
clock_ = clock;
create_store_callback_.Run(
syncer::READING_LIST,
base::Bind(&ReadingListStore::OnStoreCreated, base::AsWeakPtr(this)));
std::move(create_store_callback_)
.Run(syncer::READING_LIST,
base::BindOnce(&ReadingListStore::OnStoreCreated,
base::AsWeakPtr(this)));
}

std::unique_ptr<ReadingListModelStorage::ScopedBatchUpdate>
Expand Down
8 changes: 2 additions & 6 deletions components/reading_list/core/reading_list_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,8 @@ class ReadingListModel;

// A ReadingListModelStorage storing and syncing data in protobufs.
class ReadingListStore : public ReadingListModelStorage {
using StoreFactoryFunction = base::Callback<void(
syncer::ModelType type,
const syncer::ModelTypeStore::InitCallback& callback)>;

public:
ReadingListStore(StoreFactoryFunction create_store_callback,
ReadingListStore(syncer::OnceModelTypeStoreFactory create_store_callback,
const ChangeProcessorFactory& change_processor_factory);
~ReadingListStore() override;

Expand Down Expand Up @@ -161,7 +157,7 @@ class ReadingListStore : public ReadingListModelStorage {
std::unique_ptr<syncer::ModelTypeStore> store_;
ReadingListModel* model_;
ReadingListStoreDelegate* delegate_;
StoreFactoryFunction create_store_callback_;
syncer::OnceModelTypeStoreFactory create_store_callback_;

int pending_transaction_count_;
std::unique_ptr<syncer::ModelTypeStore::WriteBatch> batch_;
Expand Down
8 changes: 4 additions & 4 deletions components/sync/device_info/device_info_sync_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ std::unique_ptr<DeviceInfoSpecifics> ModelToSpecifics(

DeviceInfoSyncBridge::DeviceInfoSyncBridge(
LocalDeviceInfoProvider* local_device_info_provider,
const ModelTypeStoreFactory& store_factory,
OnceModelTypeStoreFactory store_factory,
const ChangeProcessorFactory& change_processor_factory)
: ModelTypeSyncBridge(change_processor_factory, DEVICE_INFO),
local_device_info_provider_(local_device_info_provider) {
Expand All @@ -100,9 +100,9 @@ DeviceInfoSyncBridge::DeviceInfoSyncBridge(
base::Unretained(this)));
}

store_factory.Run(
DEVICE_INFO,
base::Bind(&DeviceInfoSyncBridge::OnStoreCreated, base::AsWeakPtr(this)));
std::move(store_factory)
.Run(DEVICE_INFO, base::BindOnce(&DeviceInfoSyncBridge::OnStoreCreated,
base::AsWeakPtr(this)));
}

DeviceInfoSyncBridge::~DeviceInfoSyncBridge() {}
Expand Down
2 changes: 1 addition & 1 deletion components/sync/device_info/device_info_sync_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class DeviceInfoSyncBridge : public ModelTypeSyncBridge,
public DeviceInfoTracker {
public:
DeviceInfoSyncBridge(LocalDeviceInfoProvider* local_device_info_provider,
const ModelTypeStoreFactory& store_factory,
OnceModelTypeStoreFactory store_factory,
const ChangeProcessorFactory& change_processor_factory);
~DeviceInfoSyncBridge() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ class DeviceInfoSyncBridgeTest : public testing::Test,
ASSERT_TRUE(store_);
bridge_ = std::make_unique<DeviceInfoSyncBridge>(
provider_.get(),
base::Bind(&ModelTypeStoreTestUtil::MoveStoreToCallback,
base::Passed(&store_)),
base::BindOnce(&ModelTypeStoreTestUtil::MoveStoreToCallback,
base::Passed(&store_)),
RecordingModelTypeChangeProcessor::FactoryForBridgeTest(&processor_));
bridge_->AddObserver(this);
}
Expand Down
34 changes: 18 additions & 16 deletions components/sync/model/mock_model_type_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,35 @@ MockModelTypeStore::MockModelTypeStore() = default;
MockModelTypeStore::~MockModelTypeStore() = default;

void MockModelTypeStore::ReadData(const IdList& id_list,
const ReadDataCallback& callback) {
ReadDataCallback callback) {
if (!read_data_handler_.is_null()) {
read_data_handler_.Run(id_list, callback);
read_data_handler_.Run(id_list, std::move(callback));
} else {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, Result::SUCCESS,
base::Passed(std::unique_ptr<RecordList>()),
base::Passed(std::unique_ptr<IdList>())));
FROM_HERE, base::BindOnce(std::move(callback), Result::SUCCESS,
std::unique_ptr<RecordList>(),
std::unique_ptr<IdList>()));
}
}

void MockModelTypeStore::ReadAllData(const ReadAllDataCallback& callback) {
void MockModelTypeStore::ReadAllData(ReadAllDataCallback callback) {
if (!read_all_data_handler_.is_null()) {
read_all_data_handler_.Run(callback);
read_all_data_handler_.Run(std::move(callback));
} else {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, Result::SUCCESS,
base::Passed(std::unique_ptr<RecordList>())));
FROM_HERE, base::BindOnce(std::move(callback), Result::SUCCESS,
std::unique_ptr<RecordList>()));
}
}

void MockModelTypeStore::ReadAllMetadata(const ReadMetadataCallback& callback) {
void MockModelTypeStore::ReadAllMetadata(ReadMetadataCallback callback) {
if (!read_all_metadata_handler_.is_null()) {
read_all_metadata_handler_.Run(callback);
read_all_metadata_handler_.Run(std::move(callback));
} else {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, base::Optional<ModelError>(),
base::Passed(std::unique_ptr<MetadataBatch>())));
FROM_HERE,
base::BindOnce(std::move(callback), base::Optional<ModelError>(),
std::unique_ptr<MetadataBatch>()));
}
}

Expand All @@ -59,12 +60,13 @@ MockModelTypeStore::CreateWriteBatch() {

void MockModelTypeStore::CommitWriteBatch(
std::unique_ptr<WriteBatch> write_batch,
const CallbackWithResult& callback) {
CallbackWithResult callback) {
if (!commit_write_batch_handler_.is_null()) {
commit_write_batch_handler_.Run(std::move(write_batch), callback);
commit_write_batch_handler_.Run(std::move(write_batch),
std::move(callback));
} else {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, Result::SUCCESS));
FROM_HERE, base::BindOnce(std::move(callback), Result::SUCCESS));
}
}

Expand Down
29 changes: 15 additions & 14 deletions components/sync/model/mock_model_type_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace syncer {
// Here is an example:
// ===
// void OnReadData(const ModelTypeStore::IdList& id_list,
// const ModelTypeStore::ReadAllDataCallback& callback) {
// ModelTypeStore::ReadAllDataCallback callback) {
// // Verify id_list here.
// // Prepare fake response.
// std::unique_ptr<ModelTypeStore::RecordList> record_list(
Expand All @@ -44,33 +44,34 @@ namespace syncer {
class MockModelTypeStore : public ModelTypeStore {
public:
// Signatures for all ModelTypeStore virtual functions.
using ReadAllDataSignature = base::Callback<void(const ReadAllDataCallback&)>;
using ReadAllDataSignature =
base::RepeatingCallback<void(ReadAllDataCallback)>;
using ReadDataSignature =
base::Callback<void(const IdList&, const ReadDataCallback&)>;
base::RepeatingCallback<void(const IdList&, ReadDataCallback)>;
using ReadAllMetadataSignature =
base::Callback<void(const ReadMetadataCallback& callback)>;
base::RepeatingCallback<void(ReadMetadataCallback callback)>;
using CommitWriteBatchSignature =
base::Callback<void(std::unique_ptr<WriteBatch>, CallbackWithResult)>;
using WriteRecordSignature =
base::Callback<void(WriteBatch*, const std::string&, const std::string&)>;
base::RepeatingCallback<void(std::unique_ptr<WriteBatch>,
CallbackWithResult)>;
using WriteRecordSignature = base::RepeatingCallback<
void(WriteBatch*, const std::string&, const std::string&)>;
using WriteGlobalMetadataSignature =
base::Callback<void(WriteBatch*, const std::string&)>;
base::RepeatingCallback<void(WriteBatch*, const std::string&)>;
using DeleteRecordSignature =
base::Callback<void(WriteBatch*, const std::string&)>;
base::RepeatingCallback<void(WriteBatch*, const std::string&)>;
using DeleteGlobalMetadataSignature = base::Callback<void(WriteBatch*)>;

MockModelTypeStore();
~MockModelTypeStore() override;

// ModelTypeStore implementation.
void ReadData(const IdList& id_list,
const ReadDataCallback& callback) override;
void ReadAllData(const ReadAllDataCallback& callback) override;
void ReadAllMetadata(const ReadMetadataCallback& callback) override;
void ReadData(const IdList& id_list, ReadDataCallback callback) override;
void ReadAllData(ReadAllDataCallback callback) override;
void ReadAllMetadata(ReadMetadataCallback callback) override;

std::unique_ptr<WriteBatch> CreateWriteBatch() override;
void CommitWriteBatch(std::unique_ptr<WriteBatch> write_batch,
const CallbackWithResult& callback) override;
CallbackWithResult callback) override;

// Register handler functions.
void RegisterReadDataHandler(const ReadDataSignature& handler);
Expand Down
10 changes: 6 additions & 4 deletions components/sync/model/model_type_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "components/sync/model/model_type_store.h"

#include <utility>

#include "components/sync/model_impl/accumulating_metadata_change_list.h"
#include "components/sync/model_impl/model_type_store_impl.h"
#include "components/sync/model_impl/passthrough_metadata_change_list.h"
Expand All @@ -12,15 +14,15 @@ namespace syncer {

// static
void ModelTypeStore::CreateInMemoryStoreForTest(ModelType type,
const InitCallback& callback) {
ModelTypeStoreImpl::CreateInMemoryStoreForTest(type, callback);
InitCallback callback) {
ModelTypeStoreImpl::CreateInMemoryStoreForTest(type, std::move(callback));
}

// static
void ModelTypeStore::CreateStore(const std::string& path,
ModelType type,
const InitCallback& callback) {
ModelTypeStoreImpl::CreateStore(type, path, callback);
InitCallback callback) {
ModelTypeStoreImpl::CreateStore(type, path, std::move(callback));
}

ModelTypeStore::~ModelTypeStore() {}
Expand Down
Loading

0 comments on commit 6bd0344

Please sign in to comment.