diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-10-12 14:27:29 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-10-13 09:35:20 +0000 |
commit | c30a6232df03e1efbd9f3b226777b07e087a1122 (patch) | |
tree | e992f45784689f373bcc38d1b79a239ebe17ee23 /chromium/components/query_tiles | |
parent | 7b5b123ac58f58ffde0f4f6e488bcd09aa4decd3 (diff) | |
download | qtwebengine-chromium-c30a6232df03e1efbd9f3b226777b07e087a1122.tar.gz |
BASELINE: Update Chromium to 85.0.4183.14085-based
Change-Id: Iaa42f4680837c57725b1344f108c0196741f6057
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/components/query_tiles')
36 files changed, 772 insertions, 96 deletions
diff --git a/chromium/components/query_tiles/BUILD.gn b/chromium/components/query_tiles/BUILD.gn index dd9269e849d..dedb4dd86be 100644 --- a/chromium/components/query_tiles/BUILD.gn +++ b/chromium/components/query_tiles/BUILD.gn @@ -24,6 +24,7 @@ group("query_tiles") { source_set("public") { sources = [ + "logger.h", "switches.cc", "switches.h", "tile.cc", diff --git a/chromium/components/query_tiles/android/java/src/org/chromium/components/query_tiles/TestTileProvider.java b/chromium/components/query_tiles/android/java/src/org/chromium/components/query_tiles/TestTileProvider.java index be6c51ee0d7..ae598f7e966 100644 --- a/chromium/components/query_tiles/android/java/src/org/chromium/components/query_tiles/TestTileProvider.java +++ b/chromium/components/query_tiles/android/java/src/org/chromium/components/query_tiles/TestTileProvider.java @@ -27,6 +27,14 @@ public class TestTileProvider implements TileProvider { } /** + * Builds and populates a {@link TestTileProvider} around the real provider. Convenient for + * matching purposes. + */ + public TestTileProvider(TileProvider realProvider) { + realProvider.getQueryTiles(tiles -> { mTiles = tiles; }); + } + + /** * Finds a tile by traversing the tree. * @param indices The indices for each child to select as the tree is traversed. * @return The matching {@link QueryTile} node. diff --git a/chromium/components/query_tiles/android/tile_provider_bridge.cc b/chromium/components/query_tiles/android/tile_provider_bridge.cc index e94cbc972c6..63f79b3aa25 100644 --- a/chromium/components/query_tiles/android/tile_provider_bridge.cc +++ b/chromium/components/query_tiles/android/tile_provider_bridge.cc @@ -10,6 +10,7 @@ #include "base/android/callback_android.h" #include "base/android/jni_string.h" +#include "base/bind.h" #include "components/query_tiles/android/tile_conversion_bridge.h" #include "components/query_tiles/jni_headers/TileProviderBridge_jni.h" #include "ui/gfx/android/java_bitmap.h" diff --git a/chromium/components/query_tiles/internal/BUILD.gn b/chromium/components/query_tiles/internal/BUILD.gn index b4b82b914c0..c5649eacd5d 100644 --- a/chromium/components/query_tiles/internal/BUILD.gn +++ b/chromium/components/query_tiles/internal/BUILD.gn @@ -9,6 +9,8 @@ if (is_android) { source_set("internal") { sources = [ + "black_hole_log_sink.cc", + "black_hole_log_sink.h", "cached_image_loader.cc", "cached_image_loader.h", "image_loader.h", @@ -16,6 +18,10 @@ source_set("internal") { "image_prefetcher.h", "init_aware_tile_service.cc", "init_aware_tile_service.h", + "log_sink.h", + "log_source.h", + "logger_impl.cc", + "logger_impl.h", "proto_conversion.cc", "proto_conversion.h", "stats.cc", diff --git a/chromium/components/query_tiles/internal/black_hole_log_sink.cc b/chromium/components/query_tiles/internal/black_hole_log_sink.cc new file mode 100644 index 00000000000..0cd6f849aef --- /dev/null +++ b/chromium/components/query_tiles/internal/black_hole_log_sink.cc @@ -0,0 +1,15 @@ +// Copyright 2020 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/query_tiles/internal/black_hole_log_sink.h" + +namespace query_tiles { +namespace test { + +void BlackHoleLogSink::OnServiceStatusChanged() {} + +void BlackHoleLogSink::OnTileDataAvailable() {} + +} // namespace test +} // namespace query_tiles diff --git a/chromium/components/query_tiles/internal/black_hole_log_sink.h b/chromium/components/query_tiles/internal/black_hole_log_sink.h new file mode 100644 index 00000000000..3fc70228b83 --- /dev/null +++ b/chromium/components/query_tiles/internal/black_hole_log_sink.h @@ -0,0 +1,30 @@ +// Copyright 2020 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_QUERY_TILES_INTERNAL_BLACK_HOLE_LOG_SINK_H_ +#define COMPONENTS_QUERY_TILES_INTERNAL_BLACK_HOLE_LOG_SINK_H_ + +#include "components/query_tiles/internal/log_sink.h" + +namespace query_tiles { +namespace test { + +// A LogSink that does nothing with the calls to the interface. +class BlackHoleLogSink : public LogSink { + public: + BlackHoleLogSink() = default; + ~BlackHoleLogSink() override = default; + + BlackHoleLogSink(const BlackHoleLogSink& other) = delete; + BlackHoleLogSink& operator=(const BlackHoleLogSink& other) = delete; + + // LogSink implementation. + void OnServiceStatusChanged() override; + void OnTileDataAvailable() override; +}; + +} // namespace test +} // namespace query_tiles + +#endif // COMPONENTS_QUERY_TILES_INTERNAL_BLACK_HOLE_LOG_SINK_H_ diff --git a/chromium/components/query_tiles/internal/init_aware_tile_service.cc b/chromium/components/query_tiles/internal/init_aware_tile_service.cc index b956ab717e1..db53ff5fea4 100644 --- a/chromium/components/query_tiles/internal/init_aware_tile_service.cc +++ b/chromium/components/query_tiles/internal/init_aware_tile_service.cc @@ -87,7 +87,21 @@ void InitAwareTileService::StartFetchForTiles( } void InitAwareTileService::CancelTask() { - tile_service_->CancelTask(); + if (IsReady()) { + tile_service_->CancelTask(); + } else if (!IsFailed()) { + MaybeCacheApiCall(base::BindOnce(&InitAwareTileService::CancelTask, + weak_ptr_factory_.GetWeakPtr())); + } +} + +void InitAwareTileService::PurgeDb() { + if (IsReady()) { + tile_service_->PurgeDb(); + } else if (!IsFailed()) { + MaybeCacheApiCall(base::BindOnce(&InitAwareTileService::PurgeDb, + weak_ptr_factory_.GetWeakPtr())); + } } void InitAwareTileService::MaybeCacheApiCall(base::OnceClosure api_call) { diff --git a/chromium/components/query_tiles/internal/init_aware_tile_service.h b/chromium/components/query_tiles/internal/init_aware_tile_service.h index 34fd44a203e..ae753d0391d 100644 --- a/chromium/components/query_tiles/internal/init_aware_tile_service.h +++ b/chromium/components/query_tiles/internal/init_aware_tile_service.h @@ -32,6 +32,7 @@ class InitAwareTileService : public TileService { void StartFetchForTiles(bool is_from_reduced_mode, BackgroundTaskFinishedCallback callback) override; void CancelTask() override; + void PurgeDb() override; void OnTileServiceInitialized(bool success); void MaybeCacheApiCall(base::OnceClosure api_call); diff --git a/chromium/components/query_tiles/internal/init_aware_tile_service_unittest.cc b/chromium/components/query_tiles/internal/init_aware_tile_service_unittest.cc index 457bd4e677c..59c5c1e8d94 100644 --- a/chromium/components/query_tiles/internal/init_aware_tile_service_unittest.cc +++ b/chromium/components/query_tiles/internal/init_aware_tile_service_unittest.cc @@ -43,6 +43,7 @@ class MockInitializableTileService : public InitializableTileService { (bool, BackgroundTaskFinishedCallback), (override)); MOCK_METHOD(void, CancelTask, (), (override)); + MOCK_METHOD(void, PurgeDb, (), (override)); // Callback stubs. MOCK_METHOD(void, GetTilesCallbackStub, (TileList), ()); diff --git a/chromium/components/query_tiles/internal/log_sink.h b/chromium/components/query_tiles/internal/log_sink.h new file mode 100644 index 00000000000..c85ca54f20d --- /dev/null +++ b/chromium/components/query_tiles/internal/log_sink.h @@ -0,0 +1,27 @@ +// Copyright 2020 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_QUERY_TILES_INTERNAL_LOG_SINK_H_ +#define COMPONENTS_QUERY_TILES_INTERNAL_LOG_SINK_H_ + +#include "components/query_tiles/internal/tile_group.h" +#include "components/query_tiles/internal/tile_types.h" + +namespace query_tiles { + +// A destination for all interesting events from internal components. +class LogSink { + public: + virtual ~LogSink() = default; + + // To be called when status of fetcher or manager changes. + virtual void OnServiceStatusChanged() = 0; + + // To be called when the tile gorup raw data is available. + virtual void OnTileDataAvailable() = 0; +}; + +} // namespace query_tiles + +#endif // COMPONENTS_QUERY_TILES_INTERNAL_LOG_SINK_H_ diff --git a/chromium/components/query_tiles/internal/log_source.h b/chromium/components/query_tiles/internal/log_source.h new file mode 100644 index 00000000000..b9f48e65614 --- /dev/null +++ b/chromium/components/query_tiles/internal/log_source.h @@ -0,0 +1,39 @@ +// Copyright 2020 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_QUERY_TILES_INTERNAL_LOG_SOURCE_H_ +#define COMPONENTS_QUERY_TILES_INTERNAL_LOG_SOURCE_H_ + +#include <string> + +#include "base/observer_list.h" +#include "components/query_tiles/internal/log_sink.h" +#include "components/query_tiles/internal/tile_types.h" +#include "components/query_tiles/logger.h" +#include "net/base/backoff_entry.h" + +namespace query_tiles { + +// A source for all relevant logging data. LoggerImpl will pull from an +// instance of LogSource to push relevant log information to observers. +class LogSource { + public: + virtual ~LogSource() = default; + + // Returns the TileFetcher status. + virtual TileInfoRequestStatus GetFetcherStatus() = 0; + + // Returns the TileManager status. + virtual TileGroupStatus GetGroupStatus() = 0; + + // Returns a serialized string of group info. + virtual std::string GetGroupInfo() = 0; + + // Returns formatted string of tiles proto data. + virtual std::string GetTilesProto() = 0; +}; + +} // namespace query_tiles + +#endif // COMPONENTS_QUERY_TILES_INTERNAL_LOG_SOURCE_H_ diff --git a/chromium/components/query_tiles/internal/logger_impl.cc b/chromium/components/query_tiles/internal/logger_impl.cc new file mode 100644 index 00000000000..165cb9ba75c --- /dev/null +++ b/chromium/components/query_tiles/internal/logger_impl.cc @@ -0,0 +1,101 @@ +// Copyright 2020 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/query_tiles/internal/logger_impl.h" + +#include "base/strings/string_number_conversions.h" +#include "base/values.h" + +namespace query_tiles { +namespace { + +std::string FetcherStatusToString(TileInfoRequestStatus status) { + switch (status) { + case TileInfoRequestStatus::kInit: + return "INITIAL"; + case TileInfoRequestStatus::kSuccess: + return "SUCCESS"; + case TileInfoRequestStatus::kFailure: + return "FAIL"; + case TileInfoRequestStatus::kShouldSuspend: + return "SUSPEND"; + default: + return "UNKNOWN"; + } +} + +std::string GroupStatusToString(TileGroupStatus status) { + switch (status) { + case TileGroupStatus::kSuccess: + return "SUCCESS"; + case TileGroupStatus::kUninitialized: + return "UN_INIT"; + case TileGroupStatus::kNoTiles: + return "NO_TILES"; + case TileGroupStatus::kFailureDbOperation: + return "DB_FAIL"; + default: + return "UNKNOWN"; + } +} + +} // namespace + +LoggerImpl::LoggerImpl() = default; + +LoggerImpl::~LoggerImpl() = default; + +void LoggerImpl::SetLogSource(LogSource* source) { + log_source_ = source; +} + +void LoggerImpl::AddObserver(Observer* observer) { + DCHECK(!observers_.HasObserver(observer)); + observers_.AddObserver(observer); +} + +void LoggerImpl::RemoveObserver(Observer* observer) { + DCHECK(observers_.HasObserver(observer)); + observers_.RemoveObserver(observer); +} + +base::Value LoggerImpl::GetServiceStatus() { + base::DictionaryValue result; + if (!log_source_) + return std::move(result); + + result.SetString("fetcherStatus", + FetcherStatusToString(log_source_->GetFetcherStatus())); + result.SetString("groupStatus", + GroupStatusToString(log_source_->GetGroupStatus())); + return std::move(result); +} + +base::Value LoggerImpl::GetTileData() { + base::DictionaryValue result; + if (!log_source_) + return std::move(result); + + result.SetString("groupInfo", log_source_->GetGroupInfo()); + result.SetString("tileProto", log_source_->GetTilesProto()); + return std::move(result); +} + +void LoggerImpl::OnServiceStatusChanged() { + if (!observers_.might_have_observers()) + return; + base::Value service_status = GetServiceStatus(); + for (auto& observer : observers_) + observer.OnServiceStatusChanged(service_status); +} + +void LoggerImpl::OnTileDataAvailable() { + if (!observers_.might_have_observers()) + return; + base::Value tile_data = GetTileData(); + for (auto& observer : observers_) + observer.OnTileDataAvailable(tile_data); +} + +} // namespace query_tiles diff --git a/chromium/components/query_tiles/internal/logger_impl.h b/chromium/components/query_tiles/internal/logger_impl.h new file mode 100644 index 00000000000..80f0de14177 --- /dev/null +++ b/chromium/components/query_tiles/internal/logger_impl.h @@ -0,0 +1,40 @@ +// Copyright 2020 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_QUERY_TILES_INTERNAL_LOGGER_IMPL_H_ +#define COMPONENTS_QUERY_TILES_INTERNAL_LOGGER_IMPL_H_ + +#include "components/query_tiles/logger.h" + +#include "base/observer_list.h" +#include "components/query_tiles/internal/log_sink.h" +#include "components/query_tiles/internal/log_source.h" + +namespace query_tiles { + +class LoggerImpl : public Logger, public LogSink { + public: + LoggerImpl(); + ~LoggerImpl() override; + + void SetLogSource(LogSource* log_source); + + private: + // Logger implementation. + void AddObserver(Observer* observer) override; + void RemoveObserver(Observer* observer) override; + base::Value GetServiceStatus() override; + base::Value GetTileData() override; + + // LogSink implementation. + void OnServiceStatusChanged() override; + void OnTileDataAvailable() override; + + LogSource* log_source_{nullptr}; + base::ObserverList<Observer>::Unchecked observers_; +}; + +} // namespace query_tiles + +#endif // COMPONENTS_QUERY_TILES_INTERNAL_LOGGER_IMPL_H_ diff --git a/chromium/components/query_tiles/internal/stats.cc b/chromium/components/query_tiles/internal/stats.cc index 21058ac35f6..87adf0db2cd 100644 --- a/chromium/components/query_tiles/internal/stats.cc +++ b/chromium/components/query_tiles/internal/stats.cc @@ -5,31 +5,60 @@ #include "components/query_tiles/internal/stats.h" #include "base/metrics/histogram_functions.h" -#include "base/metrics/histogram_macros.h" namespace query_tiles { namespace stats { +const char kImagePreloadingHistogram[] = + "Search.QueryTiles.ImagePreloadingEvent"; + +const char kHttpResponseCodeHistogram[] = + "Search.QueryTiles.FetcherHttpResponseCode"; + +const char kNetErrorCodeHistogram[] = "Search.QueryTiles.FetcherNetErrorCode"; + +const char kRequestStatusHistogram[] = "Search.QueryTiles.RequestStatus"; + +const char kGroupStatusHistogram[] = "Search.QueryTiles.GroupStatus"; + +const char kFirstFlowDurationHistogram[] = + "Search.QueryTiles.Fetcher.FirstFlowDuration"; + +const char kFetcherStartHourHistogram[] = "Search.QueryTiles.Fetcher.Start"; + +const char kPrunedGroupReasonHistogram[] = + "Search.QueryTiles.Group.PruneReason"; + void RecordImageLoading(ImagePreloadingEvent event) { - UMA_HISTOGRAM_ENUMERATION("Search.QueryTiles.ImagePreloadingEvent", event); + base::UmaHistogramEnumeration(kImagePreloadingHistogram, event); } void RecordTileFetcherResponseCode(int response_code) { - base::UmaHistogramSparse("Search.QueryTiles.FetcherHttpResponseCode", - response_code); + base::UmaHistogramSparse(kHttpResponseCodeHistogram, response_code); } void RecordTileFetcherNetErrorCode(int error_code) { - base::UmaHistogramSparse("Search.QueryTiles.FetcherNetErrorCode", - -error_code); + base::UmaHistogramSparse(kNetErrorCodeHistogram, -error_code); } void RecordTileRequestStatus(TileInfoRequestStatus status) { - UMA_HISTOGRAM_ENUMERATION("Search.QueryTiles.RequestStatus", status); + base::UmaHistogramEnumeration(kRequestStatusHistogram, status); } void RecordTileGroupStatus(TileGroupStatus status) { - UMA_HISTOGRAM_ENUMERATION("Search.QueryTiles.GroupStatus", status); + base::UmaHistogramEnumeration(kGroupStatusHistogram, status); +} + +void RecordFirstFetchFlowDuration(int hours) { + base::UmaHistogramCounts100(kFirstFlowDurationHistogram, hours); +} + +void RecordExplodeOnFetchStarted(int explode_hour) { + base::UmaHistogramExactLinear(kFetcherStartHourHistogram, explode_hour, 24); +} + +void RecordGroupPruned(PrunedGroupReason reason) { + base::UmaHistogramEnumeration(kPrunedGroupReasonHistogram, reason); } } // namespace stats diff --git a/chromium/components/query_tiles/internal/stats.h b/chromium/components/query_tiles/internal/stats.h index 84ee417fc0b..f0467645866 100644 --- a/chromium/components/query_tiles/internal/stats.h +++ b/chromium/components/query_tiles/internal/stats.h @@ -10,6 +10,22 @@ namespace query_tiles { namespace stats { +extern const char kImagePreloadingHistogram[]; + +extern const char kHttpResponseCodeHistogram[]; + +extern const char kNetErrorCodeHistogram[]; + +extern const char kRequestStatusHistogram[]; + +extern const char kGroupStatusHistogram[]; + +extern const char kFirstFlowDurationHistogram[]; + +extern const char kFetcherStartHourHistogram[]; + +extern const char kPrunedGroupReasonHistogram[]; + // Event to track image loading metrics. enum class ImagePreloadingEvent { // Start to fetch image in full browser mode. @@ -27,6 +43,14 @@ enum class ImagePreloadingEvent { kMaxValue = kFailureReducedMode, }; +enum class PrunedGroupReason { + // Group has expired. + kExpired = 0, + // Locale mismatched. + kInvalidLocale = 1, + kMaxValue = kInvalidLocale, +}; + // Records an image preloading event. void RecordImageLoading(ImagePreloadingEvent event); @@ -42,6 +66,16 @@ void RecordTileRequestStatus(TileInfoRequestStatus status); // Records status of tile group. void RecordTileGroupStatus(TileGroupStatus status); +// Records the number of hours passed from first time schedule to first time +// run. +void RecordFirstFetchFlowDuration(int hours); + +// Records the locale explode hour when fetching starts. +void RecordExplodeOnFetchStarted(int explode); + +// Records the reason to cause TileManager to prune the group. +void RecordGroupPruned(PrunedGroupReason reason); + } // namespace stats } // namespace query_tiles diff --git a/chromium/components/query_tiles/internal/stats_unittest.cc b/chromium/components/query_tiles/internal/stats_unittest.cc index 918f1f27a16..fc7550c687e 100644 --- a/chromium/components/query_tiles/internal/stats_unittest.cc +++ b/chromium/components/query_tiles/internal/stats_unittest.cc @@ -10,13 +10,54 @@ namespace query_tiles { namespace { -const char kImagePreloadingHistogram[] = - "Search.QueryTiles.ImagePreloadingEvent"; - -TEST(StatsTest, RecordImageLoading) { +TEST(QueryTilesStatsTest, RecordImageLoading) { base::HistogramTester tester; stats::RecordImageLoading(stats::ImagePreloadingEvent::kStart); - tester.ExpectBucketCount(kImagePreloadingHistogram, 0, 1); + tester.ExpectBucketCount(stats::kImagePreloadingHistogram, 0, 1); +} + +TEST(QueryTilesStatsTest, RecordTileFetcherResponseCode) { + base::HistogramTester tester; + stats::RecordTileFetcherResponseCode(200); + tester.ExpectBucketCount(stats::kHttpResponseCodeHistogram, 200, 1); +} + +TEST(QueryTilesStatsTest, RecordTileFetcherNetErrorCode) { + base::HistogramTester tester; + stats::RecordTileFetcherNetErrorCode(105); + tester.ExpectBucketCount(stats::kNetErrorCodeHistogram, -105, 1); +} + +TEST(QueryTilesStatsTest, RecordTileRequestStatus) { + base::HistogramTester tester; + stats::RecordTileRequestStatus(TileInfoRequestStatus::kSuccess); + tester.ExpectBucketCount(stats::kRequestStatusHistogram, 1, 1); +} + +TEST(QueryTilesStatsTest, RecordTileGroupStatus) { + base::HistogramTester tester; + stats::RecordTileGroupStatus(TileGroupStatus::kNoTiles); + tester.ExpectBucketCount(stats::kGroupStatusHistogram, 3, 1); +} + +TEST(QueryTilesStatsTest, RecordFirstFetchFlowDuration) { + base::HistogramTester tester; + stats::RecordFirstFetchFlowDuration(18); + tester.ExpectBucketCount(stats::kFirstFlowDurationHistogram, 18, 1); +} + +TEST(QueryTilesStatsTest, RecordExplodeOnFetchStarted) { + base::HistogramTester tester; + stats::RecordExplodeOnFetchStarted(12); + tester.ExpectBucketCount(stats::kFetcherStartHourHistogram, 12, 1); +} + +TEST(QueryTilesStatsTest, RecordGroupPruned) { + base::HistogramTester tester; + stats::RecordGroupPruned(stats::PrunedGroupReason::kExpired); + stats::RecordGroupPruned(stats::PrunedGroupReason::kInvalidLocale); + tester.ExpectBucketCount(stats::kPrunedGroupReasonHistogram, 0, 1); + tester.ExpectBucketCount(stats::kPrunedGroupReasonHistogram, 1, 1); } } // namespace diff --git a/chromium/components/query_tiles/internal/tile_config.cc b/chromium/components/query_tiles/internal/tile_config.cc index a1ebce25e2f..af1799474ef 100644 --- a/chromium/components/query_tiles/internal/tile_config.cc +++ b/chromium/components/query_tiles/internal/tile_config.cc @@ -11,8 +11,7 @@ namespace query_tiles { // Default base URL string for the Query Tiles server. -constexpr char kDefaultBaseURL[] = - "https://autopush-gsaprototype-pa.sandbox.googleapis.com"; +constexpr char kDefaultBaseURL[] = "https://chromeupboarding-pa.googleapis.com"; // Default URL string for GetQueryTiles RPC. constexpr char kDefaultGetQueryTilePath[] = "/v1/querytiles"; @@ -58,7 +57,8 @@ constexpr int kDefaultScheduleInterval = 12 * 3600 * 1000; // 12 hours. // Default length of random window added to the interval. constexpr int kDefaultRandomWindow = 4 * 3600 * 1000; // 4 hours. -// Default length of random window added to the interval. +// Default delta value from start window time to end window time in one-off +// background task. constexpr int kDefaultOneoffTaskWindow = 2 * 3600 * 1000; // 2 hours. // Default initial delay in backoff policy. diff --git a/chromium/components/query_tiles/internal/tile_config.h b/chromium/components/query_tiles/internal/tile_config.h index 35719265d98..e2cc5b7581a 100644 --- a/chromium/components/query_tiles/internal/tile_config.h +++ b/chromium/components/query_tiles/internal/tile_config.h @@ -33,7 +33,7 @@ extern const char kIsUnmeteredNetworkRequiredKey[]; extern const char kImagePrefetchModeKey[]; // Finch parameter key for the minimum interval to next schedule. -extern const char kNextScheduleMinIntervalKey[]; +extern const char kScheduleIntervalKey[]; // Finch parameter key for random window. extern const char kMaxRandomWindowKey[]; @@ -41,6 +41,12 @@ extern const char kMaxRandomWindowKey[]; // Finch parameter key for one off task window. extern const char kOneoffTaskWindowKey[]; +// Finch parameter key for Backoff policy initial delay in ms. +extern const char kBackoffInitDelayInMsKey[]; + +// Finch parameter key for Backoff policy maximum delay in ms. +extern const char kBackoffMaxDelayInMsKey[]; + class TileConfig { public: // Gets the URL for the Query Tiles server. diff --git a/chromium/components/query_tiles/internal/tile_config_unittest.cc b/chromium/components/query_tiles/internal/tile_config_unittest.cc index 21053d8d78f..47df101d354 100644 --- a/chromium/components/query_tiles/internal/tile_config_unittest.cc +++ b/chromium/components/query_tiles/internal/tile_config_unittest.cc @@ -27,10 +27,11 @@ void TestImagePrefetchMode(std::map<std::string, std::string> params, TEST(TileConfigTest, FinchConfigEnabled) { base::test::ScopedFeatureList feature_list; std::map<std::string, std::string> params = { - {kExperimentTagKey, "1234"}, - {kBaseURLKey, "https://test.com"}, - {kExpireDurationKey, "100"}, - {kIsUnmeteredNetworkRequiredKey, "true"}}; + {kExperimentTagKey, "1234"}, {kBaseURLKey, "https://test.com"}, + {kExpireDurationKey, "100"}, {kIsUnmeteredNetworkRequiredKey, "true"}, + {kScheduleIntervalKey, "123"}, {kMaxRandomWindowKey, "234"}, + {kOneoffTaskWindowKey, "345"}, {kBackoffInitDelayInMsKey, "456"}, + {kBackoffMaxDelayInMsKey, "567"}}; feature_list.InitAndEnableFeatureWithParameters(features::kQueryTiles, params); EXPECT_EQ(TileConfig::GetQueryTilesServerUrl(), @@ -38,19 +39,32 @@ TEST(TileConfigTest, FinchConfigEnabled) { EXPECT_TRUE(TileConfig::GetIsUnMeteredNetworkRequired()); EXPECT_EQ(TileConfig::GetExperimentTag(), "1234"); EXPECT_EQ(TileConfig::GetExpireDuration(), base::TimeDelta::FromSeconds(100)); + EXPECT_EQ(TileConfig::GetScheduleIntervalInMs(), 123); + EXPECT_EQ(TileConfig::GetMaxRandomWindowInMs(), 234); + EXPECT_EQ(TileConfig::GetOneoffTaskWindowInMs(), 345); + EXPECT_EQ(TileConfig::GetBackoffPolicyArgsInitDelayInMs(), 456); + EXPECT_EQ(TileConfig::GetBackoffPolicyArgsMaxDelayInMs(), 567); } // Test to verify that if not configured, default parameter values are used. TEST(TileConfigTest, FinchConfigDefaultParameter) { base::test::ScopedFeatureList feature_list; feature_list.InitAndEnableFeature(features::kQueryTiles); - EXPECT_EQ( - TileConfig::GetQueryTilesServerUrl(), - "https://autopush-gsaprototype-pa.sandbox.googleapis.com/v1/querytiles"); + EXPECT_EQ(TileConfig::GetQueryTilesServerUrl(), + "https://chromeupboarding-pa.googleapis.com/v1/querytiles"); EXPECT_FALSE(TileConfig::GetIsUnMeteredNetworkRequired()); EXPECT_TRUE(TileConfig::GetExperimentTag().empty()); - EXPECT_EQ(TileConfig::GetExpireDuration(), - base::TimeDelta::FromSeconds(48 * 60 * 60)); + EXPECT_EQ(TileConfig::GetExpireDuration(), base::TimeDelta::FromDays(2)); + EXPECT_EQ(TileConfig::GetScheduleIntervalInMs(), + base::TimeDelta::FromHours(12).InMilliseconds()); + EXPECT_EQ(TileConfig::GetMaxRandomWindowInMs(), + base::TimeDelta::FromHours(4).InMilliseconds()); + EXPECT_EQ(TileConfig::GetOneoffTaskWindowInMs(), + base::TimeDelta::FromHours(2).InMilliseconds()); + EXPECT_EQ(TileConfig::GetBackoffPolicyArgsInitDelayInMs(), + base::TimeDelta::FromSeconds(30).InMilliseconds()); + EXPECT_EQ(TileConfig::GetBackoffPolicyArgsMaxDelayInMs(), + base::TimeDelta::FromDays(1).InMilliseconds()); } // Test to verify ImagePrefetchMode can be parsed correctly from Finch diff --git a/chromium/components/query_tiles/internal/tile_fetcher.cc b/chromium/components/query_tiles/internal/tile_fetcher.cc index 35394a83e70..9d6d92533c4 100644 --- a/chromium/components/query_tiles/internal/tile_fetcher.cc +++ b/chromium/components/query_tiles/internal/tile_fetcher.cc @@ -6,6 +6,7 @@ #include <utility> +#include "base/lazy_instance.h" #include "components/query_tiles/internal/stats.h" #include "net/base/url_util.h" #include "net/http/http_request_headers.h" @@ -18,6 +19,9 @@ namespace query_tiles { namespace { +// An override server URL for testing. +base::LazyInstance<GURL>::Leaky g_override_url_for_testing; + const char kRequestContentType[] = "application/x-protobuf"; constexpr net::NetworkTrafficAnnotationTag kQueryTilesFetcherTrafficAnnotation = @@ -94,6 +98,10 @@ class TileFetcherImpl : public TileFetcher { request->headers.SetHeader(net::HttpRequestHeaders::kAcceptLanguage, accept_languages_); } + + if (!g_override_url_for_testing.Get().is_empty()) + request->url = g_override_url_for_testing.Get(); + return request; } @@ -175,6 +183,11 @@ std::unique_ptr<TileFetcher> TileFetcher::Create( client_version, url_loader_factory); } +// static +void TileFetcher::SetOverrideURLForTesting(const GURL& url) { + g_override_url_for_testing.Get() = url; +} + TileFetcher::TileFetcher() = default; TileFetcher::~TileFetcher() = default; diff --git a/chromium/components/query_tiles/internal/tile_fetcher.h b/chromium/components/query_tiles/internal/tile_fetcher.h index 1f3871e2f35..eaacdd4ccf4 100644 --- a/chromium/components/query_tiles/internal/tile_fetcher.h +++ b/chromium/components/query_tiles/internal/tile_fetcher.h @@ -39,6 +39,9 @@ class TileFetcher { const std::string& client_version, const scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory); + // For testing only. + static void SetOverrideURLForTesting(const GURL& url); + // Start the fetch to download tiles. virtual void StartFetchForTiles(FinishedCallback callback) = 0; diff --git a/chromium/components/query_tiles/internal/tile_iterator.h b/chromium/components/query_tiles/internal/tile_iterator.h index 1b17a64f26b..f6eda0e105d 100644 --- a/chromium/components/query_tiles/internal/tile_iterator.h +++ b/chromium/components/query_tiles/internal/tile_iterator.h @@ -26,7 +26,6 @@ struct TileGroup; // } // } // -// class TileIterator { public: // Pass to |levels_| to iterates through all tiles. diff --git a/chromium/components/query_tiles/internal/tile_manager.cc b/chromium/components/query_tiles/internal/tile_manager.cc index d0d3378d0a9..0e8c812b3be 100644 --- a/chromium/components/query_tiles/internal/tile_manager.cc +++ b/chromium/components/query_tiles/internal/tile_manager.cc @@ -13,6 +13,7 @@ #include "base/task_runner.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" +#include "components/query_tiles/internal/stats.h" #include "components/query_tiles/internal/tile_config.h" #include "components/query_tiles/internal/tile_iterator.h" #include "components/query_tiles/internal/tile_manager.h" @@ -85,6 +86,18 @@ class TileManagerImpl : public TileManager { FROM_HERE, base::BindOnce(std::move(callback), result_tile)); } + TileGroupStatus PurgeDb() override { + if (!initialized_) + return TileGroupStatus::kUninitialized; + if (!tile_group_) + return TileGroupStatus::kNoTiles; + store_->Delete(tile_group_->id, + base::BindOnce(&TileManagerImpl::OnGroupDeleted, + weak_ptr_factory_.GetWeakPtr())); + tile_group_.reset(); + return TileGroupStatus::kNoTiles; + } + void SetAcceptLanguagesForTesting( const std::string& accept_languages) override { accept_languages_ = accept_languages; @@ -145,26 +158,29 @@ class TileManagerImpl : public TileManager { // Returns true if the group is expired. bool IsGroupExpired(const TileGroup* group) const { - return clock_->Now() >= - group->last_updated_ts + TileConfig::GetExpireDuration(); + if (clock_->Now() >= + group->last_updated_ts + TileConfig::GetExpireDuration()) { + stats::RecordGroupPruned(stats::PrunedGroupReason::kExpired); + return true; + } + return false; } // Check whether |locale_| matches with that of the |group|. bool ValidateLocale(const TileGroup* group) const { - if (accept_languages_.empty() || group->locale.empty()) - return false; - - // In case the primary language matches (en-GB vs en-IN), consider - // those are matching. - std::string group_primary = - group->locale.substr(0, group->locale.find("-")); - for (auto& lang : - base::SplitString(accept_languages_, ",", base::TRIM_WHITESPACE, - base::SPLIT_WANT_NONEMPTY)) { - if (lang.substr(0, lang.find("-")) == group_primary) - return true; + if (!accept_languages_.empty() && !group->locale.empty()) { + // In case the primary language matches (en-GB vs en-IN), consider + // those are matching. + std::string group_primary = + group->locale.substr(0, group->locale.find("-")); + for (auto& lang : + base::SplitString(accept_languages_, ",", base::TRIM_WHITESPACE, + base::SPLIT_WANT_NONEMPTY)) { + if (lang.substr(0, lang.find("-")) == group_primary) + return true; + } } - + stats::RecordGroupPruned(stats::PrunedGroupReason::kInvalidLocale); return false; } diff --git a/chromium/components/query_tiles/internal/tile_manager.h b/chromium/components/query_tiles/internal/tile_manager.h index d13c0e6ffed..c5b211fbb32 100644 --- a/chromium/components/query_tiles/internal/tile_manager.h +++ b/chromium/components/query_tiles/internal/tile_manager.h @@ -47,6 +47,9 @@ class TileManager { virtual void SaveTiles(std::unique_ptr<TileGroup> tile_group, TileGroupStatusCallback callback) = 0; + // Delete everything in db. Used for debugging and testing only. + virtual TileGroupStatus PurgeDb() = 0; + virtual void SetAcceptLanguagesForTesting( const std::string& accept_languages) = 0; diff --git a/chromium/components/query_tiles/internal/tile_manager_unittest.cc b/chromium/components/query_tiles/internal/tile_manager_unittest.cc index 7d87399fe2a..f1067084217 100644 --- a/chromium/components/query_tiles/internal/tile_manager_unittest.cc +++ b/chromium/components/query_tiles/internal/tile_manager_unittest.cc @@ -16,9 +16,9 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -using testing::_; -using testing::Invoke; -using testing::StrictMock; +using ::testing::_; +using ::testing::Invoke; +using ::testing::StrictMock; namespace query_tiles { namespace { @@ -28,10 +28,17 @@ const char kGuid[] = "awesome_guid"; class MockTileStore : public Store<TileGroup> { public: MockTileStore() = default; - MOCK_METHOD1(InitAndLoad, void(TileStore::LoadCallback)); - MOCK_METHOD3(Update, - void(const std::string&, const TileGroup&, UpdateCallback)); - MOCK_METHOD2(Delete, void(const std::string&, TileStore::DeleteCallback)); + ~MockTileStore() override = default; + + MOCK_METHOD(void, InitAndLoad, (TileStore::LoadCallback), (override)); + MOCK_METHOD(void, + Update, + (const std::string&, const TileGroup&, UpdateCallback), + (override)); + MOCK_METHOD(void, + Delete, + (const std::string&, TileStore::DeleteCallback), + (override)); }; class TileManagerTest : public testing::Test { @@ -51,17 +58,14 @@ class TileManagerTest : public testing::Test { manager_ = TileManager::Create(std::move(tile_store), &clock_, "en-US"); } - // Initial and load entries from store_, compare the |expected_status| to the + // Initialize the store, compare the |expected_status| to the // actual returned status. - void Init(TileGroupStatus expected_status) { - base::RunLoop loop; - manager()->Init(base::BindOnce(&TileManagerTest::OnInitCompleted, - base::Unretained(this), loop.QuitClosure(), - expected_status)); - loop.Run(); + void Init(TileGroupStatus expected_status, bool success = true) { + InitWithData(expected_status, std::vector<TileGroup>(), success); } - // TODO(crbug.com/1078163): Replace Init() with InitWithData. + // Initialize the store and load |groups|, compare the |expected_status| to + // the actual returned status. void InitWithData(TileGroupStatus expected_status, std::vector<TileGroup> groups, bool success = true) { @@ -175,13 +179,7 @@ class TileManagerTest : public testing::Test { }; TEST_F(TileManagerTest, InitAndLoadWithDbOperationFailed) { - EXPECT_CALL(*tile_store(), InitAndLoad(_)) - .WillOnce(Invoke([](base::OnceCallback<void( - bool, MockTileStore::KeysAndEntries)> callback) { - std::move(callback).Run(false, MockTileStore::KeysAndEntries()); - })); - - Init(TileGroupStatus::kFailureDbOperation); + Init(TileGroupStatus::kFailureDbOperation, false /*success*/); GetTiles(std::vector<Tile>() /*expect an empty result*/); } @@ -222,15 +220,9 @@ TEST_F(TileManagerTest, InitAndLoadSuccess) { // Failed to init an empty db, and save tiles call failed because of db is // uninitialized. GetTiles should return empty result. TEST_F(TileManagerTest, SaveTilesWhenUnintialized) { - EXPECT_CALL(*tile_store(), InitAndLoad(_)) - .WillOnce(Invoke([](base::OnceCallback<void( - bool, MockTileStore::KeysAndEntries)> callback) { - std::move(callback).Run(false, MockTileStore::KeysAndEntries()); - })); EXPECT_CALL(*tile_store(), Update(_, _, _)).Times(0); EXPECT_CALL(*tile_store(), Delete(_, _)).Times(0); - - Init(TileGroupStatus::kFailureDbOperation); + Init(TileGroupStatus::kFailureDbOperation, false /*success*/); auto tile_to_save = std::make_unique<Tile>(); test::ResetTestEntry(tile_to_save.get()); @@ -244,18 +236,12 @@ TEST_F(TileManagerTest, SaveTilesWhenUnintialized) { // Init with empty db successfully, and save tiles failed because of db // operation failed. GetTiles should return empty result. TEST_F(TileManagerTest, SaveTilesFailed) { - EXPECT_CALL(*tile_store(), InitAndLoad(_)) - .WillOnce(Invoke([](base::OnceCallback<void( - bool, MockTileStore::KeysAndEntries)> callback) { - std::move(callback).Run(true, MockTileStore::KeysAndEntries()); - })); EXPECT_CALL(*tile_store(), Update(_, _, _)) .WillOnce(Invoke([](const std::string& id, const TileGroup& group, MockTileStore::UpdateCallback callback) { std::move(callback).Run(false); })); EXPECT_CALL(*tile_store(), Delete(_, _)).Times(0); - Init(TileGroupStatus::kNoTiles); auto tile_to_save = std::make_unique<Tile>(); @@ -270,18 +256,12 @@ TEST_F(TileManagerTest, SaveTilesFailed) { // Init with empty db successfully, and save tiles successfully. GetTiles should // return the recent saved tiles, and no Delete call is executed. TEST_F(TileManagerTest, SaveTilesSuccess) { - EXPECT_CALL(*tile_store(), InitAndLoad(_)) - .WillOnce(Invoke([](base::OnceCallback<void( - bool, MockTileStore::KeysAndEntries)> callback) { - std::move(callback).Run(true, MockTileStore::KeysAndEntries()); - })); EXPECT_CALL(*tile_store(), Update(_, _, _)) .WillOnce(Invoke([](const std::string& id, const TileGroup& group, MockTileStore::UpdateCallback callback) { std::move(callback).Run(true); })); EXPECT_CALL(*tile_store(), Delete(_, _)).Times(0); - Init(TileGroupStatus::kNoTiles); auto tile_to_save = std::make_unique<Tile>(); @@ -348,6 +328,15 @@ TEST_F(TileManagerTest, GetTilesWithMatchingAcceptLanguages) { GetTiles({tile}); } +TEST_F(TileManagerTest, PurgeDb) { + TileGroup group; + test::ResetTestGroup(&group); + InitWithData(TileGroupStatus::kSuccess, {group}); + EXPECT_CALL(*tile_store(), Delete(group.id, _)); + manager()->PurgeDb(); + GetTiles(std::vector<Tile>() /*expect an empty result*/); +} + } // namespace } // namespace query_tiles diff --git a/chromium/components/query_tiles/internal/tile_service_impl.cc b/chromium/components/query_tiles/internal/tile_service_impl.cc index 3c6774e3a75..16317ce8594 100644 --- a/chromium/components/query_tiles/internal/tile_service_impl.cc +++ b/chromium/components/query_tiles/internal/tile_service_impl.cc @@ -46,7 +46,6 @@ void TileServiceImpl::OnTileManagerInitialized(SuccessCallback callback, status == TileGroupStatus::kNoTiles); DCHECK(callback); scheduler_->OnTileManagerInitialized(status); - stats::RecordTileGroupStatus(status); std::move(callback).Run(success); } @@ -66,12 +65,21 @@ void TileServiceImpl::StartFetchForTiles( tile_fetcher_->StartFetchForTiles(base::BindOnce( &TileServiceImpl::OnFetchFinished, weak_ptr_factory_.GetWeakPtr(), is_from_reduced_mode, std::move(task_finished_callback))); + + base::Time::Exploded local_explode; + base::Time::Now().LocalExplode(&local_explode); + stats::RecordExplodeOnFetchStarted(local_explode.hour); } void TileServiceImpl::CancelTask() { scheduler_->CancelTask(); } +void TileServiceImpl::PurgeDb() { + auto status = tile_manager_->PurgeDb(); + scheduler_->OnDbPurged(status); +} + void TileServiceImpl::OnFetchFinished( bool is_from_reduced_mode, BackgroundTaskFinishedCallback task_finished_callback, @@ -99,7 +107,6 @@ void TileServiceImpl::OnFetchFinished( std::move(task_finished_callback).Run(false /*reschedule*/); } scheduler_->OnFetchCompleted(status); - stats::RecordTileRequestStatus(status); } void TileServiceImpl::OnTilesSaved( diff --git a/chromium/components/query_tiles/internal/tile_service_impl.h b/chromium/components/query_tiles/internal/tile_service_impl.h index b4211f3b5bc..978afbe6166 100644 --- a/chromium/components/query_tiles/internal/tile_service_impl.h +++ b/chromium/components/query_tiles/internal/tile_service_impl.h @@ -51,6 +51,7 @@ class TileServiceImpl : public InitializableTileService { void StartFetchForTiles(bool is_from_reduced_mode, BackgroundTaskFinishedCallback callback) override; void CancelTask() override; + void PurgeDb() override; // Called when tile manager is initialized. void OnTileManagerInitialized(SuccessCallback callback, diff --git a/chromium/components/query_tiles/internal/tile_service_impl_unittest.cc b/chromium/components/query_tiles/internal/tile_service_impl_unittest.cc index 57710c8ea6e..f035492367e 100644 --- a/chromium/components/query_tiles/internal/tile_service_impl_unittest.cc +++ b/chromium/components/query_tiles/internal/tile_service_impl_unittest.cc @@ -36,6 +36,7 @@ class MockTileManager : public TileManager { MOCK_METHOD2(SaveTiles, void(std::unique_ptr<TileGroup>, TileGroupStatusCallback)); MOCK_METHOD1(SetAcceptLanguagesForTesting, void(const std::string&)); + MOCK_METHOD0(PurgeDb, TileGroupStatus()); }; class MockTileServiceScheduler : public TileServiceScheduler { @@ -44,6 +45,7 @@ class MockTileServiceScheduler : public TileServiceScheduler { MOCK_METHOD1(OnFetchCompleted, void(TileInfoRequestStatus)); MOCK_METHOD1(OnTileManagerInitialized, void(TileGroupStatus)); MOCK_METHOD0(CancelTask, void()); + MOCK_METHOD1(OnDbPurged, void(TileGroupStatus)); }; class MockImagePrefetcher : public ImagePrefetcher { @@ -104,6 +106,15 @@ class TileServiceImplTest : public testing::Test { std::move(closure).Run(); } + void OnGetTileDone(const std::string& expected_id, + base::Optional<Tile> actual_tile) { + EXPECT_EQ(expected_id, actual_tile->id); + } + + void OnGetTilesDone(size_t expected_size, std::vector<Tile> tiles) { + EXPECT_EQ(expected_size, tiles.size()); + } + void FetchForTilesSuceeded() { EXPECT_CALL(*scheduler(), OnFetchCompleted(TileInfoRequestStatus::kSuccess)); @@ -137,6 +148,7 @@ class TileServiceImplTest : public testing::Test { MockTileServiceScheduler* scheduler() { return scheduler_; } MockTileManager* tile_manager() { return tile_manager_; } MockImagePrefetcher* image_prefetcher() { return image_prefetcher_; } + TileService* query_tiles_service() { return tile_service_impl_.get(); } private: base::test::TaskEnvironment task_environment_; @@ -199,4 +211,44 @@ TEST_F(TileServiceImplTest, FetchForTilesFailed) { FetchForTilesWithError(); } +TEST_F(TileServiceImplTest, CancelTask) { + EXPECT_CALL(*scheduler(), CancelTask()); + query_tiles_service()->CancelTask(); +} + +TEST_F(TileServiceImplTest, GetTiles) { + int expected_size = 5; + EXPECT_CALL(*tile_manager(), GetTiles(_)) + .WillOnce(Invoke([&](GetTilesCallback callback) { + std::vector<Tile> out = std::vector<Tile>(expected_size, Tile()); + std::move(callback).Run(std::move(out)); + })); + query_tiles_service()->GetQueryTiles( + base::BindOnce(&TileServiceImplTest::OnGetTilesDone, + base::Unretained(this), expected_size)); +} + +TEST_F(TileServiceImplTest, GetTile) { + std::string tile_id = "test-id"; + EXPECT_CALL(*tile_manager(), GetTile(tile_id, _)) + .WillOnce(Invoke([&](const std::string& id, TileCallback callback) { + EXPECT_EQ(id, tile_id); + Tile out; + out.id = tile_id; + std::move(callback).Run(std::move(out)); + })); + query_tiles_service()->GetTile( + tile_id, base::BindOnce(&TileServiceImplTest::OnGetTileDone, + base::Unretained(this), tile_id)); +} + +TEST_F(TileServiceImplTest, PurgeDb) { + EXPECT_CALL(*tile_manager(), PurgeDb()); + EXPECT_CALL(*tile_manager(), GetTiles(_)); + EXPECT_CALL(*scheduler(), OnDbPurged(_)); + query_tiles_service()->PurgeDb(); + query_tiles_service()->GetQueryTiles(base::BindOnce( + &TileServiceImplTest::OnGetTilesDone, base::Unretained(this), 0)); +} + } // namespace query_tiles diff --git a/chromium/components/query_tiles/internal/tile_service_scheduler.cc b/chromium/components/query_tiles/internal/tile_service_scheduler.cc index 80494d5362d..190e13c828c 100644 --- a/chromium/components/query_tiles/internal/tile_service_scheduler.cc +++ b/chromium/components/query_tiles/internal/tile_service_scheduler.cc @@ -11,6 +11,7 @@ #include "base/rand_util.h" #include "base/time/default_tick_clock.h" #include "components/prefs/pref_service.h" +#include "components/query_tiles/internal/stats.h" #include "components/query_tiles/internal/tile_config.h" #include "components/query_tiles/switches.h" #include "net/base/backoff_entry_serializer.h" @@ -43,12 +44,27 @@ class TileServiceSchedulerImpl : public TileServiceScheduler { scheduler_->Cancel( static_cast<int>(background_task::TaskIds::QUERY_TILE_JOB_ID)); ResetBackoff(); + MarkFirstRunFinished(); } + void OnDbPurged(TileGroupStatus status) override { CancelTask(); } + void OnFetchCompleted(TileInfoRequestStatus status) override { + MarkFirstRunFinished(); + if (IsInstantFetchMode()) return; + // If this task was marked at first flow, record the duration, and mark the + // flow is finished now. + if (IsDuringFirstFlow()) { + auto first_schedule_time = prefs_->GetTime(kFirstScheduleTimeKey); + auto hours_past = (clock_->Now() - first_schedule_time).InHours(); + if (hours_past >= 0) { + stats::RecordFirstFetchFlowDuration(hours_past); + } + } + if (status == TileInfoRequestStatus::kShouldSuspend) { MaximizeBackoff(); ScheduleTask(false); @@ -60,6 +76,7 @@ class TileServiceSchedulerImpl : public TileServiceScheduler { ResetBackoff(); ScheduleTask(true); } + stats::RecordTileRequestStatus(status); } void OnTileManagerInitialized(TileGroupStatus status) override { @@ -69,14 +86,17 @@ class TileServiceSchedulerImpl : public TileServiceScheduler { return; } - if (status == TileGroupStatus::kNoTiles && !is_suspend_) { + if (status == TileGroupStatus::kNoTiles && !is_suspend_ && + !IsDuringFirstFlow()) { ResetBackoff(); ScheduleTask(true); + MarkFirstRunScheduled(); } else if (status == TileGroupStatus::kFailureDbOperation) { MaximizeBackoff(); ScheduleTask(false); is_suspend_ = true; } + stats::RecordTileGroupStatus(status); } void ScheduleTask(bool is_init_schedule) { @@ -176,6 +196,22 @@ class TileServiceSchedulerImpl : public TileServiceScheduler { switches::kQueryTilesInstantBackgroundTask); } + void MarkFirstRunScheduled() { + prefs_->SetTime(kFirstScheduleTimeKey, clock_->Now()); + } + + void MarkFirstRunFinished() { + prefs_->SetTime(kFirstScheduleTimeKey, base::Time()); + } + + // 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() { + return prefs_->GetTime(kFirstScheduleTimeKey) != base::Time(); + } + // Native Background Scheduler instance. background_task::BackgroundTaskScheduler* scheduler_; diff --git a/chromium/components/query_tiles/internal/tile_service_scheduler.h b/chromium/components/query_tiles/internal/tile_service_scheduler.h index 2e292bafaec..397ea708e7b 100644 --- a/chromium/components/query_tiles/internal/tile_service_scheduler.h +++ b/chromium/components/query_tiles/internal/tile_service_scheduler.h @@ -44,6 +44,9 @@ class TileServiceScheduler { // directly set the release time until 24 hours later. virtual void OnTileManagerInitialized(TileGroupStatus status) = 0; + // Called when datanase is purged. Reset the flow and update the status. + virtual void OnDbPurged(TileGroupStatus status) = 0; + // Cancel current existing task, and reset backoff. virtual void CancelTask() = 0; diff --git a/chromium/components/query_tiles/internal/tile_service_scheduler_unittest.cc b/chromium/components/query_tiles/internal/tile_service_scheduler_unittest.cc index dcd62b334df..7c1fbf44190 100644 --- a/chromium/components/query_tiles/internal/tile_service_scheduler_unittest.cc +++ b/chromium/components/query_tiles/internal/tile_service_scheduler_unittest.cc @@ -27,6 +27,15 @@ using ::testing::Invoke; namespace query_tiles { namespace { +constexpr net::BackoffEntry::Policy kTestPolicy = { + 0 /*num_errors_to_ignore*/, + 1000 /*init_delay_ms*/, + 2 /*multiply_factor*/, + 0 /*jitter_factor*/, + 4000 /*max_backoff_ms*/, + -1 /*entry_lifetime_ms*/, + false /*always_use_init_delay*/}; + class MockBackgroundTaskScheduler : public background_task::BackgroundTaskScheduler { public: @@ -46,14 +55,7 @@ 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()); - auto policy = std::make_unique<net::BackoffEntry::Policy>(); - policy->num_errors_to_ignore = 0; - policy->initial_delay_ms = 1 * 1000; - policy->multiply_factor = 2; - policy->jitter_factor = 0; - policy->maximum_backoff_ms = 4 * 1000; - policy->always_use_initial_delay = false; - policy->entry_lifetime_ms = -1; + auto policy = std::make_unique<net::BackoffEntry::Policy>(kTestPolicy); tile_service_scheduler_ = TileServiceScheduler::Create(&mocked_native_scheduler_, &prefs_, &clock_, &tick_clock_, std::move(policy)); @@ -67,11 +69,27 @@ class TileServiceSchedulerTest : public testing::Test { MockBackgroundTaskScheduler* native_scheduler() { return &mocked_native_scheduler_; } + TileServiceScheduler* tile_service_scheduler() { return tile_service_scheduler_.get(); } + TestingPrefServiceSimple* prefs() { return &prefs_; } + base::SimpleTestClock* clock() { return &clock_; } + + base::SimpleTestTickClock* tick_clock() { return &tick_clock_; } + + std::unique_ptr<net::BackoffEntry> GetBackoffPolicy() { + std::unique_ptr<net::BackoffEntry> result; + const base::ListValue* value = prefs()->GetList(kBackoffEntryKey); + if (value) { + result = net::BackoffEntrySerializer::DeserializeFromValue( + *value, &kTestPolicy, tick_clock(), clock()->Now()); + } + return result; + } + private: base::test::TaskEnvironment task_environment_; @@ -133,6 +151,8 @@ TEST_F(TileServiceSchedulerTest, OnFetchCompletedSuspend) { EXPECT_CALL(*native_scheduler(), Schedule(TaskInfoEq(4000, 4000))); tile_service_scheduler()->OnFetchCompleted( TileInfoRequestStatus::kShouldSuspend); + auto backoff = GetBackoffPolicy(); + EXPECT_EQ(backoff->GetTimeUntilRelease().InMilliseconds(), 4000); } // Verify the failure will add delay that using test backoff policy. @@ -141,6 +161,9 @@ TEST_F(TileServiceSchedulerTest, OnFetchCompletedFailure) { EXPECT_CALL(*native_scheduler(), Schedule(TaskInfoEq(1000 * pow(2, i), 1000 * pow(2, i)))); tile_service_scheduler()->OnFetchCompleted(TileInfoRequestStatus::kFailure); + auto backoff = GetBackoffPolicy(); + EXPECT_EQ(backoff->GetTimeUntilRelease().InMilliseconds(), + 1000 * pow(2, i)); } } @@ -185,5 +208,53 @@ TEST_F(TileServiceSchedulerTest, OnTileGroupLoadedWithOtherStatus) { } } +// OnTileManagerInitialized(NoTiles) could be called many time before the first +// fetch task started. Ensure only the first one actually schedules the task, +// other calls should not override the existing task until it is executed and +// marked finished. +TEST_F(TileServiceSchedulerTest, FirstKickoffNotOverride) { + // Verifying only schedule once also implying only the first schedule + // call works. + EXPECT_CALL(*native_scheduler(), Schedule(_)).Times(1); + auto now = clock()->Now(); + tile_service_scheduler()->OnTileManagerInitialized(TileGroupStatus::kNoTiles); + EXPECT_EQ(prefs()->GetTime(kFirstScheduleTimeKey), now); + auto two_hours_later = now + base::TimeDelta::FromHours(2); + clock()->SetNow(two_hours_later); + tile_service_scheduler()->OnTileManagerInitialized(TileGroupStatus::kNoTiles); + tile_service_scheduler()->OnTileManagerInitialized(TileGroupStatus::kNoTiles); + EXPECT_EQ(prefs()->GetTime(kFirstScheduleTimeKey), now); + + EXPECT_CALL(*native_scheduler(), Schedule(_)).Times(1); + tile_service_scheduler()->OnFetchCompleted(TileInfoRequestStatus::kSuccess); + EXPECT_EQ(prefs()->GetTime(kFirstScheduleTimeKey), base::Time()); +} + +TEST_F(TileServiceSchedulerTest, FirstRunFinishedAfterInstantFetchComplete) { + base::test::ScopedCommandLine scoped_command_line; + auto now = clock()->Now(); + EXPECT_CALL(*native_scheduler(), Schedule(_)).Times(1); + tile_service_scheduler()->OnTileManagerInitialized(TileGroupStatus::kNoTiles); + EXPECT_EQ(prefs()->GetTime(kFirstScheduleTimeKey), now); + + // Set instant-fetch flag to true after first-kickoff flow was marked and + // scheduled, expecting the mark of first flow also being reset. + scoped_command_line.GetProcessCommandLine()->AppendSwitchASCII( + query_tiles::switches::kQueryTilesInstantBackgroundTask, "true"); + EXPECT_CALL(*native_scheduler(), Schedule(_)).Times(0); + tile_service_scheduler()->OnFetchCompleted(TileInfoRequestStatus::kSuccess); + EXPECT_EQ(prefs()->GetTime(kFirstScheduleTimeKey), base::Time()); + + // Set instant-fetch flag to false after 2 hours. Chrome restarts with no + // tiles, the scheduler should start a new first kickoff flow. + scoped_command_line.GetProcessCommandLine()->RemoveSwitch( + query_tiles::switches::kQueryTilesInstantBackgroundTask); + auto two_hours_later = now + base::TimeDelta::FromHours(2); + clock()->SetNow(two_hours_later); + EXPECT_CALL(*native_scheduler(), Schedule(_)).Times(1); + tile_service_scheduler()->OnTileManagerInitialized(TileGroupStatus::kNoTiles); + EXPECT_EQ(prefs()->GetTime(kFirstScheduleTimeKey), two_hours_later); +} + } // namespace } // namespace query_tiles diff --git a/chromium/components/query_tiles/logger.h b/chromium/components/query_tiles/logger.h new file mode 100644 index 00000000000..92c3938c9cd --- /dev/null +++ b/chromium/components/query_tiles/logger.h @@ -0,0 +1,57 @@ +// Copyright 2020 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_QUERY_TILES_LOGGER_H_ +#define COMPONENTS_QUERY_TILES_LOGGER_H_ + +namespace base { +class Value; +} + +namespace query_tiles { + +// A helper class to expose internals of the query-tiles service to a logging +// component and/or debug UI. +class Logger { + public: + class Observer { + public: + virtual ~Observer() = default; + + // Called whenever the service status changes. + virtual void OnServiceStatusChanged(const base::Value& status) = 0; + + // Called when the tile group data is available. + virtual void OnTileDataAvailable(const base::Value& status) = 0; + }; + + virtual ~Logger() = default; + + virtual void AddObserver(Observer* observer) = 0; + virtual void RemoveObserver(Observer* observer) = 0; + + Logger(const Logger& other) = delete; + Logger& operator=(const Logger& other) = delete; + + // Returns the current status of query tile service. + // The serialized format will be: + // { + // fetcherStatus:[INITIAL, SUCCESS, FAIL, SUSPEND] + // groupStatus:[SUCCESS, UN_INIT, NO_TILES, DB_FAIL] + // } + virtual base::Value GetServiceStatus() = 0; + + // Returns the formatted raw data stored in database. + // The serialized format will be: + // { + // groupInfo(string) + // tileProto(string) + // } + virtual base::Value GetTileData() = 0; + + protected: + Logger() = default; +}; +} // namespace query_tiles +#endif // COMPONENTS_QUERY_TILES_LOGGER_H_ diff --git a/chromium/components/query_tiles/test/BUILD.gn b/chromium/components/query_tiles/test/BUILD.gn index 5996f31f651..0c6c9cb14ad 100644 --- a/chromium/components/query_tiles/test/BUILD.gn +++ b/chromium/components/query_tiles/test/BUILD.gn @@ -6,11 +6,19 @@ source_set("test_support") { testonly = true sources = [ + "empty_logger.cc", + "empty_logger.h", + "fake_server_response.cc", + "fake_server_response.h", "fake_tile_service.cc", "fake_tile_service.h", ] - deps = [ "//components/query_tiles:public" ] + deps = [ + "//components/query_tiles:public", + "//components/query_tiles/internal", + "//components/query_tiles/proto", + ] } source_set("test_lib") { diff --git a/chromium/components/query_tiles/tile_service.h b/chromium/components/query_tiles/tile_service.h index 5834cf4c1b7..b5d4f3540d8 100644 --- a/chromium/components/query_tiles/tile_service.h +++ b/chromium/components/query_tiles/tile_service.h @@ -12,6 +12,7 @@ #include "base/optional.h" #include "base/supports_user_data.h" #include "components/keyed_service/core/keyed_service.h" +#include "components/query_tiles/logger.h" #include "components/query_tiles/tile.h" namespace gfx { @@ -43,6 +44,9 @@ class TileService : public KeyedService, public base::SupportsUserData { // Cancel any existing scheduled task, and reset backoff. virtual void CancelTask() = 0; + // Used for debugging and testing only. Clear everything in db. + virtual void PurgeDb() = 0; + TileService() = default; ~TileService() override = default; diff --git a/chromium/components/query_tiles/tile_service_prefs.cc b/chromium/components/query_tiles/tile_service_prefs.cc index 1a97dea6bbf..97480c6f7c1 100644 --- a/chromium/components/query_tiles/tile_service_prefs.cc +++ b/chromium/components/query_tiles/tile_service_prefs.cc @@ -10,8 +10,10 @@ namespace query_tiles { constexpr char kBackoffEntryKey[] = "query_tiles.backoff_entry_key"; +constexpr char kFirstScheduleTimeKey[] = "query_tiles.first_schedule_time_key"; void RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterListPref(kBackoffEntryKey); + registry->RegisterTimePref(kFirstScheduleTimeKey, base::Time()); } } // namespace query_tiles diff --git a/chromium/components/query_tiles/tile_service_prefs.h b/chromium/components/query_tiles/tile_service_prefs.h index 391d51c9cd1..a32c0ceab46 100644 --- a/chromium/components/query_tiles/tile_service_prefs.h +++ b/chromium/components/query_tiles/tile_service_prefs.h @@ -12,6 +12,10 @@ namespace query_tiles { // Key for query tiles backoff entry stored in pref service. extern const char kBackoffEntryKey[]; +// Key to record a timestamp when the first query tiles background task is +// scheduled. +extern const char kFirstScheduleTimeKey[]; + // Register to prefs service. void RegisterPrefs(PrefRegistrySimple* registry); |