0

[component updater] Introduce scheduler interface

This is a stepping stone to implement a platform-specific scheduler.

Bug: 801229
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9da94fc261a0834265e846069a0a6e3c4e51cebf
Reviewed-on: https://chromium-review.googlesource.com/1097838
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569389}
This commit is contained in:
Tibor Goldschwendt
2018-06-21 22:50:40 +00:00
committed by Commit Bot
parent 48d7548720
commit 5f173cb15b
11 changed files with 261 additions and 38 deletions

@@ -88,6 +88,7 @@
#include "chrome/common/url_constants.h"
#include "chrome/installer/util/google_update_settings.h"
#include "components/component_updater/component_updater_service.h"
#include "components/component_updater/timer_update_scheduler.h"
#include "components/crash/core/common/crash_key.h"
#include "components/gcm_driver/gcm_driver.h"
#include "components/metrics/metrics_pref_names.h"
@@ -1002,7 +1003,8 @@ BrowserProcessImpl::component_updater() {
component_updater_ = component_updater::ComponentUpdateServiceFactory(
component_updater::MakeChromeComponentUpdaterConfigurator(
base::CommandLine::ForCurrentProcess(),
g_browser_process->local_state()));
g_browser_process->local_state()),
std::make_unique<component_updater::TimerUpdateScheduler>());
return component_updater_.get();
}

