Skip to content

Commit

Permalink
[Query Tiles]: Complete query-tiles WebUI internals.
Browse files Browse the repository at this point in the history
- Add InternalsUIMessageHandler as observer.

- Inject Logger to TileServiceImpl.

- Inject LogSink into TileServiceScheduler and ping the observer.

- Set scheduler as LogSource.

Bug: 1066556
Change-Id: I6fdf79da6dea82327af5be1aa1b79a6cbee9926d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264934
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786090}
  • Loading branch information
Hesen Zhang authored and Commit Bot committed Jul 8, 2020
1 parent d4ae2d6 commit 7189597
Show file tree
Hide file tree
Showing 16 changed files with 106 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ QueryTilesInternalsUIMessageHandler::QueryTilesInternalsUIMessageHandler(
DCHECK(tile_service_);
}

QueryTilesInternalsUIMessageHandler::~QueryTilesInternalsUIMessageHandler() =
default;
QueryTilesInternalsUIMessageHandler::~QueryTilesInternalsUIMessageHandler() {
tile_service_->GetLogger()->RemoveObserver(this);
}

void QueryTilesInternalsUIMessageHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback(
Expand All @@ -45,16 +46,28 @@ void QueryTilesInternalsUIMessageHandler::RegisterMessages() {
"getTileData",
base::Bind(&QueryTilesInternalsUIMessageHandler::HandleGetTileData,
weak_ptr_factory_.GetWeakPtr()));

tile_service_->GetLogger()->AddObserver(this);
}

void QueryTilesInternalsUIMessageHandler::HandleGetTileData(
const base::ListValue* args) {
NOTIMPLEMENTED();
AllowJavascript();
const base::Value* callback_id;
auto result = args->Get(0, &callback_id);
DCHECK(result);
ResolveJavascriptCallback(*callback_id,
tile_service_->GetLogger()->GetTileData());
}

void QueryTilesInternalsUIMessageHandler::HandleGetServiceStatus(
const base::ListValue* args) {
NOTIMPLEMENTED();
AllowJavascript();
const base::Value* callback_id;
auto result = args->Get(0, &callback_id);
DCHECK(result);
ResolveJavascriptCallback(*callback_id,
tile_service_->GetLogger()->GetServiceStatus());
}

