Commit c6b1a263 authored by Hesen Zhang's avatar Hesen Zhang Committed by Commit Bot

[Query Tiles]: TileServiceScheduler enhancement.

- In the header file, change TileServiceScheduler to a pure interface,
and create TileServiceSchedulerImpl class to inherit from LogSource and
TileServiceScheduler both.

- Implemented LogSource in Scheduler - as TileServiceImpl will actually
owns the Logger.

- Create Delegate in scheduler and GetTileGroup through TileManager,
in order to expose to LogSource.

- Added two more scheduler event APIs OnFetchStarted and OnGroupDataSaved
in order to update the internal status.

- (TODO) convert TileGroup to base::Value with pretty format.

- Follow up CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2264934

Bug: 1066556
Change-Id: Ifa8099499f17e0cddf745dffc7384b3e3595fa2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277073
Auto-Submit: Hesen Zhang <hesen@chromium.org>
Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785892}
parent 677daa5d
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/observer_list.h" #include "base/observer_list.h"
#include "components/query_tiles/internal/log_sink.h" #include "components/query_tiles/internal/log_sink.h"
#include "components/query_tiles/internal/tile_group.h"
#include "components/query_tiles/internal/tile_types.h" #include "components/query_tiles/internal/tile_types.h"
#include "components/query_tiles/logger.h" #include "components/query_tiles/logger.h"
#include "net/base/backoff_entry.h" #include "net/base/backoff_entry.h"
...@@ -27,11 +28,8 @@ class LogSource { ...@@ -27,11 +28,8 @@ class LogSource {
// Returns the TileManager status. // Returns the TileManager status.
virtual TileGroupStatus GetGroupStatus() = 0; virtual TileGroupStatus GetGroupStatus() = 0;
// Returns a serialized string of group info. // Returns the pointer of TileGroup holds in memory. Nullptr if not exists.
virtual std::string GetGroupInfo() = 0; virtual TileGroup* GetTileGroup() = 0;
// Returns formatted string of tiles proto data.
virtual std::string GetTilesProto() = 0;
}; };
} // namespace query_tiles } // namespace query_tiles
......
...@@ -76,9 +76,11 @@ base::Value LoggerImpl::GetTileData() { ...@@ -76,9 +76,11 @@ base::Value LoggerImpl::GetTileData() {
base::DictionaryValue result; base::DictionaryValue result;
if (!log_source_) if (!log_source_)
return std::move(result); return std::move(result);
auto* tile_group = log_source_->GetTileGroup();
result.SetString("groupInfo", log_source_->GetGroupInfo()); // (crbug.com/1101557): Make the format pretty with every field in TileGroup
result.SetString("tileProto", log_source_->GetTilesProto()); // explicitly appears in the DictValue.
if (tile_group)
result.SetString("groupInfo", tile_group->DebugString());
return std::move(result); return std::move(result);
} }
......
...@@ -103,9 +103,8 @@ class TileManagerImpl : public TileManager { ...@@ -103,9 +103,8 @@ class TileManagerImpl : public TileManager {
accept_languages_ = accept_languages; accept_languages_ = accept_languages;
} }
void GetTileGroupForTesting(TileGroup* tile_group) override { TileGroup* GetTileGroup() override {
if (tile_group_) return tile_group_ ? tile_group_.get() : nullptr;
*tile_group = *tile_group_.get();
} }
void OnTileStoreInitialized( void OnTileStoreInitialized(
......
...@@ -47,17 +47,19 @@ class TileManager { ...@@ -47,17 +47,19 @@ class TileManager {
virtual void SaveTiles(std::unique_ptr<TileGroup> tile_group, virtual void SaveTiles(std::unique_ptr<TileGroup> tile_group,
TileGroupStatusCallback callback) = 0; TileGroupStatusCallback callback) = 0;
// Delete everything in db. Used for debugging and testing only. // Delete everything in db. Used for debugging in WebUI only.
virtual TileGroupStatus PurgeDb() = 0; virtual TileGroupStatus PurgeDb() = 0;
// Dump the group. Used for debugging in WebUI only.
virtual TileGroup* GetTileGroup() = 0;
TileManager(); TileManager();
virtual ~TileManager() = default; virtual ~TileManager() = default;
TileManager(const TileManager& other) = delete; TileManager(const TileManager& other) = delete;
TileManager& operator=(const TileManager& other) = delete; TileManager& operator=(const TileManager& other) = delete;
//-------Debug and Testing----------------------------------------- // ------------------------Testing------------------------
virtual void GetTileGroupForTesting(TileGroup* group) = 0;
virtual void SetAcceptLanguagesForTesting( virtual void SetAcceptLanguagesForTesting(
const std::string& accept_languages) = 0; const std::string& accept_languages) = 0;
}; };
......
...@@ -342,9 +342,8 @@ TEST_F(TileManagerTest, GetTileGroup) { ...@@ -342,9 +342,8 @@ TEST_F(TileManagerTest, GetTileGroup) {
test::ResetTestGroup(&expected); test::ResetTestGroup(&expected);
InitWithData(TileGroupStatus::kSuccess, {expected}); InitWithData(TileGroupStatus::kSuccess, {expected});
TileGroup actual; TileGroup* actual = manager()->GetTileGroup();
manager()->GetTileGroupForTesting(&actual); EXPECT_TRUE(test::AreTileGroupsIdentical(*actual, expected));
EXPECT_TRUE(test::AreTileGroupsIdentical(actual, expected));
} }
} // namespace } // namespace
......
...@@ -9,14 +9,11 @@ ...@@ -9,14 +9,11 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/guid.h" #include "base/guid.h"
#include "base/rand_util.h" #include "base/rand_util.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "components/query_tiles/internal/proto_conversion.h" #include "components/query_tiles/internal/proto_conversion.h"
#include "components/query_tiles/internal/stats.h"
#include "components/query_tiles/internal/tile_config.h" #include "components/query_tiles/internal/tile_config.h"
#include "components/query_tiles/switches.h"
namespace query_tiles { namespace query_tiles {
...@@ -45,6 +42,7 @@ void TileServiceImpl::OnTileManagerInitialized(SuccessCallback callback, ...@@ -45,6 +42,7 @@ void TileServiceImpl::OnTileManagerInitialized(SuccessCallback callback,
bool success = (status == TileGroupStatus::kSuccess || bool success = (status == TileGroupStatus::kSuccess ||
status == TileGroupStatus::kNoTiles); status == TileGroupStatus::kNoTiles);
DCHECK(callback); DCHECK(callback);
scheduler_->SetDelegate(this);
scheduler_->OnTileManagerInitialized(status); scheduler_->OnTileManagerInitialized(status);
std::move(callback).Run(success); std::move(callback).Run(success);
} }
...@@ -65,10 +63,7 @@ void TileServiceImpl::StartFetchForTiles( ...@@ -65,10 +63,7 @@ void TileServiceImpl::StartFetchForTiles(
tile_fetcher_->StartFetchForTiles(base::BindOnce( tile_fetcher_->StartFetchForTiles(base::BindOnce(
&TileServiceImpl::OnFetchFinished, weak_ptr_factory_.GetWeakPtr(), &TileServiceImpl::OnFetchFinished, weak_ptr_factory_.GetWeakPtr(),
is_from_reduced_mode, std::move(task_finished_callback))); is_from_reduced_mode, std::move(task_finished_callback)));
scheduler_->OnFetchStarted();
base::Time::Exploded local_explode;
base::Time::Now().LocalExplode(&local_explode);
stats::RecordExplodeOnFetchStarted(local_explode.hour);
} }
void TileServiceImpl::CancelTask() { void TileServiceImpl::CancelTask() {
...@@ -114,6 +109,8 @@ void TileServiceImpl::OnTilesSaved( ...@@ -114,6 +109,8 @@ void TileServiceImpl::OnTilesSaved(
bool is_from_reduced_mode, bool is_from_reduced_mode,
BackgroundTaskFinishedCallback task_finished_callback, BackgroundTaskFinishedCallback task_finished_callback,
TileGroupStatus status) { TileGroupStatus status) {
scheduler_->OnGroupDataSaved(status);
if (status != TileGroupStatus::kSuccess) { if (status != TileGroupStatus::kSuccess) {
std::move(task_finished_callback).Run(false /*reschedule*/); std::move(task_finished_callback).Run(false /*reschedule*/);
return; return;
...@@ -132,4 +129,8 @@ void TileServiceImpl::OnPrefetchImagesDone( ...@@ -132,4 +129,8 @@ void TileServiceImpl::OnPrefetchImagesDone(
std::move(task_finished_callback).Run(false /*reschedule*/); std::move(task_finished_callback).Run(false /*reschedule*/);
} }
TileGroup* TileServiceImpl::GetTileGroup() {
return tile_manager_->GetTileGroup();
}
} // namespace query_tiles } // namespace query_tiles
...@@ -28,7 +28,8 @@ class InitializableTileService : public TileService { ...@@ -28,7 +28,8 @@ class InitializableTileService : public TileService {
~InitializableTileService() override = default; ~InitializableTileService() override = default;
}; };
class TileServiceImpl : public InitializableTileService { class TileServiceImpl : public InitializableTileService,
public TileServiceScheduler::Delegate {
public: public:
TileServiceImpl(std::unique_ptr<ImagePrefetcher> image_prefetcher, TileServiceImpl(std::unique_ptr<ImagePrefetcher> image_prefetcher,
std::unique_ptr<TileManager> tile_manager, std::unique_ptr<TileManager> tile_manager,
...@@ -53,6 +54,9 @@ class TileServiceImpl : public InitializableTileService { ...@@ -53,6 +54,9 @@ class TileServiceImpl : public InitializableTileService {
void CancelTask() override; void CancelTask() override;
void PurgeDb() override; void PurgeDb() override;
// TileServiceScheduler::Delegate implementation.
TileGroup* GetTileGroup() override;
// Called when tile manager is initialized. // Called when tile manager is initialized.
void OnTileManagerInitialized(SuccessCallback callback, void OnTileManagerInitialized(SuccessCallback callback,
TileGroupStatus status); TileGroupStatus status);
......
...@@ -32,23 +32,27 @@ namespace { ...@@ -32,23 +32,27 @@ namespace {
class MockTileManager : public TileManager { class MockTileManager : public TileManager {
public: public:
MockTileManager() = default; MockTileManager() = default;
MOCK_METHOD1(Init, void(TileGroupStatusCallback)); MOCK_METHOD(void, Init, (TileGroupStatusCallback));
MOCK_METHOD1(GetTiles, void(GetTilesCallback)); MOCK_METHOD(void, GetTiles, (GetTilesCallback));
MOCK_METHOD2(GetTile, void(const std::string&, TileCallback)); MOCK_METHOD(void, GetTile, (const std::string&, TileCallback));
MOCK_METHOD2(SaveTiles, MOCK_METHOD(void,
void(std::unique_ptr<TileGroup>, TileGroupStatusCallback)); SaveTiles,
MOCK_METHOD1(SetAcceptLanguagesForTesting, void(const std::string&)); (std::unique_ptr<TileGroup>, TileGroupStatusCallback));
MOCK_METHOD0(PurgeDb, TileGroupStatus()); MOCK_METHOD(void, SetAcceptLanguagesForTesting, (const std::string&));
MOCK_METHOD1(GetTileGroupForTesting, void(TileGroup*)); MOCK_METHOD(TileGroupStatus, PurgeDb, ());
MOCK_METHOD(TileGroup*, GetTileGroup, ());
}; };
class MockTileServiceScheduler : public TileServiceScheduler { class MockTileServiceScheduler : public TileServiceScheduler {
public: public:
MockTileServiceScheduler() = default; MockTileServiceScheduler() = default;
MOCK_METHOD1(OnFetchCompleted, void(TileInfoRequestStatus)); MOCK_METHOD(void, OnFetchStarted, ());
MOCK_METHOD1(OnTileManagerInitialized, void(TileGroupStatus)); MOCK_METHOD(void, OnFetchCompleted, (TileInfoRequestStatus));
MOCK_METHOD0(CancelTask, void()); MOCK_METHOD(void, OnTileManagerInitialized, (TileGroupStatus));
MOCK_METHOD1(OnDbPurged, void(TileGroupStatus)); MOCK_METHOD(void, CancelTask, ());
MOCK_METHOD(void, OnDbPurged, (TileGroupStatus));
MOCK_METHOD(void, OnGroupDataSaved, (TileGroupStatus));
MOCK_METHOD(void, SetDelegate, (TileServiceScheduler::Delegate*));
}; };
class MockImagePrefetcher : public ImagePrefetcher { class MockImagePrefetcher : public ImagePrefetcher {
......
...@@ -6,10 +6,12 @@ ...@@ -6,10 +6,12 @@
#define COMPONENTS_QUERY_TILES_INTERNAL_TILE_SERVICE_SCHEDULER_H_ #define COMPONENTS_QUERY_TILES_INTERNAL_TILE_SERVICE_SCHEDULER_H_
#include <memory> #include <memory>
#include <string>
#include "base/time/clock.h" #include "base/time/clock.h"
#include "base/time/tick_clock.h" #include "base/time/tick_clock.h"
#include "components/background_task_scheduler/background_task_scheduler.h" #include "components/background_task_scheduler/background_task_scheduler.h"
#include "components/query_tiles/internal/log_source.h"
#include "components/query_tiles/internal/tile_config.h" #include "components/query_tiles/internal/tile_config.h"
#include "components/query_tiles/internal/tile_types.h" #include "components/query_tiles/internal/tile_types.h"
#include "components/query_tiles/tile_service_prefs.h" #include "components/query_tiles/tile_service_prefs.h"
...@@ -23,13 +25,23 @@ namespace query_tiles { ...@@ -23,13 +25,23 @@ namespace query_tiles {
// TileBackgroundTask. // TileBackgroundTask.
class TileServiceScheduler { class TileServiceScheduler {
public: public:
// Method to create a TileServiceScheduler instance. class Delegate {
static std::unique_ptr<TileServiceScheduler> Create( public:
background_task::BackgroundTaskScheduler* scheduler, Delegate() = default;
PrefService* prefs, virtual ~Delegate() = default;
base::Clock* clock,
const base::TickClock* tick_clock, Delegate(const Delegate& other) = delete;
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy); Delegate& operator=(const Delegate& other) = delete;
// Returns the tile group instance holds in memory.
virtual TileGroup* GetTileGroup() = 0;
};
// Set delegate object for the scheduler.
virtual void SetDelegate(Delegate* delegate) = 0;
// Called when fetching task starts.
virtual void OnFetchStarted() = 0;
// Called on fetch task completed, schedule another task with or without // Called on fetch task completed, schedule another task with or without
// backoff based on the status. Success status will lead a regular schedule // backoff based on the status. Success status will lead a regular schedule
...@@ -44,19 +56,99 @@ class TileServiceScheduler { ...@@ -44,19 +56,99 @@ class TileServiceScheduler {
// directly set the release time until 24 hours later. // directly set the release time until 24 hours later.
virtual void OnTileManagerInitialized(TileGroupStatus status) = 0; virtual void OnTileManagerInitialized(TileGroupStatus status) = 0;
// Called when datanase is purged. Reset the flow and update the status. // Called when database is purged. Reset the flow and update the status.
virtual void OnDbPurged(TileGroupStatus status) = 0; virtual void OnDbPurged(TileGroupStatus status) = 0;
// Cancel current existing task, and reset backoff. // Called when parsed group data are saved.
virtual void OnGroupDataSaved(TileGroupStatus status) = 0;
// Cancel current existing task, and reset scheduler.
virtual void CancelTask() = 0; virtual void CancelTask() = 0;
virtual ~TileServiceScheduler(); virtual ~TileServiceScheduler() = default;
TileServiceScheduler(const TileServiceScheduler& other) = delete; TileServiceScheduler(const TileServiceScheduler& other) = delete;
TileServiceScheduler& operator=(const TileServiceScheduler& other) = delete; TileServiceScheduler& operator=(const TileServiceScheduler& other) = delete;
protected: protected:
TileServiceScheduler(); TileServiceScheduler() = default;
};
// An implementation of TileServiceScheduler interface and LogSource interface.
class TileServiceSchedulerImpl : public TileServiceScheduler, public LogSource {
public:
TileServiceSchedulerImpl(
background_task::BackgroundTaskScheduler* scheduler,
PrefService* prefs,
base::Clock* clock,
const base::TickClock* tick_clock,
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy);
~TileServiceSchedulerImpl() override;
private:
// TileServiceScheduler implementation.
void CancelTask() override;
void OnFetchStarted() override;
void OnFetchCompleted(TileInfoRequestStatus status) override;
void OnTileManagerInitialized(TileGroupStatus status) override;
void OnDbPurged(TileGroupStatus status) override;
void OnGroupDataSaved(TileGroupStatus status) override;
void SetDelegate(Delegate* delegate) override;
// LogSource implementation.
TileInfoRequestStatus GetFetcherStatus() override;
TileGroupStatus GetGroupStatus() override;
TileGroup* GetTileGroup() override;
void ScheduleTask(bool is_init_schedule);
std::unique_ptr<net::BackoffEntry> GetBackoff();
void AddBackoff();
void ResetBackoff();
void MaximizeBackoff();
int64_t GetDelaysFromBackoff();
void GetInstantTaskWindow(int64_t* start_time_ms,
int64_t* end_time_ms,
bool is_init_schedule);
void GetTaskWindow(int64_t* start_time_ms,
int64_t* end_time_ms,
bool is_init_schedule);
void UpdateBackoff(net::BackoffEntry* backoff);
void MarkFirstRunScheduled();
void MarkFirstRunFinished();
// Returns true if the initial task has been scheduled because no tiles in
// db(kickoff condition), but still waiting to be completed at the designated
// window. Returns false either the first task is not scheduled yet or it is
// already finished.
bool IsDuringFirstFlow();
// Native Background Scheduler instance.
background_task::BackgroundTaskScheduler* scheduler_;
// PrefService.
PrefService* prefs_;
// Clock object to get current time.
base::Clock* clock_;
// TickClock used for building backoff entry.
const base::TickClock* tick_clock_;
// Backoff policy used for reschdule.
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy_;
// Flag to indicate if currently have a suspend status to avoid overwriting if
// already scheduled a suspend task during this lifecycle.
bool is_suspend_;
// Delegate object.
Delegate* delegate_;
// Internal fetcher status.
TileInfoRequestStatus fetcher_status_;
// Internal group status.
TileGroupStatus group_status_;
}; };
} // namespace query_tiles } // namespace query_tiles
......
...@@ -56,9 +56,9 @@ class TileServiceSchedulerTest : public testing::Test { ...@@ -56,9 +56,9 @@ class TileServiceSchedulerTest : public testing::Test {
clock_.SetNow(fake_now); clock_.SetNow(fake_now);
query_tiles::RegisterPrefs(prefs()->registry()); query_tiles::RegisterPrefs(prefs()->registry());
auto policy = std::make_unique<net::BackoffEntry::Policy>(kTestPolicy); auto policy = std::make_unique<net::BackoffEntry::Policy>(kTestPolicy);
tile_service_scheduler_ = tile_service_scheduler_ = std::make_unique<TileServiceSchedulerImpl>(
TileServiceScheduler::Create(&mocked_native_scheduler_, &prefs_, &mocked_native_scheduler_, &prefs_, &clock_, &tick_clock_,
&clock_, &tick_clock_, std::move(policy)); std::move(policy));
EXPECT_CALL( EXPECT_CALL(
*native_scheduler(), *native_scheduler(),
Cancel(static_cast<int>(background_task::TaskIds::QUERY_TILE_JOB_ID))); Cancel(static_cast<int>(background_task::TaskIds::QUERY_TILE_JOB_ID)));
......
...@@ -91,9 +91,10 @@ std::unique_ptr<TileService> CreateTileService( ...@@ -91,9 +91,10 @@ std::unique_ptr<TileService> CreateTileService(
// Wrap background task scheduler. // Wrap background task scheduler.
auto policy = std::make_unique<net::BackoffEntry::Policy>(); auto policy = std::make_unique<net::BackoffEntry::Policy>();
BuildBackoffPolicy(policy.get()); BuildBackoffPolicy(policy.get());
auto tile_background_task_scheduler = TileServiceScheduler::Create( auto tile_background_task_scheduler =
scheduler, pref_service, clock, base::DefaultTickClock::GetInstance(), std::make_unique<TileServiceSchedulerImpl>(
std::move(policy)); scheduler, pref_service, clock, base::DefaultTickClock::GetInstance(),
std::move(policy));
auto tile_service_impl = std::make_unique<TileServiceImpl>( auto tile_service_impl = std::make_unique<TileServiceImpl>(
std::move(image_prefetcher), std::move(tile_manager), std::move(image_prefetcher), std::move(tile_manager),
......
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