@@ -23,6 +23,9 @@ static_library("component_updater") {
"pref_names.h",
"timer.cc",
"timer.h",
"timer_update_scheduler.cc",
"timer_update_scheduler.h",
"update_scheduler.h",
]
deps = [

@@ -168,6 +168,16 @@ class MockInstallerPolicy : public ComponentInstallerPolicy {
}
};
class MockUpdateScheduler : public UpdateScheduler {
public:
MOCK_METHOD4(Schedule,
void(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UserTask& user_task,
const OnStopTaskCallback& on_stop));
MOCK_METHOD0(Stop, void());
};
class ComponentInstallerTest : public testing::Test {
public:
ComponentInstallerTest();
@@ -179,6 +189,7 @@ class ComponentInstallerTest : public testing::Test {
}
scoped_refptr<TestConfigurator> configurator() const { return config_; }
base::OnceClosure quit_closure() { return runloop_.QuitClosure(); }
MockUpdateScheduler& scheduler() { return *scheduler_; }
protected:
void RunThreads();
@@ -189,6 +200,10 @@ class ComponentInstallerTest : public testing::Test {
private:
void UnpackComplete(const ComponentUnpacker::Result& result);
void Schedule(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UpdateScheduler::UserTask& user_task,
const UpdateScheduler::OnStopTaskCallback& on_stop);
const scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_ =
base::ThreadTaskRunnerHandle::Get();
@@ -196,6 +211,7 @@ class ComponentInstallerTest : public testing::Test {
scoped_refptr<TestConfigurator> config_ =
base::MakeRefCounted<TestConfigurator>();
MockUpdateScheduler* scheduler_ = nullptr;
scoped_refptr<MockUpdateClient> update_client_ =
base::MakeRefCounted<MockUpdateClient>();
std::unique_ptr<ComponentUpdateService> component_updater_;
@@ -204,8 +220,12 @@ class ComponentInstallerTest : public testing::Test {
ComponentInstallerTest::ComponentInstallerTest() {
EXPECT_CALL(update_client(), AddObserver(_)).Times(1);
component_updater_ =
std::make_unique<CrxUpdateService>(config_, update_client_);
auto scheduler = std::make_unique<MockUpdateScheduler>();
scheduler_ = scheduler.get();
ON_CALL(*scheduler_, Schedule(_, _, _, _))
.WillByDefault(Invoke(this, &ComponentInstallerTest::Schedule));
component_updater_ = std::make_unique<CrxUpdateService>(
config_, std::move(scheduler), update_client_);
}
ComponentInstallerTest::~ComponentInstallerTest() {
@@ -237,6 +257,14 @@ void ComponentInstallerTest::UnpackComplete(
main_thread_task_runner_->PostTask(FROM_HERE, quit_closure());
}
void ComponentInstallerTest::Schedule(
const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UpdateScheduler::UserTask& user_task,
const UpdateScheduler::OnStopTaskCallback& on_stop) {
user_task.Run(base::DoNothing());
}
} // namespace
// Tests that the component metadata is propagated from the component installer
@@ -272,6 +300,8 @@ TEST_F(ComponentInstallerTest, RegisterComponent) {
EXPECT_CALL(update_client(), GetCrxUpdateState(id, _)).Times(1);
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Schedule(_, _, _, _)).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
auto installer = base::MakeRefCounted<ComponentInstaller>(
std::make_unique<MockInstallerPolicy>());
@@ -324,6 +354,7 @@ TEST_F(ComponentInstallerTest, UnpackPathInstallSuccess) {
EXPECT_FALSE(base::PathExists(unpack_path));
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
}
// Tests that the unpack path is removed when the install failed.
@@ -354,6 +385,7 @@ TEST_F(ComponentInstallerTest, UnpackPathInstallError) {
EXPECT_FALSE(base::PathExists(unpack_path));
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
}
} // namespace component_updater

@@ -23,7 +23,6 @@
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "components/component_updater/component_updater_service_internal.h"
#include "components/component_updater/timer.h"
#include "components/update_client/configurator.h"
#include "components/update_client/crx_update_item.h"
#include "components/update_client/update_client.h"
@@ -56,8 +55,11 @@ ComponentInfo::ComponentInfo(ComponentInfo&& other) = default;
ComponentInfo::~ComponentInfo() {}
CrxUpdateService::CrxUpdateService(scoped_refptr<Configurator> config,
std::unique_ptr<UpdateScheduler> scheduler,
scoped_refptr<UpdateClient> update_client)
: config_(config), update_client_(update_client) {
: config_(config),
scheduler_(std::move(scheduler)),
update_client_(update_client) {
AddObserver(this);
}
@@ -91,18 +93,20 @@ void CrxUpdateService::Start() {
<< "Next update attempt will take place in "
<< config_->NextCheckDelay() << " seconds. ";
timer_.Start(
scheduler_->Schedule(
base::TimeDelta::FromSeconds(config_->InitialDelay()),
base::TimeDelta::FromSeconds(config_->NextCheckDelay()),
base::Bind(base::IgnoreResult(&CrxUpdateService::CheckForUpdates),
base::Unretained(this)));
base::Unretained(this)),
// TODO: Stop component update if requested.
base::DoNothing());
}
// Stops the update loop. In flight operations will be completed.
void CrxUpdateService::Stop() {
DCHECK(thread_checker_.CalledOnValidThread());
VLOG(1) << "CrxUpdateService stopping";
timer_.Stop();
scheduler_->Stop();
update_client_->Stop();
}
@@ -299,7 +303,8 @@ void CrxUpdateService::OnDemandUpdateInternal(const std::string& id,
base::TimeTicks::Now()));
}
bool CrxUpdateService::CheckForUpdates() {
bool CrxUpdateService::CheckForUpdates(
UpdateScheduler::OnFinishedCallback on_finished) {
DCHECK(thread_checker_.CalledOnValidThread());
// TODO(xiaochu): remove this log after https://crbug.com/851151 is fixed.
@@ -320,24 +325,38 @@ bool CrxUpdateService::CheckForUpdates() {
unsecure_ids.push_back(id);
}
if (unsecure_ids.empty() && secure_ids.empty()) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
std::move(on_finished));
return true;
}
Callback on_finished_callback = base::BindOnce(
[](UpdateScheduler::OnFinishedCallback on_finished,
update_client::Error error) { std::move(on_finished).Run(); },
std::move(on_finished));
if (!unsecure_ids.empty()) {
update_client_->Update(unsecure_ids,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
false,
base::BindOnce(&CrxUpdateService::OnUpdateComplete,
base::Unretained(this), Callback(),
base::TimeTicks::Now()));
update_client_->Update(
unsecure_ids,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
false,
base::BindOnce(
&CrxUpdateService::OnUpdateComplete, base::Unretained(this),
secure_ids.empty() ? std::move(on_finished_callback) : Callback(),
base::TimeTicks::Now()));
}
if (!secure_ids.empty()) {
update_client_->Update(secure_ids,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
false,
base::BindOnce(&CrxUpdateService::OnUpdateComplete,
base::Unretained(this), Callback(),
base::TimeTicks::Now()));
update_client_->Update(
secure_ids,
base::BindOnce(&CrxUpdateService::GetCrxComponents,
base::Unretained(this)),
false,
base::BindOnce(&CrxUpdateService::OnUpdateComplete,
base::Unretained(this), std::move(on_finished_callback),
base::TimeTicks::Now()));
}
return true;
@@ -440,10 +459,13 @@ void CrxUpdateService::OnEvent(Events event, const std::string& id) {
// is the job of the browser process.
// TODO(sorin): consider making this a singleton.
std::unique_ptr<ComponentUpdateService> ComponentUpdateServiceFactory(
scoped_refptr<Configurator> config) {
scoped_refptr<Configurator> config,
std::unique_ptr<UpdateScheduler> scheduler) {
DCHECK(config);
DCHECK(scheduler);
auto update_client = update_client::UpdateClientFactory(config);
return std::make_unique<CrxUpdateService>(config, std::move(update_client));
return std::make_unique<CrxUpdateService>(config, std::move(scheduler),
std::move(update_client));
}
} // namespace component_updater

@@ -39,6 +39,7 @@ namespace component_updater {
using Callback = update_client::Callback;
class OnDemandUpdater;
class UpdateScheduler;
using Configurator = update_client::Configurator;
using CrxComponent = update_client::CrxComponent;
@@ -175,7 +176,8 @@ class OnDemandUpdater {
// Creates the component updater.
std::unique_ptr<ComponentUpdateService> ComponentUpdateServiceFactory(
scoped_refptr<Configurator> config);
scoped_refptr<Configurator> config,
std::unique_ptr<UpdateScheduler> scheduler);
} // namespace component_updater

@@ -13,7 +13,7 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/threading/thread_checker.h"
#include "components/component_updater/timer.h"
#include "components/component_updater/update_scheduler.h"
namespace base {
class TimeTicks;
@@ -37,6 +37,7 @@ class CrxUpdateService : public ComponentUpdateService,
public:
CrxUpdateService(scoped_refptr<Configurator> config,
std::unique_ptr<UpdateScheduler> scheduler,
scoped_refptr<UpdateClient> update_client);
~CrxUpdateService() override;
@@ -65,9 +66,10 @@ class CrxUpdateService : public ComponentUpdateService,
void Start();
void Stop();
bool CheckForUpdates();
bool CheckForUpdates(UpdateScheduler::OnFinishedCallback on_finished);
void OnDemandUpdateInternal(const std::string& id, Callback callback);
bool OnDemandUpdateWithCooldown(const std::string& id);
bool DoUnregisterComponent(const CrxComponent& component);
@@ -85,11 +87,10 @@ class CrxUpdateService : public ComponentUpdateService,
base::ThreadChecker thread_checker_;
scoped_refptr<Configurator> config_;
std::unique_ptr<UpdateScheduler> scheduler_;
scoped_refptr<UpdateClient> update_client_;
Timer timer_;
// A collection of every registered component.
using Components = std::map<std::string, CrxComponent>;
Components components_;

@@ -11,6 +11,7 @@
#include <vector>
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/macros.h"
@@ -128,6 +129,16 @@ class MockServiceObserver : public ServiceObserver {
MOCK_METHOD2(OnEvent, void(Events event, const std::string&));
};
class MockUpdateScheduler : public UpdateScheduler {
public:
MOCK_METHOD4(Schedule,
void(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UserTask& user_task,
const OnStopTaskCallback& on_stop));
MOCK_METHOD0(Stop, void());
};
class ComponentUpdaterTest : public testing::Test {
public:
ComponentUpdaterTest();
@@ -144,17 +155,25 @@ class ComponentUpdaterTest : public testing::Test {
ComponentUpdateService& component_updater() { return *component_updater_; }
scoped_refptr<TestConfigurator> configurator() const { return config_; }
base::OnceClosure quit_closure() { return runloop_.QuitClosure(); }
MockUpdateScheduler& scheduler() { return *scheduler_; }
static void ReadyCallback() {}
protected:
void RunThreads();
private:
void RunUpdateTask(const UpdateScheduler::UserTask& user_task);
void Schedule(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UpdateScheduler::UserTask& user_task,
const UpdateScheduler::OnStopTaskCallback& on_stop);
base::test::ScopedTaskEnvironment scoped_task_environment_;
base::RunLoop runloop_;
scoped_refptr<TestConfigurator> config_ =
base::MakeRefCounted<TestConfigurator>();
MockUpdateScheduler* scheduler_;
scoped_refptr<MockUpdateClient> update_client_ =
base::MakeRefCounted<MockUpdateClient>();
std::unique_ptr<ComponentUpdateService> component_updater_;
@@ -206,13 +225,18 @@ std::unique_ptr<ComponentUpdateService> TestComponentUpdateServiceFactory(
scoped_refptr<Configurator> config) {
DCHECK(config);
return std::make_unique<CrxUpdateService>(
config, base::MakeRefCounted<MockUpdateClient>());
config, std::make_unique<MockUpdateScheduler>(),
base::MakeRefCounted<MockUpdateClient>());
}
ComponentUpdaterTest::ComponentUpdaterTest() {
EXPECT_CALL(update_client(), AddObserver(_)).Times(1);
component_updater_ =
std::make_unique<CrxUpdateService>(config_, update_client_);
auto scheduler = std::make_unique<MockUpdateScheduler>();
scheduler_ = scheduler.get();
ON_CALL(*scheduler_, Schedule(_, _, _, _))
.WillByDefault(Invoke(this, &ComponentUpdaterTest::Schedule));
component_updater_ = std::make_unique<CrxUpdateService>(
config_, std::move(scheduler), update_client_);
}
ComponentUpdaterTest::~ComponentUpdaterTest() {
@@ -230,10 +254,35 @@ void ComponentUpdaterTest::RunThreads() {
runloop_.Run();
}
void ComponentUpdaterTest::RunUpdateTask(
const UpdateScheduler::UserTask& user_task) {
scoped_task_environment_.GetMainThreadTaskRunner()->PostTask(
FROM_HERE, base::BindRepeating(
[](const UpdateScheduler::UserTask& user_task,
ComponentUpdaterTest* test) {
user_task.Run(base::BindOnce(
[](const UpdateScheduler::UserTask& user_task,
ComponentUpdaterTest* test) {
test->RunUpdateTask(user_task);
},
user_task, base::Unretained(test)));
},
user_task, base::Unretained(this)));
}
void ComponentUpdaterTest::Schedule(
const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UpdateScheduler::UserTask& user_task,
const UpdateScheduler::OnStopTaskCallback& on_stop) {
RunUpdateTask(user_task);
}
TEST_F(ComponentUpdaterTest, AddObserver) {
MockServiceObserver observer;
EXPECT_CALL(update_client(), AddObserver(&observer)).Times(1);
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
component_updater().AddObserver(&observer);
}
@@ -241,6 +290,7 @@ TEST_F(ComponentUpdaterTest, RemoveObserver) {
MockServiceObserver observer;
EXPECT_CALL(update_client(), RemoveObserver(&observer)).Times(1);
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
component_updater().RemoveObserver(&observer);
}
@@ -297,6 +347,8 @@ TEST_F(ComponentUpdaterTest, RegisterComponent) {
EXPECT_CALL(update_client(), IsUpdating(id1)).Times(1);
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Schedule(_, _, _, _)).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
EXPECT_TRUE(component_updater().RegisterComponent(crx_component1));
EXPECT_TRUE(component_updater().RegisterComponent(crx_component2));
@@ -333,8 +385,8 @@ TEST_F(ComponentUpdaterTest, OnDemandUpdate) {
base::HistogramTester ht;
auto config = configurator();
config->SetInitialDelay(3600);
// Don't run periodic update task.
ON_CALL(scheduler(), Schedule(_, _, _, _)).WillByDefault(Return());
auto& cus = component_updater();
@@ -357,6 +409,8 @@ TEST_F(ComponentUpdaterTest, OnDemandUpdate) {
EXPECT_CALL(update_client(), DoInstall("jebgalgnebhfojomionfpkfelancnnkf", _))
.WillOnce(Invoke(&loop_handler, &LoopHandler::OnInstall));
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Schedule(_, _, _, _)).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
EXPECT_TRUE(cus.RegisterComponent(crx_component));
OnDemandTester ondemand_tester;
@@ -394,8 +448,8 @@ TEST_F(ComponentUpdaterTest, MaybeThrottle) {
base::HistogramTester ht;
auto config = configurator();
config->SetInitialDelay(3600);
// Don't run periodic update task.
ON_CALL(scheduler(), Schedule(_, _, _, _)).WillByDefault(Return());
scoped_refptr<MockInstaller> installer =
base::MakeRefCounted<MockInstaller>();
@@ -410,6 +464,8 @@ TEST_F(ComponentUpdaterTest, MaybeThrottle) {
EXPECT_CALL(update_client(), DoInstall("jebgalgnebhfojomionfpkfelancnnkf", _))
.WillOnce(Invoke(&loop_handler, &LoopHandler::OnInstall));
EXPECT_CALL(update_client(), Stop()).Times(1);
EXPECT_CALL(scheduler(), Schedule(_, _, _, _)).Times(1);
EXPECT_CALL(scheduler(), Stop()).Times(1);
EXPECT_TRUE(component_updater().RegisterComponent(crx_component));
component_updater().MaybeThrottle(

@@ -0,0 +1,27 @@
// Copyright 2018 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/component_updater/timer_update_scheduler.h"
namespace component_updater {
TimerUpdateScheduler::TimerUpdateScheduler() = default;
TimerUpdateScheduler::~TimerUpdateScheduler() = default;
void TimerUpdateScheduler::Schedule(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UserTask& user_task,
const OnStopTaskCallback& on_stop) {
timer_.Start(
initial_delay, delay,
base::BindRepeating(
[](const UserTask& user_task) { user_task.Run(base::DoNothing()); },
user_task));
}
void TimerUpdateScheduler::Stop() {
timer_.Stop();
}
} // namespace component_updater

@@ -0,0 +1,37 @@
// Copyright 2018 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_COMPONENT_UPDATER_TIMER_UPDATE_SCHEDULER_H_
#define COMPONENTS_COMPONENT_UPDATER_TIMER_UPDATE_SCHEDULER_H_
#include "base/callback.h"
#include "base/macros.h"
#include "components/component_updater/timer.h"
#include "components/component_updater/update_scheduler.h"
namespace component_updater {
// Scheduler that uses base::Timer to schedule updates.
class TimerUpdateScheduler : public UpdateScheduler {
public:
TimerUpdateScheduler();
~TimerUpdateScheduler() override;
// UpdateScheduler:
void Schedule(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UserTask& user_task,
const OnStopTaskCallback& on_stop) override;
void Stop() override;
private:
Timer timer_;
base::RepeatingClosure user_task_;
DISALLOW_COPY_AND_ASSIGN(TimerUpdateScheduler);
};
} // namespace component_updater
#endif // COMPONENTS_COMPONENT_UPDATER_TIMER_UPDATE_SCHEDULER_H_

@@ -0,0 +1,39 @@
// Copyright 2018 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_COMPONENT_UPDATER_UPDATE_SCHEDULER_H_
#define COMPONENTS_COMPONENT_UPDATER_UPDATE_SCHEDULER_H_
#include "base/callback_forward.h"
#include "base/time/time.h"
namespace component_updater {
// Abstract interface for an update task scheduler.
class UpdateScheduler {
public:
using OnFinishedCallback = base::OnceCallback<void()>;
// Type of task to be run by the scheduler. The task can start asynchronous
// operations and must call |on_finished| when all operations have completed.
using UserTask =
base::RepeatingCallback<void(OnFinishedCallback on_finished)>;
using OnStopTaskCallback = base::RepeatingCallback<void()>;
virtual ~UpdateScheduler() = default;
// Schedules |user_task| to be run periodically with at least an interval of
// |delay|. The first time |user_task| will be run after at least
// |initial_delay|. If the execution of |user_task| must be stopped before it
// called its |on_finished| callback, |on_stop| will be called.
virtual void Schedule(const base::TimeDelta& initial_delay,
const base::TimeDelta& delay,
const UserTask& user_task,
const OnStopTaskCallback& on_stop) = 0;
// Stops to periodically run |user_task| previously scheduled with |Schedule|.
virtual void Stop() = 0;
};
} // namespace component_updater
#endif // COMPONENTS_COMPONENT_UPDATER_UPDATE_SCHEDULER_H_

@@ -18,6 +18,7 @@
#include "base/time/default_clock.h"
#include "base/time/default_tick_clock.h"
#include "components/component_updater/component_updater_service.h"
#include "components/component_updater/timer_update_scheduler.h"
#include "components/gcm_driver/gcm_client_factory.h"
#include "components/gcm_driver/gcm_desktop_utils.h"
#include "components/gcm_driver/gcm_driver.h"
@@ -314,7 +315,8 @@ ApplicationContextImpl::GetComponentUpdateService() {
// be registered and Start() needs to be called.
component_updater_ = component_updater::ComponentUpdateServiceFactory(
component_updater::MakeIOSComponentUpdaterConfigurator(
base::CommandLine::ForCurrentProcess()));
base::CommandLine::ForCurrentProcess()),
std::make_unique<component_updater::TimerUpdateScheduler>());
}
return component_updater_.get();
}