Commit 4e0349a6 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

Delete ModelSafeWorker::GROUP_FILE and GROUP_HISTORY

The corresponding data types have been migrated to USS, which uses
GROUP_NON_BLOCKING (which essentially means "don't use a
ModelSafeWorker at all"). So the workers for FILE and HISTORY were
still instantiated, but never actually used.
This also lets us remove the whole HistoryModelWorker class.

TBR=sky
for trivial updates to components/history/core/browser/BUILD.gn
(removing the deleted sync files).

Bug: 923287
Change-Id: Ief4ed46fce41346ea35ea46535b8d06d6c4d212e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1720709
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#681746}
parent b3686d71
......@@ -60,7 +60,6 @@
#include "components/consent_auditor/consent_auditor.h"
#include "components/dom_distiller/core/dom_distiller_service.h"
#include "components/history/core/browser/history_service.h"
#include "components/history/core/browser/sync/history_model_worker.h"
#include "components/history/core/common/pref_names.h"
#include "components/invalidation/impl/invalidation_switches.h"
#include "components/invalidation/impl/profile_invalidation_provider.h"
......@@ -574,28 +573,11 @@ ChromeSyncClient::CreateModelWorkerForGroup(syncer::ModelSafeGroup group) {
case syncer::GROUP_DB:
return new syncer::SequencedModelWorker(web_data_service_thread_,
syncer::GROUP_DB);
// TODO(stanisc): crbug.com/731903: Rename GROUP_FILE to reflect that it is
// used only for app and extension settings.
case syncer::GROUP_FILE:
#if BUILDFLAG(ENABLE_EXTENSIONS)
return new syncer::SequencedModelWorker(
extensions::GetBackendTaskRunner(), syncer::GROUP_FILE);
#else
return nullptr;
#endif // BUILDFLAG(ENABLE_EXTENSIONS)
case syncer::GROUP_UI:
return new syncer::UIModelWorker(
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI}));
case syncer::GROUP_PASSIVE:
return new syncer::PassiveModelWorker();
case syncer::GROUP_HISTORY: {
history::HistoryService* history_service = GetHistoryService();
if (!history_service)
return nullptr;
return new HistoryModelWorker(
history_service->AsWeakPtr(),
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI}));
}
case syncer::GROUP_PASSWORD: {
if (!password_store_.get())
return nullptr;
......
......@@ -52,8 +52,6 @@ static_library("browser") {
"sync/delete_directive_handler.h",
"sync/history_delete_directives_model_type_controller.cc",
"sync/history_delete_directives_model_type_controller.h",
"sync/history_model_worker.cc",
"sync/history_model_worker.h",
"sync/typed_url_model_type_controller.cc",
"sync/typed_url_model_type_controller.h",
"sync/typed_url_sync_bridge.cc",
......@@ -204,7 +202,6 @@ source_set("unit_tests") {
"history_service_unittest.cc",
"history_types_unittest.cc",
"sync/delete_directive_handler_unittest.cc",
"sync/history_model_worker_unittest.cc",
"sync/typed_url_sync_bridge_unittest.cc",
"sync/typed_url_sync_metadata_database_unittest.cc",
"thumbnail_database_unittest.cc",
......
// Copyright 2012 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/history/core/browser/sync/history_model_worker.h"
#include <memory>
#include <utility>
#include "base/bind.h"
#include "components/history/core/browser/history_db_task.h"
#include "components/history/core/browser/history_service.h"
namespace browser_sync {
class WorkerTask : public history::HistoryDBTask {
public:
explicit WorkerTask(base::OnceClosure work) : work_(std::move(work)) {}
bool RunOnDBThread(history::HistoryBackend* backend,
history::HistoryDatabase* db) override {
std::move(work_).Run();
return true;
}
// Since the DoWorkAndWaitUntilDone() is synchronous, we don't need to run
// any code asynchronously on the main thread after completion.
void DoneRunOnMainThread() override {}
private:
// A OnceClosure is deleted right after it runs. This is important to unblock
// DoWorkAndWaitUntilDone() right after the task runs.
base::OnceClosure work_;
DISALLOW_COPY_AND_ASSIGN(WorkerTask);
};
namespace {
// Post the work task on |history_service|'s DB thread from the UI
// thread.
void PostWorkerTask(
const base::WeakPtr<history::HistoryService>& history_service,
base::OnceClosure work,
base::CancelableTaskTracker* cancelable_tracker) {
if (history_service) {
history_service->ScheduleDBTask(
FROM_HERE, std::make_unique<WorkerTask>(std::move(work)),
cancelable_tracker);
}
}
} // namespace
HistoryModelWorker::HistoryModelWorker(
const base::WeakPtr<history::HistoryService>& history_service,
scoped_refptr<base::SingleThreadTaskRunner> ui_thread)
: history_service_(history_service),
ui_thread_(std::move(ui_thread)),
// base::OnTaskRunnerDeleter ensures that |cancelable_tracker_| is
// destroyed on the UI thread, even if |this| is deleted on a different
// thread. It is a requirement of base::CancelableTaskTracker to be
// constructed and deleted on the same sequence.
cancelable_tracker_(new base::CancelableTaskTracker,
base::OnTaskRunnerDeleter(ui_thread_)) {
CHECK(history_service);
DCHECK(ui_thread_->BelongsToCurrentThread());
}
syncer::ModelSafeGroup HistoryModelWorker::GetModelSafeGroup() {
return syncer::GROUP_HISTORY;
}
bool HistoryModelWorker::IsOnModelSequence() {
// Ideally HistoryService would expose a way to check whether this is the
// history DB thread. Since it doesn't, just return true to bypass a CHECK in
// the sync code.
return true;
}
HistoryModelWorker::~HistoryModelWorker() = default;
void HistoryModelWorker::ScheduleWork(base::OnceClosure work) {
ui_thread_->PostTask(
FROM_HERE, base::BindOnce(&PostWorkerTask, history_service_,
std::move(work), cancelable_tracker_.get()));
}
} // namespace browser_sync
// Copyright 2012 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_HISTORY_CORE_BROWSER_SYNC_HISTORY_MODEL_WORKER_H_
#define COMPONENTS_HISTORY_CORE_BROWSER_SYNC_HISTORY_MODEL_WORKER_H_
#include <memory>
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/single_thread_task_runner.h"
#include "base/task/cancelable_task_tracker.h"
#include "components/sync/engine/model_safe_worker.h"
namespace history {
class HistoryService;
}
namespace browser_sync {
// A syncer::ModelSafeWorker for history models that accepts requests
// from the syncapi that need to be fulfilled on the history thread.
class HistoryModelWorker : public syncer::ModelSafeWorker {
public:
explicit HistoryModelWorker(
const base::WeakPtr<history::HistoryService>& history_service,
scoped_refptr<base::SingleThreadTaskRunner> ui_thread);
// syncer::ModelSafeWorker implementation.
syncer::ModelSafeGroup GetModelSafeGroup() override;
bool IsOnModelSequence() override;
private:
~HistoryModelWorker() override;
void ScheduleWork(base::OnceClosure work) override;
const base::WeakPtr<history::HistoryService> history_service_;
// A reference to the UI thread's task runner.
const scoped_refptr<base::SingleThreadTaskRunner> ui_thread_;
// Helper object to make sure we don't leave tasks running on the history
// thread.
const std::unique_ptr<base::CancelableTaskTracker, base::OnTaskRunnerDeleter>
cancelable_tracker_;
DISALLOW_COPY_AND_ASSIGN(HistoryModelWorker);
};
} // namespace browser_sync
#endif // COMPONENTS_HISTORY_CORE_BROWSER_SYNC_HISTORY_MODEL_WORKER_H_
// 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 "components/history/core/browser/sync/history_model_worker.h"
#include <memory>
#include <utility>
#include "base/bind.h"
#include "base/macros.h"
#include "base/single_thread_task_runner.h"
#include "base/synchronization/atomic_flag.h"
#include "base/test/test_simple_task_runner.h"
#include "base/test/test_timeouts.h"
#include "base/threading/platform_thread.h"
#include "base/threading/thread.h"
#include "components/history/core/browser/history_db_task.h"
#include "components/history/core/browser/history_service.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace browser_sync {
namespace {
class HistoryServiceMock : public history::HistoryService {
public:
explicit HistoryServiceMock(
scoped_refptr<base::SingleThreadTaskRunner> history_thread)
: history_thread_(std::move(history_thread)) {}
base::CancelableTaskTracker::TaskId ScheduleDBTask(
const base::Location& from_here,
std::unique_ptr<history::HistoryDBTask> task,
base::CancelableTaskTracker* tracker) override {
history::HistoryDBTask* task_raw = task.get();
history_thread_->PostTaskAndReply(
from_here,
base::BindOnce(
base::IgnoreResult(&history::HistoryDBTask::RunOnDBThread),
base::Unretained(task_raw), nullptr, nullptr),
base::BindOnce(&history::HistoryDBTask::DoneRunOnMainThread,
std::move(task)));
return base::CancelableTaskTracker::kBadTaskId; // Unused.
}
private:
const scoped_refptr<base::SingleThreadTaskRunner> history_thread_;
DISALLOW_COPY_AND_ASSIGN(HistoryServiceMock);
};
syncer::WorkCallback ClosureToWorkCallback(base::Closure work) {
return base::BindOnce(
[](base::Closure work) {
work.Run();
return syncer::SyncerError(syncer::SyncerError::SYNCER_OK);
},
std::move(work));
}
class HistoryModelWorkerTest : public testing::Test {
public:
HistoryModelWorkerTest()
: sync_thread_("SyncThreadForTest"),
history_service_(history_thread_),
history_service_factory_(&history_service_) {
sync_thread_.Start();
worker_ = new HistoryModelWorker(history_service_factory_.GetWeakPtr(),
ui_thread_);
}
~HistoryModelWorkerTest() override {
// Run tasks that might still have a reference to |worker_|.
ui_thread_->RunUntilIdle();
history_thread_->RunUntilIdle();
// Release the last reference to |worker_|.
EXPECT_TRUE(worker_->HasOneRef());
worker_ = nullptr;
// Run the DeleteSoon() task posted from ~HistoryModelWorker. This prevents
// a leak.
ui_thread_->RunUntilIdle();
}
protected:
void DoWorkAndWaitUntilDoneOnSyncThread(base::Closure work) {
sync_thread_.task_runner()->PostTask(
FROM_HERE,
base::BindOnce(
base::IgnoreResult(&HistoryModelWorker::DoWorkAndWaitUntilDone),
worker_, ClosureToWorkCallback(work)));
sync_thread_.task_runner()->PostTask(
FROM_HERE, base::BindOnce(&base::AtomicFlag::Set,
base::Unretained(&sync_thread_unblocked_)));
}
const scoped_refptr<base::TestSimpleTaskRunner> ui_thread_ =
new base::TestSimpleTaskRunner();
scoped_refptr<base::TestSimpleTaskRunner> history_thread_ =
new base::TestSimpleTaskRunner();
base::AtomicFlag sync_thread_unblocked_;
base::Thread sync_thread_;
HistoryServiceMock history_service_;
scoped_refptr<HistoryModelWorker> worker_;
private:
base::WeakPtrFactory<HistoryServiceMock> history_service_factory_;
DISALLOW_COPY_AND_ASSIGN(HistoryModelWorkerTest);
};
} // namespace
TEST_F(HistoryModelWorkerTest, DoWorkAndWaitUntilDone) {
bool did_work = false;
DoWorkAndWaitUntilDoneOnSyncThread(base::Bind(
[](bool* did_work) { *did_work = true; }, base::Unretained(&did_work)));
EXPECT_FALSE(did_work);
EXPECT_FALSE(sync_thread_unblocked_.IsSet());
// Wait for a task to be posted to the UI thread and run it. Expect this task
// to post another task to the history DB thread and run it.
while (!ui_thread_->HasPendingTask())
base::PlatformThread::YieldCurrentThread();
ui_thread_->RunUntilIdle();
EXPECT_TRUE(history_thread_->HasPendingTask());
history_thread_->RunUntilIdle();
EXPECT_TRUE(did_work);
sync_thread_.Stop();
EXPECT_TRUE(sync_thread_unblocked_.IsSet());
}
TEST_F(HistoryModelWorkerTest, DoWorkAndWaitUntilDoneRequestStopBeforeRunWork) {
bool did_work = false;
DoWorkAndWaitUntilDoneOnSyncThread(base::Bind(
[](bool* did_work) { *did_work = true; }, base::Unretained(&did_work)));
EXPECT_FALSE(did_work);
EXPECT_FALSE(sync_thread_unblocked_.IsSet());
// Wait for a task to be posted to the UI thread and run it.
while (!ui_thread_->HasPendingTask())
base::PlatformThread::YieldCurrentThread();
ui_thread_->RunUntilIdle();
// Stop the worker.
worker_->RequestStop();
// The WorkCallback should not run on the history DB thread.
EXPECT_TRUE(history_thread_->HasPendingTask());
history_thread_->RunUntilIdle();
EXPECT_FALSE(did_work);
sync_thread_.Stop();
EXPECT_TRUE(sync_thread_unblocked_.IsSet());
}
TEST_F(HistoryModelWorkerTest,
DoWorkAndWaitUntilDoneRequestStopBeforeUITaskRun) {
bool did_work = false;
DoWorkAndWaitUntilDoneOnSyncThread(base::Bind(
[](bool* did_work) { *did_work = true; }, base::Unretained(&did_work)));
EXPECT_FALSE(did_work);
EXPECT_FALSE(sync_thread_unblocked_.IsSet());
// Wait for a task to be posted to the UI thread.
while (!ui_thread_->HasPendingTask())
base::PlatformThread::YieldCurrentThread();
// Stop the worker.
worker_->RequestStop();
// Stopping the worker should unblock the sync thread.
sync_thread_.Stop();
EXPECT_TRUE(sync_thread_unblocked_.IsSet());
}
TEST_F(HistoryModelWorkerTest, DoWorkAndWaitUntilDoneDeleteWorkBeforeRun) {
bool did_work = false;
DoWorkAndWaitUntilDoneOnSyncThread(base::Bind(
[](bool* did_work) { *did_work = true; }, base::Unretained(&did_work)));
EXPECT_FALSE(did_work);
EXPECT_FALSE(sync_thread_unblocked_.IsSet());
// Wait for a task to be posted to the UI thread. Delete it before it can run.
while (!ui_thread_->HasPendingTask())
base::PlatformThread::YieldCurrentThread();
ui_thread_->ClearPendingTasks();
EXPECT_FALSE(did_work);
// Deleting the task should have unblocked the sync thread.
sync_thread_.Stop();
EXPECT_TRUE(sync_thread_unblocked_.IsSet());
}
TEST_F(HistoryModelWorkerTest, DoWorkAndWaitUntilDoneRequestStopDuringRunWork) {
bool did_work = false;
DoWorkAndWaitUntilDoneOnSyncThread(base::Bind(
[](scoped_refptr<HistoryModelWorker> worker,
base::AtomicFlag* sync_thread_unblocked, bool* did_work) {
worker->RequestStop();
base::PlatformThread::Sleep(TestTimeouts::tiny_timeout());
// The sync thread should not be unblocked while a WorkCallback is
// running.
EXPECT_FALSE(sync_thread_unblocked->IsSet());
*did_work = true;
},
worker_, base::Unretained(&sync_thread_unblocked_),
base::Unretained(&did_work)));
EXPECT_FALSE(did_work);
EXPECT_FALSE(sync_thread_unblocked_.IsSet());
// Wait for a task to be posted to the UI thread and run it.
while (!ui_thread_->HasPendingTask())
base::PlatformThread::YieldCurrentThread();
ui_thread_->RunUntilIdle();
// Expect a task to be posted to the history DB thread. Run it.
EXPECT_TRUE(history_thread_->HasPendingTask());
history_thread_->RunUntilIdle();
EXPECT_TRUE(did_work);
sync_thread_.Stop();
EXPECT_TRUE(sync_thread_unblocked_.IsSet());
}
} // namespace browser_sync
......@@ -54,10 +54,6 @@ std::string ModelSafeGroupToString(ModelSafeGroup group) {
return "Group UI";
case GROUP_DB:
return "Group DB";
case GROUP_FILE:
return "Group File";
case GROUP_HISTORY:
return "Group History";
case GROUP_PASSIVE:
return "Group Passive";
case GROUP_PASSWORD:
......
......@@ -32,9 +32,6 @@ enum ModelSafeGroup {
// native model.
GROUP_UI, // Models that live on UI thread and are being synced.
GROUP_DB, // Models that live on DB thread and are being synced.
GROUP_FILE, // Models that live on FILE thread and are being synced.
GROUP_HISTORY, // Models that live on history thread and are being
// synced.
GROUP_PASSWORD, // Models that live on the password thread and are
// being synced. On windows and linux, this runs on the
// DB thread.
......
......@@ -20,10 +20,8 @@ SyncBackendRegistrar::SyncBackendRegistrar(
: name_(name) {
DCHECK(!worker_factory.is_null());
MaybeAddWorker(worker_factory, GROUP_DB);
MaybeAddWorker(worker_factory, GROUP_FILE);
MaybeAddWorker(worker_factory, GROUP_UI);
MaybeAddWorker(worker_factory, GROUP_PASSIVE);
MaybeAddWorker(worker_factory, GROUP_HISTORY);
MaybeAddWorker(worker_factory, GROUP_PASSWORD);
}
......@@ -61,12 +59,6 @@ void SyncBackendRegistrar::SetInitialTypes(ModelTypeSet initial_types) {
}
}
if (!workers_.count(GROUP_HISTORY)) {
LOG_IF(WARNING, initial_types.Has(TYPED_URLS))
<< "History store disabled, cannot sync Omnibox History";
routing_info_.erase(TYPED_URLS);
}
if (!workers_.count(GROUP_PASSWORD)) {
LOG_IF(WARNING, initial_types.Has(PASSWORDS))
<< "Password store not initialized, cannot sync passwords";
......@@ -99,10 +91,6 @@ ModelTypeSet SyncBackendRegistrar::ConfigureDataTypes(
ModelTypeSet types_to_remove) {
DCHECK(Intersection(types_to_add, types_to_remove).Empty());
ModelTypeSet filtered_types_to_add = types_to_add;
if (workers_.count(GROUP_HISTORY) == 0) {
LOG(WARNING) << "No history worker -- removing TYPED_URLS";
filtered_types_to_add.Remove(TYPED_URLS);
}
if (workers_.count(GROUP_PASSWORD) == 0) {
LOG(WARNING) << "No password worker -- removing PASSWORDS";
filtered_types_to_add.Remove(PASSWORDS);
......
......@@ -30,12 +30,10 @@ class SyncBackendRegistrarTest : public testing::Test {
public:
SyncBackendRegistrarTest()
: db_thread_("DBThreadForTest"),
file_thread_("FileThreadForTest"),
sync_thread_("SyncThreadForTest") {}
void SetUp() override {
db_thread_.StartAndWaitForTesting();
file_thread_.StartAndWaitForTesting();
sync_thread_.StartAndWaitForTesting();
test_user_share_.SetUp();
registrar_ = std::make_unique<SyncBackendRegistrar>(
......@@ -100,8 +98,6 @@ class SyncBackendRegistrarTest : public testing::Test {
task_environment_.GetMainThreadTaskRunner(), group);
case GROUP_DB:
return new SequencedModelWorker(db_thread_.task_runner(), group);
case GROUP_FILE:
return new SequencedModelWorker(file_thread_.task_runner(), group);
case GROUP_PASSIVE:
return new PassiveModelWorker();
default:
......@@ -111,7 +107,6 @@ class SyncBackendRegistrarTest : public testing::Test {
base::test::ScopedTaskEnvironment task_environment_;
base::Thread db_thread_;
base::Thread file_thread_;
base::Thread sync_thread_;
TestUserShare test_user_share_;
......@@ -121,7 +116,7 @@ class SyncBackendRegistrarTest : public testing::Test {
TEST_F(SyncBackendRegistrarTest, ConstructorEmpty) {
registrar()->SetInitialTypes(ModelTypeSet());
EXPECT_FALSE(registrar()->IsNigoriEnabled());
EXPECT_EQ(4u, GetWorkersSize());
EXPECT_EQ(3u, GetWorkersSize());
ExpectRoutingInfo(ModelSafeRoutingInfo());
ExpectHasProcessorsForTypes(ModelTypeSet());
}
......@@ -130,7 +125,7 @@ TEST_F(SyncBackendRegistrarTest, ConstructorNonEmpty) {
registrar()->RegisterNonBlockingType(BOOKMARKS);
registrar()->SetInitialTypes(ModelTypeSet(BOOKMARKS, NIGORI, PASSWORDS));
EXPECT_TRUE(registrar()->IsNigoriEnabled());
EXPECT_EQ(4u, GetWorkersSize());
EXPECT_EQ(3u, GetWorkersSize());
EXPECT_EQ(ModelTypeSet(NIGORI), registrar()->GetLastConfiguredTypes());
// Bookmarks dropped because it is nonblocking.
// Passwords dropped because of no password store.
......@@ -143,7 +138,7 @@ TEST_F(SyncBackendRegistrarTest, ConstructorNonEmptyReversedInitialization) {
registrar()->SetInitialTypes(ModelTypeSet(BOOKMARKS, NIGORI, PASSWORDS));
registrar()->RegisterNonBlockingType(BOOKMARKS);
EXPECT_TRUE(registrar()->IsNigoriEnabled());
EXPECT_EQ(4u, GetWorkersSize());
EXPECT_EQ(3u, GetWorkersSize());
EXPECT_EQ(ModelTypeSet(NIGORI), registrar()->GetLastConfiguredTypes());
// Bookmarks dropped because it is nonblocking.
// Passwords dropped because of no password store.
......
......@@ -143,7 +143,7 @@ TEST_F(ModelTypeRegistryTest, DirectoryTypes) {
// Try registering type with unknown worker.
EXPECT_DCHECK_DEATH(
registry()->RegisterDirectoryType(SESSIONS, GROUP_HISTORY));
registry()->RegisterDirectoryType(SESSIONS, GROUP_PASSWORD));
}
TEST_F(ModelTypeRegistryTest, NonBlockingTypes) {
......
......@@ -24,7 +24,6 @@
#include "components/consent_auditor/consent_auditor.h"
#include "components/dom_distiller/core/dom_distiller_service.h"
#include "components/history/core/browser/history_service.h"
#include "components/history/core/browser/sync/history_model_worker.h"
#include "components/history/core/browser/sync/typed_url_sync_bridge.h"
#include "components/invalidation/impl/invalidation_switches.h"
#include "components/invalidation/impl/profile_invalidation_provider.h"
......@@ -303,22 +302,11 @@ IOSChromeSyncClient::CreateModelWorkerForGroup(syncer::ModelSafeGroup group) {
switch (group) {
case syncer::GROUP_DB:
return new syncer::SequencedModelWorker(db_thread_, syncer::GROUP_DB);
case syncer::GROUP_FILE:
// Not supported on iOS.
return nullptr;
case syncer::GROUP_UI:
return new syncer::UIModelWorker(
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::UI}));
case syncer::GROUP_PASSIVE:
return new syncer::PassiveModelWorker();
case syncer::GROUP_HISTORY: {
history::HistoryService* history_service = GetHistoryService();
if (!history_service)
return nullptr;
return new browser_sync::HistoryModelWorker(
history_service->AsWeakPtr(),
base::CreateSingleThreadTaskRunnerWithTraits({web::WebThread::UI}));
}
case syncer::GROUP_PASSWORD: {
if (!password_store_)
return nullptr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment