Skip to content

Commit

Permalink
[Feed] Fork RequestThrottler.
Browse files Browse the repository at this point in the history
Bug: 831648
Change-Id: I3a9525384ca17c05f38748fc3592af3f46f742db
Reviewed-on: https://chromium-review.googlesource.com/1089930
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579566}
  • Loading branch information
Sky Malice authored and Commit Bot committed Jul 31, 2018
1 parent 2de9079 commit 8ff6430
Show file tree
Hide file tree
Showing 10 changed files with 363 additions and 3 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/prefs/browser_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@
#include "components/ntp_tiles/popular_sites_impl.h"
#if BUILDFLAG(ENABLE_FEED_IN_CHROME)
#include "components/feed/core/feed_scheduler_host.h"
#include "components/feed/core/refresh_throttler.h"
#include "components/feed/core/user_classifier.h"
#endif // BUILDFLAG(ENABLE_FEED_IN_CHROME)
#else
#include "chrome/browser/gcm/gcm_product_util.h"
#include "chrome/browser/media/router/media_router_feature.h"
#include "chrome/browser/metrics/tab_stats_tracker.h"
#include "chrome/browser/search/instant_service.h"
#include "chrome/browser/signin/signin_promo.h"
Expand Down Expand Up @@ -619,6 +619,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
RecentTabsPagePrefs::RegisterProfilePrefs(registry);
#if BUILDFLAG(ENABLE_FEED_IN_CHROME)
feed::FeedSchedulerHost::RegisterProfilePrefs(registry);
feed::RefreshThrottler::RegisterProfilePrefs(registry);
feed::UserClassifier::RegisterProfilePrefs(registry);
#endif // BUILDFLAG(ENABLE_FEED_IN_CHROME)
#else
Expand Down
3 changes: 3 additions & 0 deletions components/feed/core/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ source_set("feed_core") {
"feed_storage_database.h",
"pref_names.cc",
"pref_names.h",
"refresh_throttler.cc",
"refresh_throttler.h",
"time_serialization.cc",
"time_serialization.h",
"user_classifier.cc",
Expand Down Expand Up @@ -69,6 +71,7 @@ source_set("core_unit_tests") {
"feed_networking_host_unittest.cc",
"feed_scheduler_host_unittest.cc",
"feed_storage_database_unittest.cc",
"refresh_throttler_unittest.cc",
"user_classifier_unittest.cc",
]

Expand Down
22 changes: 20 additions & 2 deletions components/feed/core/feed_scheduler_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "components/feed/core/feed_scheduler_host.h"

#include <map>
#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -159,6 +158,19 @@ FeedSchedulerHost::FeedSchedulerHost(PrefService* profile_prefs,
if (eula_accepted_notifier_) {
eula_accepted_notifier_->Init(this);
}

throttlers_.emplace(
UserClassifier::UserClass::kRareNtpUser,
std::make_unique<RefreshThrottler>(
UserClassifier::UserClass::kRareNtpUser, profile_prefs_, clock_));
throttlers_.emplace(
UserClassifier::UserClass::kActiveNtpUser,
std::make_unique<RefreshThrottler>(
UserClassifier::UserClass::kActiveNtpUser, profile_prefs_, clock_));
throttlers_.emplace(UserClassifier::UserClass::kActiveSuggestionsConsumer,
std::make_unique<RefreshThrottler>(
UserClassifier::UserClass::kActiveSuggestionsConsumer,
profile_prefs_, clock_));
}

FeedSchedulerHost::~FeedSchedulerHost() = default;
Expand Down Expand Up @@ -360,7 +372,13 @@ bool FeedSchedulerHost::ShouldRefresh(TriggerType trigger) {
return false;
}

// TODO(skym): Check with throttler.
auto throttlerIter = throttlers_.find(user_class);
if (throttlerIter == throttlers_.end() ||
!throttlerIter->second->RequestQuota()) {
DVLOG(2) << "Throttler stopped refresh from trigger "
<< static_cast<int>(trigger);
return false;
}