void QueryTilesInternalsUIMessageHandler::HandleStartFetch(
Expand All @@ -68,3 +81,18 @@ void QueryTilesInternalsUIMessageHandler::HandlePurgeDb(
const base::ListValue* args) {
tile_service_->PurgeDb();
}

void QueryTilesInternalsUIMessageHandler::OnServiceStatusChanged(
const base::Value& status) {
if (!IsJavascriptAllowed())
return;

FireWebUIListener("service-status-changed", status);
}

void QueryTilesInternalsUIMessageHandler::OnTileDataAvailable(
const base::Value& data) {
if (!IsJavascriptAllowed())
return;
FireWebUIListener("tile-data-available", data);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "components/query_tiles/logger.h"
#include "content/public/browser/web_ui_message_handler.h"

namespace base {
Expand All @@ -20,7 +21,8 @@ class TileService;
}

class QueryTilesInternalsUIMessageHandler
: public content::WebUIMessageHandler {
: public content::WebUIMessageHandler,
public query_tiles::Logger::Observer {
public:
explicit QueryTilesInternalsUIMessageHandler(Profile* profile);
~QueryTilesInternalsUIMessageHandler() override;
Expand All @@ -29,6 +31,10 @@ class QueryTilesInternalsUIMessageHandler
void RegisterMessages() override;

private:
// Logger::Observer implementation.
void OnServiceStatusChanged(const base::Value& status) override;
void OnTileDataAvailable(const base::Value& status) override;

void HandleGetServiceStatus(const base::ListValue* args);
void HandleGetTileData(const base::ListValue* args);
void HandleStartFetch(const base::ListValue* args);
Expand Down
1 change: 1 addition & 0 deletions components/query_tiles/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ source_set("unit_tests") {
"//components/query_tiles:public",
"//components/query_tiles/proto",
"//components/query_tiles/test:test_lib",
"//components/query_tiles/test:test_support",
"//skia",
"//testing/gmock",
"//testing/gtest",
Expand Down
4 changes: 4 additions & 0 deletions components/query_tiles/internal/init_aware_tile_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ void InitAwareTileService::PurgeDb() {
}
}

Logger* InitAwareTileService::GetLogger() {
return tile_service_->GetLogger();
}

void InitAwareTileService::MaybeCacheApiCall(base::OnceClosure api_call) {
DCHECK(!init_success_.has_value())
<< "Only cache API calls before initialization.";
Expand Down
1 change: 1 addition & 0 deletions components/query_tiles/internal/init_aware_tile_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class InitAwareTileService : public TileService {
BackgroundTaskFinishedCallback callback) override;
void CancelTask() override;
void PurgeDb() override;
Logger* GetLogger() override;

void OnTileServiceInitialized(bool success);
void MaybeCacheApiCall(base::OnceClosure api_call);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class MockInitializableTileService : public InitializableTileService {
(override));
MOCK_METHOD(void, CancelTask, (), (override));
MOCK_METHOD(void, PurgeDb, (), (override));
MOCK_METHOD(Logger*, GetLogger, (), (override));

// Callback stubs.
MOCK_METHOD(void, GetTilesCallbackStub, (TileList), ());
Expand Down
10 changes: 8 additions & 2 deletions components/query_tiles/internal/tile_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ TileServiceImpl::TileServiceImpl(
std::unique_ptr<TileManager> tile_manager,
std::unique_ptr<TileServiceScheduler> scheduler,
std::unique_ptr<TileFetcher> tile_fetcher,
base::Clock* clock)
base::Clock* clock,
std::unique_ptr<Logger> logger)
: image_prefetcher_(std::move(image_prefetcher)),
tile_manager_(std::move(tile_manager)),
scheduler_(std::move(scheduler)),
tile_fetcher_(std::move(tile_fetcher)),
clock_(clock) {}
clock_(clock),
logger_(std::move(logger)) {}

TileServiceImpl::~TileServiceImpl() = default;

Expand Down Expand Up @@ -133,4 +135,8 @@ TileGroup* TileServiceImpl::GetTileGroup() {
return tile_manager_->GetTileGroup();
}

Logger* TileServiceImpl::GetLogger() {
return logger_.get();
}

} // namespace query_tiles
7 changes: 6 additions & 1 deletion components/query_tiles/internal/tile_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "components/query_tiles/internal/tile_manager.h"
#include "components/query_tiles/internal/tile_service_scheduler.h"
#include "components/query_tiles/internal/tile_types.h"
#include "components/query_tiles/logger.h"
#include "components/query_tiles/tile_service.h"

namespace query_tiles {
Expand All @@ -35,7 +36,8 @@ class TileServiceImpl : public InitializableTileService,
std::unique_ptr<TileManager> tile_manager,
std::unique_ptr<TileServiceScheduler> scheduler,
std::unique_ptr<TileFetcher> tile_fetcher,
base::Clock* clock);
base::Clock* clock,
std::unique_ptr<Logger> logger);
~TileServiceImpl() override;

// Disallow copy/assign.
Expand All @@ -53,6 +55,7 @@ class TileServiceImpl : public InitializableTileService,
BackgroundTaskFinishedCallback callback) override;
void CancelTask() override;
void PurgeDb() override;
Logger* GetLogger() override;

// TileServiceScheduler::Delegate implementation.
TileGroup* GetTileGroup() override;
Expand Down Expand Up @@ -92,6 +95,8 @@ class TileServiceImpl : public InitializableTileService,
// Clock object.
base::Clock* clock_;

std::unique_ptr<Logger> logger_;

base::WeakPtrFactory<TileServiceImpl> weak_ptr_factory_{this};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/test/task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/query_tiles/internal/image_prefetcher.h"
#include "components/query_tiles/test/empty_logger.h"
#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
#include "services/network/test/test_utils.h"
Expand Down Expand Up @@ -93,9 +94,11 @@ class TileServiceImplTest : public testing::Test {
EXPECT_TRUE(request.url.is_valid() && !request.url.is_empty());
last_resource_request_ = request;
}));
auto logger = std::make_unique<test::EmptyLogger>();
tile_service_impl_ = std::make_unique<TileServiceImpl>(
std::move(image_prefetcher), std::move(tile_manager),
std::move(scheduler), std::move(tile_fetcher), &clock_);
std::move(scheduler), std::move(tile_fetcher), &clock_,
std::move(logger));
}

void Initialize(bool expected_result) {
Expand Down
15 changes: 13 additions & 2 deletions components/query_tiles/internal/tile_service_scheduler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ TileServiceSchedulerImpl::TileServiceSchedulerImpl(
PrefService* prefs,
base::Clock* clock,
const base::TickClock* tick_clock,
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy)
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy,
LogSink* log_sink)
: scheduler_(scheduler),
prefs_(prefs),
clock_(clock),
Expand All @@ -44,7 +45,8 @@ TileServiceSchedulerImpl::TileServiceSchedulerImpl(
is_suspend_(false),
delegate_(nullptr),
fetcher_status_(TileInfoRequestStatus::kInit),
group_status_(TileGroupStatus::kUninitialized) {}
group_status_(TileGroupStatus::kUninitialized),
log_sink_(log_sink) {}

TileServiceSchedulerImpl::~TileServiceSchedulerImpl() = default;

Expand Down Expand Up @@ -93,15 +95,18 @@ void TileServiceSchedulerImpl::OnFetchCompleted(TileInfoRequestStatus status) {
ScheduleTask(true);
}
stats::RecordTileRequestStatus(status);
PingLogSink();
}

void TileServiceSchedulerImpl::OnDbPurged(TileGroupStatus status) {
CancelTask();
group_status_ = status;
PingLogSink();
}

void TileServiceSchedulerImpl::OnGroupDataSaved(TileGroupStatus status) {
group_status_ = status;
PingLogSink();
}

void TileServiceSchedulerImpl::OnTileManagerInitialized(
Expand All @@ -125,6 +130,7 @@ void TileServiceSchedulerImpl::OnTileManagerInitialized(
is_suspend_ = true;
}
stats::RecordTileGroupStatus(status);
PingLogSink();
}

TileInfoRequestStatus TileServiceSchedulerImpl::GetFetcherStatus() {
Expand Down Expand Up @@ -246,4 +252,9 @@ void TileServiceSchedulerImpl::SetDelegate(Delegate* delegate) {
delegate_ = delegate;
}

void TileServiceSchedulerImpl::PingLogSink() {
log_sink_->OnServiceStatusChanged();
log_sink_->OnTileDataAvailable();
}

} // namespace query_tiles
10 changes: 9 additions & 1 deletion components/query_tiles/internal/tile_service_scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ class TileServiceSchedulerImpl : public TileServiceScheduler, public LogSource {
PrefService* prefs,
base::Clock* clock,
const base::TickClock* tick_clock,
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy);
std::unique_ptr<net::BackoffEntry::Policy> backoff_policy,
LogSink* log_sink);

~TileServiceSchedulerImpl() override;