switch (trigger) {
case TriggerType::kNtpShown:
Expand Down
7 changes: 7 additions & 0 deletions components/feed/core/feed_scheduler_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
#include <set>

#include "base/callback.h"
#include "base/containers/flat_map.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "components/feed/core/refresh_throttler.h"
#include "components/feed/core/user_classifier.h"
#include "components/web_resource/eula_accepted_notifier.h"

Expand Down Expand Up @@ -181,6 +183,11 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer {
bool time_until_first_shown_trigger_reported_ = false;
bool time_until_first_foregrounded_trigger_reported_ = false;

// In the case the user transitions between user classes, hold onto a
// throttler for any situation.
base::flat_map<UserClassifier::UserClass, std::unique_ptr<RefreshThrottler>>
throttlers_;

DISALLOW_COPY_AND_ASSIGN(FeedSchedulerHost);
};

Expand Down
17 changes: 17 additions & 0 deletions components/feed/core/feed_scheduler_host_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "components/feed/core/feed_scheduler_host.h"

#include <algorithm>
#include <string>
#include <vector>

Expand All @@ -12,6 +13,7 @@
#include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_clock.h"
#include "components/feed/core/pref_names.h"
#include "components/feed/core/refresh_throttler.h"
#include "components/feed/core/time_serialization.h"
#include "components/feed/core/user_classifier.h"
#include "components/feed/feed_feature_list.h"
Expand Down Expand Up @@ -44,6 +46,7 @@ class FeedSchedulerHostTest : public ::testing::Test {
protected:
FeedSchedulerHostTest() : weak_factory_(this) {
FeedSchedulerHost::RegisterProfilePrefs(profile_prefs_.registry());
RefreshThrottler::RegisterProfilePrefs(profile_prefs_.registry());
UserClassifier::RegisterProfilePrefs(profile_prefs_.registry());
local_state()->registry()->RegisterBooleanPref(::prefs::kEulaAccepted,
true);
Expand Down Expand Up @@ -874,4 +877,18 @@ TEST_F(FeedSchedulerHostTest, TimeUntilFirstMetrics) {
EXPECT_EQ(2, histogram_tester.GetBucketCount(forgroundedHistogram, 0));
}

TEST_F(FeedSchedulerHostTest, RefreshThrottler) {
variations::testing::VariationParamsManager variation_params(
kInterestFeedContentSuggestions.name,
{{"quota_SuggestionFetcherActiveNTPUser", "3"}},
{kInterestFeedContentSuggestions.name});
NewScheduler();

for (int i = 0; i < 5; i++) {
scheduler()->OnForegrounded();
ResetRefreshState(base::Time());
EXPECT_EQ(std::min(i + 1, 3), refresh_call_count());
}
}

} // namespace feed
3 changes: 3 additions & 0 deletions components/feed/core/pref_names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ const char kBackgroundRefreshPeriod[] = "feed.background_refresh_period";

const char kLastFetchAttemptTime[] = "feed.last_fetch_attempt";

const char kThrottlerRequestCount[] = "feed.refresh_throttler.count";
const char kThrottlerRequestsDay[] = "feed.refresh_throttler.day";

const char kUserClassifierAverageNTPOpenedPerHour[] =
"feed.user_classifier.average_ntp_opened_per_hour";
const char kUserClassifierAverageSuggestionsUsedPerHour[] =
Expand Down
6 changes: 6 additions & 0 deletions components/feed/core/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ extern const char kBackgroundRefreshPeriod[];
// The pref name for the last time when a background fetch was attempted.
extern const char kLastFetchAttemptTime[];

// The pref name for today's count of RefreshThrottler requests, so far.
extern const char kThrottlerRequestCount[];
// The pref name for the current day for the counter of RefreshThrottler's
// requests.
extern const char kThrottlerRequestsDay[];

// The pref name for the discounted average number of browsing sessions per hour
// that involve opening a new NTP.
extern const char kUserClassifierAverageNTPOpenedPerHour[];
Expand Down
146 changes: 146 additions & 0 deletions components/feed/core/refresh_throttler.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "components/feed/core/refresh_throttler.h"

#include <limits>
#include <set>
#include <utility>
#include <vector>

#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram.h"
#include "base/metrics/histogram_base.h"
#include "base/strings/stringprintf.h"
#include "base/time/clock.h"
#include "components/feed/core/pref_names.h"
#include "components/feed/feed_feature_list.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"

namespace feed {

namespace {

// Values correspond to ntp_snippets::RequestStatus and histograms.xml
enum class RequestStatus {
kObsolete1 = 0,
kQuotaGranted = 1,
kQuotaExceeded = 2,
kObsolete2 = 3,
kStatusCount = 4
};

// When adding a new type here, extend also the "RequestThrottlerTypes"
// <histogram_suffixes> in histograms.xml with the |name| string. First value in
// the pair is the name, second is the default requests per day.
std::pair<std::string, int> GetThrottlerParams(
UserClassifier::UserClass user_class) {
switch (user_class) {
case UserClassifier::UserClass::kRareNtpUser:
return {"SuggestionFetcherRareNTPUser", 5};
case UserClassifier::UserClass::kActiveNtpUser:
return {"SuggestionFetcherActiveNTPUser", 20};
case UserClassifier::UserClass::kActiveSuggestionsConsumer:
return {"SuggestionFetcherActiveSuggestionsConsumer", 20};
}
}

} // namespace

RefreshThrottler::RefreshThrottler(UserClassifier::UserClass user_class,
PrefService* pref_service,
base::Clock* clock)
: pref_service_(pref_service), clock_(clock) {
DCHECK(pref_service);
DCHECK(clock);

std::pair<std::string, int> throttler_params = GetThrottlerParams(user_class);
name_ = throttler_params.first;
max_requests_per_day_ = base::GetFieldTrialParamByFeatureAsInt(
kInterestFeedContentSuggestions,
base::StringPrintf("quota_%s", name_.c_str()), throttler_params.second);

// Since the histogram names are dynamic, we cannot use the standard macros
// and we need to lookup the histograms, instead.
int status_count = static_cast<int>(RequestStatus::kStatusCount);
// Corresponds to UMA_HISTOGRAM_ENUMERATION(name, sample, |status_count|).
histogram_request_status_ = base::LinearHistogram::FactoryGet(
base::StringPrintf("NewTabPage.RequestThrottler.RequestStatus_%s",
name_.c_str()),
1, status_count, status_count + 1,
base::HistogramBase::kUmaTargetedHistogramFlag);
// Corresponds to UMA_HISTOGRAM_COUNTS_100(name, sample).
histogram_per_day_ = base::Histogram::FactoryGet(
base::StringPrintf("NewTabPage.RequestThrottler.PerDay_%s",
name_.c_str()),
1, 100, 50, base::HistogramBase::kUmaTargetedHistogramFlag);
}

// static
void RefreshThrottler::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterIntegerPref(prefs::kThrottlerRequestCount, 0);
registry->RegisterIntegerPref(prefs::kThrottlerRequestsDay, 0);
}

bool RefreshThrottler::RequestQuota() {
ResetCounterIfDayChanged();

// Increment |new_count| in a overflow safe fashion.
int new_count = GetCount();
if (new_count < std::numeric_limits<int>::max()) {
new_count++;
}
SetCount(new_count);
bool available = (new_count <= GetQuota());

histogram_request_status_->Add(
static_cast<int>(available ? RequestStatus::kQuotaGranted
: RequestStatus::kQuotaExceeded));

return available;
}

void RefreshThrottler::ResetCounterIfDayChanged() {
// Grant new quota on local midnight to spread out when clients that start
// making un-throttled requests to server.
int now_day = clock_->Now().LocalMidnight().since_origin().InDays();

if (!HasDay()) {
// The counter is used for the first time in this profile.
SetDay(now_day);
} else if (now_day != GetDay()) {
// Day has changed - report the number of requests from the previous day.
histogram_per_day_->Add(GetCount());
// Reset the counters.
SetCount(0);
SetDay(now_day);
}
}

int RefreshThrottler::GetQuota() const {
return max_requests_per_day_;
}

int RefreshThrottler::GetCount() const {
return pref_service_->GetInteger(prefs::kThrottlerRequestCount);
}

void RefreshThrottler::SetCount(int count) {
pref_service_->SetInteger(prefs::kThrottlerRequestCount, count);
}

int RefreshThrottler::GetDay() const {
return pref_service_->GetInteger(prefs::kThrottlerRequestsDay);
}

void RefreshThrottler::SetDay(int day) {
pref_service_->SetInteger(prefs::kThrottlerRequestsDay, day);
}

bool RefreshThrottler::HasDay() const {
return pref_service_->HasPrefPath(prefs::kThrottlerRequestsDay);
}

} // namespace feed
78 changes: 78 additions & 0 deletions components/feed/core/refresh_throttler.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_FEED_CORE_REFRESH_THROTTLER_H_
#define COMPONENTS_FEED_CORE_REFRESH_THROTTLER_H_

#include <string>

#include "base/macros.h"
#include "components/feed/core/user_classifier.h"

class PrefRegistrySimple;
class PrefService;

namespace base {
class Clock;
class HistogramBase;
} // namespace base

namespace feed {

// Counts requests to perform refreshes, compares them to a daily quota, and
// reports them to UMA. In the application code, create one local instance for
// each given throttler name, identified by the UserClass. Reports to the same
// histograms that previous NTP implementation used:
// - "NewTabPage.RequestThrottler.RequestStatus_|name|" - status of each
// request;
// - "NewTabPage.RequestThrottler.PerDay_|name|" - the daily count of requests.
class RefreshThrottler {
public:
RefreshThrottler(UserClassifier::UserClass user_class,
PrefService* pref_service,
base::Clock* clock);

// Registers profile prefs, called from browser_prefs.cc.
static void RegisterProfilePrefs(PrefRegistrySimple* registry);

// Returns whether quota is available for another request, persists the usage
// of said quota, and reports this information to UMA.
bool RequestQuota();

private:
// Also emits the PerDay histogram if the day changed.
void ResetCounterIfDayChanged();

int GetQuota() const;
int GetCount() const;
void SetCount(int count);
int GetDay() const;
void SetDay(int day);
bool HasDay() const;

// Provides durable storage.
PrefService* pref_service_;

// Used to access current time, injected for testing.
base::Clock* clock_;

// The name used by this throttler, based off UserClass, which will be used as
// a suffix when constructing histogram or finch param names.
std::string name_;

// The total requests allowed before RequestQuota() starts returning false,
// reset every time |clock_| changes days. Read from a variation param during
// initialization.
int max_requests_per_day_;

// The histograms for reporting the requests of the given |type_|.
base::HistogramBase* histogram_request_status_;
base::HistogramBase* histogram_per_day_;

DISALLOW_COPY_AND_ASSIGN(RefreshThrottler);
};

} // namespace feed

#endif // COMPONENTS_FEED_CORE_REFRESH_THROTTLER_H_
Loading

0 comments on commit 8ff6430

Please sign in to comment.