Expand Down Expand Up @@ -122,6 +123,10 @@ class TileServiceSchedulerImpl : public TileServiceScheduler, public LogSource {
// window. Returns false either the first task is not scheduled yet or it is
// already finished.
bool IsDuringFirstFlow();

// Ping the log sink to update.
void PingLogSink();

// Native Background Scheduler instance.
background_task::BackgroundTaskScheduler* scheduler_;

Expand Down Expand Up @@ -149,6 +154,9 @@ class TileServiceSchedulerImpl : public TileServiceScheduler, public LogSource {

// Internal group status.
TileGroupStatus group_status_;

// Log Sink object.
LogSink* log_sink_;
};

} // namespace query_tiles
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/test/task_environment.h"
#include "base/test/test_mock_time_task_runner.h"
#include "components/prefs/testing_pref_service.h"
#include "components/query_tiles/internal/black_hole_log_sink.h"
#include "components/query_tiles/internal/tile_config.h"
#include "components/query_tiles/internal/tile_store.h"
#include "components/query_tiles/switches.h"
Expand Down Expand Up @@ -55,10 +56,11 @@ class TileServiceSchedulerTest : public testing::Test {
EXPECT_TRUE(base::Time::FromString("05/18/20 01:00:00 AM", &fake_now));
clock_.SetNow(fake_now);
query_tiles::RegisterPrefs(prefs()->registry());
log_sink_ = std::make_unique<test::BlackHoleLogSink>();
auto policy = std::make_unique<net::BackoffEntry::Policy>(kTestPolicy);
tile_service_scheduler_ = std::make_unique<TileServiceSchedulerImpl>(
&mocked_native_scheduler_, &prefs_, &clock_, &tick_clock_,
std::move(policy));
std::move(policy), log_sink_.get());
EXPECT_CALL(
*native_scheduler(),
Cancel(static_cast<int>(background_task::TaskIds::QUERY_TILE_JOB_ID)));
Expand Down Expand Up @@ -97,7 +99,7 @@ class TileServiceSchedulerTest : public testing::Test {
base::SimpleTestTickClock tick_clock_;
TestingPrefServiceSimple prefs_;
MockBackgroundTaskScheduler mocked_native_scheduler_;

std::unique_ptr<LogSink> log_sink_;
std::unique_ptr<TileServiceScheduler> tile_service_scheduler_;
};

Expand Down
4 changes: 4 additions & 0 deletions components/query_tiles/test/fake_tile_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,8 @@ void FakeTileService::CancelTask() {}

void FakeTileService::PurgeDb() {}

Logger* FakeTileService::GetLogger() {
return nullptr;
}

} // namespace query_tiles
1 change: 1 addition & 0 deletions components/query_tiles/test/fake_tile_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class FakeTileService : public TileService {
BackgroundTaskFinishedCallback callback) override;
void CancelTask() override;
void PurgeDb() override;
Logger* GetLogger() override;

std::vector<std::unique_ptr<Tile>> tiles_;
base::WeakPtrFactory<FakeTileService> weak_ptr_factory_{this};
Expand Down
4 changes: 4 additions & 0 deletions components/query_tiles/tile_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class TileService : public KeyedService, public base::SupportsUserData {
// Used for debugging and testing only. Clear everything in db.
virtual void PurgeDb() = 0;

// Returns a Logger instance that is meant to be used by logging and debug UI
// components in the larger system.
virtual Logger* GetLogger() = 0;

TileService() = default;
~TileService() override = default;

Expand Down
10 changes: 7 additions & 3 deletions components/query_tiles/tile_service_factory_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "components/query_tiles/internal/cached_image_loader.h"
#include "components/query_tiles/internal/image_prefetcher.h"
#include "components/query_tiles/internal/init_aware_tile_service.h"
#include "components/query_tiles/internal/logger_impl.h"
#include "components/query_tiles/internal/tile_config.h"
#include "components/query_tiles/internal/tile_fetcher.h"
#include "components/query_tiles/internal/tile_manager.h"
Expand Down Expand Up @@ -71,6 +72,8 @@ std::unique_ptr<TileService> CreateTileService(
TileConfig::GetImagePrefetchMode(), std::move(image_loader));

auto* clock = base::DefaultClock::GetInstance();
auto logger = std::make_unique<LoggerImpl>();

// Create tile store and manager.
auto task_runner = base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE});
Expand All @@ -94,12 +97,13 @@ std::unique_ptr<TileService> CreateTileService(
auto tile_background_task_scheduler =
std::make_unique<TileServiceSchedulerImpl>(
scheduler, pref_service, clock, base::DefaultTickClock::GetInstance(),
std::move(policy));
std::move(policy), logger.get());
logger->SetLogSource(tile_background_task_scheduler.get());

auto tile_service_impl = std::make_unique<TileServiceImpl>(
std::move(image_prefetcher), std::move(tile_manager),
std::move(tile_background_task_scheduler), std::move(tile_fetcher),
clock);
std::move(tile_background_task_scheduler), std::move(tile_fetcher), clock,
std::move(logger));
return std::make_unique<InitAwareTileService>(std::move(tile_service_impl));
}

Expand Down

0 comments on commit 7189597

Please sign in to comment.