diff options
Diffstat (limited to 'chromium/components/previews/core')
14 files changed, 579 insertions, 435 deletions
diff --git a/chromium/components/previews/core/BUILD.gn b/chromium/components/previews/core/BUILD.gn index e2f014cac8f..61052354174 100644 --- a/chromium/components/previews/core/BUILD.gn +++ b/chromium/components/previews/core/BUILD.gn @@ -11,6 +11,8 @@ static_library("core") { "previews_decider.h", "previews_experiments.cc", "previews_experiments.h", + "previews_features.cc", + "previews_features.h", "previews_io_data.cc", "previews_io_data.h", "previews_opt_out_store.h", diff --git a/chromium/components/previews/core/previews_black_list.cc b/chromium/components/previews/core/previews_black_list.cc index ea5de7d822f..c4f0570d188 100644 --- a/chromium/components/previews/core/previews_black_list.cc +++ b/chromium/components/previews/core/previews_black_list.cc @@ -6,8 +6,9 @@ #include "base/bind.h" #include "base/memory/ptr_util.h" -#include "base/metrics/histogram_macros.h" +#include "base/metrics/histogram.h" #include "base/optional.h" +#include "base/strings/stringprintf.h" #include "base/time/clock.h" #include "components/previews/core/previews_black_list_item.h" #include "components/previews/core/previews_experiments.h" @@ -74,13 +75,12 @@ void PreviewsBlackList::AddPreviewNavigation(const GURL& url, PreviewsType type) { DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(url.has_host()); - switch (type) { - case PreviewsType::OFFLINE: - UMA_HISTOGRAM_BOOLEAN("Previews.OptOut.UserOptedOut.Offline", opt_out); - break; - default: - NOTREACHED(); - } + + base::BooleanHistogram::FactoryGet( + base::StringPrintf("Previews.OptOut.UserOptedOut.%s", + GetStringNameForType(type).c_str()), + base::HistogramBase::kUmaTargetedHistogramFlag) + ->Add(opt_out); if (opt_out) { last_opt_out_time_ = clock_->Now(); } @@ -158,6 +158,14 @@ void PreviewsBlackList::ClearBlackListSync(base::Time begin_time, DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(loaded_); DCHECK_LE(begin_time, end_time); + + // Clear last_opt_out_time_ if the period being cleared is larger than the + // short black list timeout and the last time the user opted out was before + // |end_time|. + if (end_time - begin_time > params::SingleOptOutDuration() && + last_opt_out_time_ && last_opt_out_time_.value() < end_time) { + last_opt_out_time_.reset(); + } black_list_item_map_.reset(); host_indifferent_black_list_item_.reset(); loaded_ = false; diff --git a/chromium/components/previews/core/previews_black_list.h b/chromium/components/previews/core/previews_black_list.h index 6abac75b2e7..f45aa7770d0 100644 --- a/chromium/components/previews/core/previews_black_list.h +++ b/chromium/components/previews/core/previews_black_list.h @@ -49,8 +49,8 @@ enum class PreviewsEligibilityReason { NETWORK_QUALITY_UNAVAILABLE = 6, // The network was fast enough to not warrant previews. NETWORK_NOT_SLOW = 7, - // If the page was reloaded, the user should not be shown an offline preview. - RELOAD_DISALLOWED_FOR_OFFLINE = 8, + // If the page was reloaded, the user should not be shown a stale preview. + RELOAD_DISALLOWED = 8, LAST = 9, }; diff --git a/chromium/components/previews/core/previews_black_list_unittest.cc b/chromium/components/previews/core/previews_black_list_unittest.cc index 2c8a1fc3b80..6c809c24191 100644 --- a/chromium/components/previews/core/previews_black_list_unittest.cc +++ b/chromium/components/previews/core/previews_black_list_unittest.cc @@ -4,6 +4,7 @@ #include "components/previews/core/previews_black_list.h" +#include <map> #include <memory> #include <string> @@ -25,12 +26,6 @@ #include "testing/gtest/include/gtest/gtest.h" #include "url/gurl.h" -namespace { - -using PreviewsBlackListTest = testing::Test; - -} // namespace - namespace previews { namespace { @@ -75,292 +70,328 @@ class TestPreviewsOptOutStore : public PreviewsOptOutStore { int clear_blacklist_count_; }; -} // namespace +class PreviewsBlackListTest : public testing::Test { + public: + PreviewsBlackListTest() : field_trial_list_(nullptr) {} + ~PreviewsBlackListTest() override {} + + void TearDown() override { variations::testing::ClearAllVariationParams(); } + + void StartTest(bool null_opt_out) { + if (params_.size() > 0) { + ASSERT_TRUE(variations::AssociateVariationParams("ClientSidePreviews", + "Enabled", params_)); + ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", + "Enabled")); + params_.clear(); + } + std::unique_ptr<base::SimpleTestClock> test_clock = + base::MakeUnique<base::SimpleTestClock>(); + test_clock_ = test_clock.get(); + std::unique_ptr<TestPreviewsOptOutStore> opt_out_store = + null_opt_out ? nullptr : base::MakeUnique<TestPreviewsOptOutStore>(); + opt_out_store_ = opt_out_store.get(); + black_list_ = base::MakeUnique<PreviewsBlackList>(std::move(opt_out_store), + std::move(test_clock)); + start_ = test_clock_->Now(); + } + + void SetHostHistoryParam(size_t host_history) { + params_["per_host_max_stored_history_length"] = + base::SizeTToString(host_history); + } + + void SetHostIndifferentHistoryParam(size_t host_indifferent_history) { + params_["host_indifferent_max_stored_history_length"] = + base::SizeTToString(host_indifferent_history); + } + + void SetHostThresholdParam(int per_host_threshold) { + params_["per_host_opt_out_threshold"] = + base::IntToString(per_host_threshold); + } + + void SetHostIndifferentThresholdParam(int host_indifferent_threshold) { + params_["host_indifferent_opt_out_threshold"] = + base::IntToString(host_indifferent_threshold); + } + + void SetHostDurationParam(int duration_in_days) { + params_["per_host_black_list_duration_in_days"] = + base::IntToString(duration_in_days); + } + + void SetSingleOptOutDurationParam(int single_opt_out_duration) { + params_["single_opt_out_duration_in_seconds"] = + base::IntToString(single_opt_out_duration); + } + + void SetMaxHostInBlackListParam(size_t max_hosts_in_blacklist) { + params_["max_hosts_in_blacklist"] = + base::IntToString(max_hosts_in_blacklist); + } + + // Adds an opt out and either clears the black list for a time either longer + // or shorter than the single opt out duration parameter depending on + // |short_time|. + void RunClearingBlackListTest(const GURL& url, bool short_time) { + const size_t host_indifferent_history = 1; + const int single_opt_out_duration = 5; + SetHostDurationParam(365); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetSingleOptOutDurationParam(single_opt_out_duration); + + StartTest(false /* null_opt_out */); + if (!short_time) + test_clock_->Advance( + base::TimeDelta::FromSeconds(single_opt_out_duration)); + + black_list_->AddPreviewNavigation(url, true /* opt_out */, + PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->ClearBlackList(start_, test_clock_->Now()); + base::RunLoop().RunUntilIdle(); + } + + protected: + base::MessageLoop loop_; + + // Unowned raw pointers tied to the lifetime of |black_list_|. + base::SimpleTestClock* test_clock_; + TestPreviewsOptOutStore* opt_out_store_; + base::Time start_; + std::map<std::string, std::string> params_; + base::FieldTrialList field_trial_list_; + + std::unique_ptr<PreviewsBlackList> black_list_; + + private: + DISALLOW_COPY_AND_ASSIGN(PreviewsBlackListTest); +}; TEST_F(PreviewsBlackListTest, PerHostBlackListNoStore) { - // Tests the black list behavior when a null OptOutSture is passed in. + // Tests the black list behavior when a null OptOutStore is passed in. const GURL url_a("http://www.url_a.com"); const GURL url_b("http://www.url_b.com"); - const size_t per_host_history = 4; - const int per_host_threshold = 2; + // Host indifferent blacklisting should have no effect with the following // params. const size_t host_indifferent_history = 1; - const int host_indifferent_threshold = host_indifferent_history + 1; - const int duration_in_days = 365; + SetHostHistoryParam(4); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostThresholdParam(2); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetHostDurationParam(365); // Disable single opt out by setting duration to 0. - const int single_opt_out_duration = 0; - base::FieldTrialList field_trial_list(nullptr); - std::map<std::string, std::string> params; - params["per_host_max_stored_history_length"] = - base::SizeTToString(per_host_history); - params["host_indifferent_max_stored_history_length"] = - base::SizeTToString(host_indifferent_history); - params["per_host_opt_out_threshold"] = base::IntToString(per_host_threshold); - params["host_indifferent_opt_out_threshold"] = - base::IntToString(host_indifferent_threshold); - params["per_host_black_list_duration_in_days"] = - base::IntToString(duration_in_days); - params["single_opt_out_duration_in_seconds"] = - base::IntToString(single_opt_out_duration); - ASSERT_TRUE( - variations::AssociateVariationParams("ClientSidePreviews", "Enabled", - params) && - base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled")); - - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); - base::Time start = test_clock->Now(); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - - std::unique_ptr<PreviewsBlackList> black_list( - new PreviewsBlackList(nullptr, base::WrapUnique(test_clock))); + SetSingleOptOutDurationParam(0); + + StartTest(true /* null_opt_out */); + + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->ClearBlackList(start, test_clock->Now()); + black_list_->ClearBlackList(start_, test_clock_->Now()); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - - variations::testing::ClearAllVariationParams(); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); } TEST_F(PreviewsBlackListTest, PerHostBlackListWithStore) { - // Tests the black list behavior when a non-null OptOutSture is passed in. + // Tests the black list behavior when a non-null OptOutStore is passed in. const GURL url_a1("http://www.url_a.com/a1"); const GURL url_a2("http://www.url_a.com/a2"); const GURL url_b("http://www.url_b.com"); - const size_t per_host_history = 4; - const int per_host_threshold = 2; + // Host indifferent blacklisting should have no effect with the following // params. const size_t host_indifferent_history = 1; - const int host_indifferent_threshold = host_indifferent_history + 1; - const int duration_in_days = 365; + SetHostHistoryParam(4); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostThresholdParam(2); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetHostDurationParam(365); // Disable single opt out by setting duration to 0. - const int single_opt_out_duration = 0; - base::FieldTrialList field_trial_list(nullptr); - std::map<std::string, std::string> params; - params["per_host_max_stored_history_length"] = - base::SizeTToString(per_host_history); - params["host_indifferent_max_stored_history_length"] = - base::SizeTToString(host_indifferent_history); - params["per_host_opt_out_threshold"] = base::IntToString(per_host_threshold); - params["host_indifferent_opt_out_threshold"] = - base::IntToString(host_indifferent_threshold); - params["per_host_black_list_duration_in_days"] = - base::IntToString(duration_in_days); - params["single_opt_out_duration_in_seconds"] = - base::IntToString(single_opt_out_duration); - ASSERT_TRUE( - variations::AssociateVariationParams("ClientSidePreviews", "Enabled", - params) && - base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled")); - - base::MessageLoop loop; - - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); - base::Time start = test_clock->Now(); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - - TestPreviewsOptOutStore* opt_out_store = new TestPreviewsOptOutStore(); - - std::unique_ptr<PreviewsBlackList> black_list(new PreviewsBlackList( - base::WrapUnique(opt_out_store), base::WrapUnique(test_clock))); + SetSingleOptOutDurationParam(0); + + StartTest(false /* null_opt_out */); + + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); base::RunLoop().RunUntilIdle(); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url_a1, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_a1, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_a1, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_a1, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - EXPECT_EQ(0, opt_out_store->clear_blacklist_count()); - black_list->ClearBlackList(start, base::Time::Now()); - EXPECT_EQ(1, opt_out_store->clear_blacklist_count()); + EXPECT_EQ(0, opt_out_store_->clear_blacklist_count()); + black_list_->ClearBlackList(start_, base::Time::Now()); + EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); base::RunLoop().RunUntilIdle(); - EXPECT_EQ(1, opt_out_store->clear_blacklist_count()); + EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); - - variations::testing::ClearAllVariationParams(); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); } TEST_F(PreviewsBlackListTest, HostIndifferentBlackList) { - // Tests the black list behavior when a null OptOutSture is passed in. + // Tests the black list behavior when a null OptOutStore is passed in. const GURL urls[] = { GURL("http://www.url_0.com"), GURL("http://www.url_1.com"), GURL("http://www.url_2.com"), GURL("http://www.url_3.com"), }; + // Per host blacklisting should have no effect with the following params. const size_t per_host_history = 1; - const int per_host_threshold = per_host_history + 1; const size_t host_indifferent_history = 4; - const int host_indifferent_threshold = host_indifferent_history; - const int duration_in_days = 365; + const size_t host_indifferent_threshold = host_indifferent_history; + SetHostHistoryParam(per_host_history); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostThresholdParam(per_host_history + 1); + SetHostIndifferentThresholdParam(host_indifferent_threshold); + SetHostDurationParam(365); // Disable single opt out by setting duration to 0. - const int single_opt_out_duration = 0; - base::FieldTrialList field_trial_list(nullptr); - std::map<std::string, std::string> params; - params["per_host_max_stored_history_length"] = - base::SizeTToString(per_host_history); - params["host_indifferent_max_stored_history_length"] = - base::SizeTToString(host_indifferent_history); - params["per_host_opt_out_threshold"] = base::IntToString(per_host_threshold); - params["host_indifferent_opt_out_threshold"] = - base::IntToString(host_indifferent_threshold); - params["per_host_black_list_duration_in_days"] = - base::IntToString(duration_in_days); - params["single_opt_out_duration_in_seconds"] = - base::IntToString(single_opt_out_duration); - ASSERT_TRUE( - variations::AssociateVariationParams("ClientSidePreviews", "Enabled", - params) && - base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled")); - - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - - std::unique_ptr<PreviewsBlackList> black_list( - new PreviewsBlackList(nullptr, base::WrapUnique(test_clock))); + SetSingleOptOutDurationParam(0); + + StartTest(true /* null_opt_out */); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); for (size_t i = 0; i < host_indifferent_threshold; i++) { - black_list->AddPreviewNavigation(urls[i], true, PreviewsType::OFFLINE); + black_list_->AddPreviewNavigation(urls[i], true, PreviewsType::OFFLINE); EXPECT_EQ(i != 3 ? PreviewsEligibilityReason::ALLOWED : PreviewsEligibilityReason::USER_BLACKLISTED, - black_list->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); } EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::USER_BLACKLISTED, - black_list->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(urls[3], false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(urls[3], false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); // New non-opt-out entry will cause these to be allowed now. EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[0], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[1], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(urls[2], PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); - - variations::testing::ClearAllVariationParams(); + black_list_->IsLoadedAndAllowed(urls[3], PreviewsType::OFFLINE)); } TEST_F(PreviewsBlackListTest, QueueBehavior) { @@ -369,76 +400,54 @@ TEST_F(PreviewsBlackListTest, QueueBehavior) { // queued. const GURL url("http://www.url.com"); const GURL url2("http://www.url2.com"); + // Host indifferent blacklisting should have no effect with the following // params. const size_t host_indifferent_history = 1; - const int host_indifferent_threshold = host_indifferent_history + 1; - const int duration_in_days = 365; + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetHostDurationParam(365); // Disable single opt out by setting duration to 0. - const int single_opt_out_duration = 0; - base::FieldTrialList field_trial_list(nullptr); - std::map<std::string, std::string> params; - params["per_host_black_list_duration_in_days"] = - base::IntToString(duration_in_days); - params["host_indifferent_max_stored_history_length"] = - base::SizeTToString(host_indifferent_history); - params["host_indifferent_opt_out_threshold"] = - base::IntToString(host_indifferent_threshold); - params["single_opt_out_duration_in_seconds"] = - base::IntToString(single_opt_out_duration); - ASSERT_TRUE( - variations::AssociateVariationParams("ClientSidePreviews", "Enabled", - params) && - base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled")); - - base::MessageLoop loop; + SetSingleOptOutDurationParam(0); std::vector<bool> test_opt_out{true, false}; for (auto opt_out : test_opt_out) { - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); - base::Time start = test_clock->Now(); - - TestPreviewsOptOutStore* opt_out_store = new TestPreviewsOptOutStore(); - - std::unique_ptr<PreviewsBlackList> black_list(new PreviewsBlackList( - base::WrapUnique(opt_out_store), base::WrapUnique(test_clock))); + StartTest(false /* null_opt_out */); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); EXPECT_EQ(PreviewsEligibilityReason::BLACKLIST_DATA_NOT_LOADED, - black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); base::RunLoop().RunUntilIdle(); EXPECT_EQ(opt_out ? PreviewsEligibilityReason::HOST_BLACKLISTED : PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); - black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - EXPECT_EQ(0, opt_out_store->clear_blacklist_count()); - black_list->ClearBlackList( - start, test_clock->Now() + base::TimeDelta::FromSeconds(1)); - EXPECT_EQ(1, opt_out_store->clear_blacklist_count()); - black_list->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + EXPECT_EQ(0, opt_out_store_->clear_blacklist_count()); + black_list_->ClearBlackList( + start_, test_clock_->Now() + base::TimeDelta::FromSeconds(1)); + EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); + black_list_->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); base::RunLoop().RunUntilIdle(); - EXPECT_EQ(1, opt_out_store->clear_blacklist_count()); + EXPECT_EQ(1, opt_out_store_->clear_blacklist_count()); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); EXPECT_EQ(opt_out ? PreviewsEligibilityReason::HOST_BLACKLISTED : PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url2, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url2, PreviewsType::OFFLINE)); } - - variations::testing::ClearAllVariationParams(); } TEST_F(PreviewsBlackListTest, MaxHosts) { @@ -449,69 +458,46 @@ TEST_F(PreviewsBlackListTest, MaxHosts) { const GURL url_c("http://www.url_c.com"); const GURL url_d("http://www.url_d.com"); const GURL url_e("http://www.url_e.com"); - const size_t stored_history_length = 1; - const int opt_out_threshold = 1; - const int black_list_duration_in_days = 365; - // Disable single opt out by setting duration to 0. - const int single_opt_out_duration = 0; - const size_t max_hosts_in_blacklist = 2; + // Host indifferent blacklisting should have no effect with the following // params. const size_t host_indifferent_history = 1; - const int host_indifferent_threshold = host_indifferent_history + 1; - base::FieldTrialList field_trial_list(nullptr); - std::map<std::string, std::string> params; - params["per_host_stored_history_length"] = - base::SizeTToString(stored_history_length); - params["per_host_opt_out_threshold"] = base::IntToString(opt_out_threshold); - params["per_host_black_list_duration_in_days"] = - base::IntToString(black_list_duration_in_days); - params["max_hosts_in_blacklist"] = - base::SizeTToString(max_hosts_in_blacklist); - params["host_indifferent_max_stored_history_length"] = - base::SizeTToString(host_indifferent_history); - params["host_indifferent_opt_out_threshold"] = - base::IntToString(host_indifferent_threshold); - params["single_opt_out_duration_in_seconds"] = - base::IntToString(single_opt_out_duration); - ASSERT_TRUE( - variations::AssociateVariationParams("ClientSidePreviews", "Enabled", - params) && - base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled")); - - base::MessageLoop loop; - - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); - - std::unique_ptr<PreviewsBlackList> black_list( - new PreviewsBlackList(nullptr, base::WrapUnique(test_clock))); - - black_list->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_c, false, PreviewsType::OFFLINE); + const size_t stored_history_length = 1; + SetHostHistoryParam(stored_history_length); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetMaxHostInBlackListParam(2); + SetHostThresholdParam(stored_history_length); + SetHostDurationParam(365); + // Disable single opt out by setting duration to 0. + SetSingleOptOutDurationParam(0); + + StartTest(true /* null_opt_out */); + + black_list_->AddPreviewNavigation(url_a, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_b, false, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_c, false, PreviewsType::OFFLINE); // url_a should stay in the map, since it has an opt out time. EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_d, true, PreviewsType::OFFLINE); - test_clock->Advance(base::TimeDelta::FromSeconds(1)); - black_list->AddPreviewNavigation(url_e, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_d, true, PreviewsType::OFFLINE); + test_clock_->Advance(base::TimeDelta::FromSeconds(1)); + black_list_->AddPreviewNavigation(url_e, true, PreviewsType::OFFLINE); // url_d and url_e should remain in the map, but url_a should be evicted. EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_d, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_d, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::HOST_BLACKLISTED, - black_list->IsLoadedAndAllowed(url_e, PreviewsType::OFFLINE)); - - variations::testing::ClearAllVariationParams(); + black_list_->IsLoadedAndAllowed(url_e, PreviewsType::OFFLINE)); } TEST_F(PreviewsBlackListTest, SingleOptOut) { @@ -520,87 +506,90 @@ TEST_F(PreviewsBlackListTest, SingleOptOut) { const GURL url_a("http://www.url_a.com"); const GURL url_b("http://www.url_b.com"); const GURL url_c("http://www.url_c.com"); - const size_t stored_history_length = 1; - const int opt_out_threshold = 2; - const int black_list_duration_in_days = 365; - const int single_opt_out_duration = 5; - const size_t max_hosts_in_blacklist = 10; + // Host indifferent blacklisting should have no effect with the following // params. const size_t host_indifferent_history = 1; - const int host_indifferent_threshold = host_indifferent_history + 1; - base::FieldTrialList field_trial_list(nullptr); - std::map<std::string, std::string> params; - params["per_host_stored_history_length"] = - base::SizeTToString(stored_history_length); - params["per_host_opt_out_threshold"] = base::IntToString(opt_out_threshold); - params["per_host_black_list_duration_in_days"] = - base::IntToString(black_list_duration_in_days); - params["max_hosts_in_blacklist"] = - base::SizeTToString(max_hosts_in_blacklist); - params["host_indifferent_max_stored_history_length"] = - base::SizeTToString(host_indifferent_history); - params["host_indifferent_opt_out_threshold"] = - base::IntToString(host_indifferent_threshold); - params["single_opt_out_duration_in_seconds"] = - base::IntToString(single_opt_out_duration); - ASSERT_TRUE( - variations::AssociateVariationParams("ClientSidePreviews", "Enabled", - params) && - base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled")); - - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); - - std::unique_ptr<PreviewsBlackList> black_list( - new PreviewsBlackList(nullptr, base::WrapUnique(test_clock))); - - black_list->AddPreviewNavigation(url_a, false, PreviewsType::OFFLINE); + const int single_opt_out_duration = 5; + SetHostHistoryParam(1); + SetHostIndifferentHistoryParam(2); + SetHostDurationParam(365); + SetMaxHostInBlackListParam(10); + SetHostIndifferentHistoryParam(host_indifferent_history); + SetHostIndifferentThresholdParam(host_indifferent_history + 1); + SetSingleOptOutDurationParam(single_opt_out_duration); + + StartTest(true /* null_opt_out */); + + black_list_->AddPreviewNavigation(url_a, false, PreviewsType::OFFLINE); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); - test_clock->Advance( + test_clock_->Advance( base::TimeDelta::FromSeconds(single_opt_out_duration + 1)); - black_list->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); + black_list_->AddPreviewNavigation(url_b, true, PreviewsType::OFFLINE); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); - test_clock->Advance( + test_clock_->Advance( base::TimeDelta::FromSeconds(single_opt_out_duration - 1)); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, - black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); - test_clock->Advance( + test_clock_->Advance( base::TimeDelta::FromSeconds(single_opt_out_duration + 1)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); + black_list_->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE)); EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, - black_list->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); - - variations::testing::ClearAllVariationParams(); + black_list_->IsLoadedAndAllowed(url_c, PreviewsType::OFFLINE)); } TEST_F(PreviewsBlackListTest, AddPreviewUMA) { base::HistogramTester histogram_tester; - base::SimpleTestClock* test_clock = new base::SimpleTestClock(); const GURL url("http://www.url.com"); - std::unique_ptr<PreviewsBlackList> black_list( - new PreviewsBlackList(nullptr, base::WrapUnique(test_clock))); - black_list->AddPreviewNavigation(url, false, PreviewsType::OFFLINE); + StartTest(false /* null_opt_out */); + + black_list_->AddPreviewNavigation(url, false, PreviewsType::OFFLINE); histogram_tester.ExpectUniqueSample("Previews.OptOut.UserOptedOut.Offline", 0, 1); - black_list->AddPreviewNavigation(url, true, PreviewsType::OFFLINE); + black_list_->AddPreviewNavigation(url, true, PreviewsType::OFFLINE); histogram_tester.ExpectBucketCount("Previews.OptOut.UserOptedOut.Offline", 1, 1); } +TEST_F(PreviewsBlackListTest, ClearShortTime) { + // Tests that clearing the black list for a short amount of time (relative to + // "SetSingleOptOutDurationParam") does not reset the blacklist's recent + // opt out rule. + + const GURL url("http://www.url.com"); + RunClearingBlackListTest(url, true /* short_time */); + EXPECT_EQ(PreviewsEligibilityReason::USER_RECENTLY_OPTED_OUT, + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); +} + +TEST_F(PreviewsBlackListTest, ClearingBlackListClearsRecentNavigation) { + // Tests that clearing the black list for a long amount of time (relative to + // "single_opt_out_duration_in_seconds") resets the blacklist's recent opt out + // rule. + + const GURL url("http://www.url.com"); + RunClearingBlackListTest(url, false /* short_time */); + + EXPECT_EQ(PreviewsEligibilityReason::ALLOWED, + black_list_->IsLoadedAndAllowed(url, PreviewsType::OFFLINE)); +} + +} // namespace + } // namespace previews diff --git a/chromium/components/previews/core/previews_decider.h b/chromium/components/previews/core/previews_decider.h index f293c87c1ad..2c518fe3d94 100644 --- a/chromium/components/previews/core/previews_decider.h +++ b/chromium/components/previews/core/previews_decider.h @@ -7,6 +7,8 @@ #include "components/previews/core/previews_experiments.h" +#include "net/nqe/effective_connection_type.h" + namespace net { class URLRequest; } @@ -15,7 +17,19 @@ namespace previews { class PreviewsDecider { public: - // Whether |request| is allowed to show a preview of |type|. + // Whether |request| is allowed to show a preview of |type|. If the current + // ECT is strictly faster than |effective_connection_type_threshold|, the + // preview will be disallowed; preview types that check network quality before + // calling ShouldAllowPreviewAtECT should pass in + // EFFECTIVE_CONNECTION_TYPE_4G. + virtual bool ShouldAllowPreviewAtECT( + const net::URLRequest& request, + PreviewsType type, + net::EffectiveConnectionType effective_connection_type_threshold) + const = 0; + + // Same as ShouldAllowPreviewAtECT, but uses the previews default + // EffectiveConnectionType. virtual bool ShouldAllowPreview(const net::URLRequest& request, PreviewsType type) const = 0; diff --git a/chromium/components/previews/core/previews_experiments.cc b/chromium/components/previews/core/previews_experiments.cc index e8d738aa60a..ccab155619a 100644 --- a/chromium/components/previews/core/previews_experiments.cc +++ b/chromium/components/previews/core/previews_experiments.cc @@ -4,13 +4,13 @@ #include "components/previews/core/previews_experiments.h" -#include <string> - +#include "base/feature_list.h" #include "base/logging.h" #include "base/metrics/field_trial.h" #include "base/metrics/field_trial_params.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" +#include "components/previews/core/previews_features.h" namespace previews { @@ -75,6 +75,15 @@ net::EffectiveConnectionType GetParamValueAsECT( return value; } +bool IsIncludedInClientSidePreviewsExperimentsFieldTrial() { + // By convention, an experiment in the client-side previews study enables use + // of at least one client-side previews optimization if its name begins with + // "Enabled." + return base::StartsWith( + base::FieldTrialList::FindFullName(kClientSidePreviewsFieldTrial), + kEnabled, base::CompareCase::SENSITIVE); +} + } // namespace namespace params { @@ -128,7 +137,7 @@ base::TimeDelta OfflinePreviewFreshnessDuration() { "offline_preview_freshness_duration_in_days", 7)); } -net::EffectiveConnectionType EffectiveConnectionTypeThresholdForOffline() { +net::EffectiveConnectionType DefaultEffectiveConnectionTypeThreshold() { return GetParamValueAsECT(kClientSidePreviewsFieldTrial, kEffectiveConnectionTypeThreshold, net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G); @@ -136,10 +145,11 @@ net::EffectiveConnectionType EffectiveConnectionTypeThresholdForOffline() { bool IsOfflinePreviewsEnabled() { // Check if "show_offline_pages" is set to "true". - return IsIncludedInClientSidePreviewsExperimentsFieldTrial() && - base::GetFieldTrialParamValue(kClientSidePreviewsFieldTrial, - kOfflinePagesSlowNetwork) == - kExperimentEnabled; + return base::FeatureList::IsEnabled(features::kOfflinePreviews) || + (IsIncludedInClientSidePreviewsExperimentsFieldTrial() && + base::GetFieldTrialParamValue(kClientSidePreviewsFieldTrial, + kOfflinePagesSlowNetwork) == + kExperimentEnabled); } int OfflinePreviewsVersion() { @@ -147,9 +157,10 @@ int OfflinePreviewsVersion() { } bool IsClientLoFiEnabled() { - return base::StartsWith( - base::FieldTrialList::FindFullName(kClientLoFiExperimentName), kEnabled, - base::CompareCase::SENSITIVE); + return base::FeatureList::IsEnabled(features::kClientLoFi) || + base::StartsWith( + base::FieldTrialList::FindFullName(kClientLoFiExperimentName), + kEnabled, base::CompareCase::SENSITIVE); } int ClientLoFiVersion() { @@ -164,13 +175,20 @@ net::EffectiveConnectionType EffectiveConnectionTypeThresholdForClientLoFi() { } // namespace params -bool IsIncludedInClientSidePreviewsExperimentsFieldTrial() { - // By convention, an experiment in the client-side previews study enables use - // of at least one client-side previews optimization if its name begins with - // "Enabled." - return base::StartsWith( - base::FieldTrialList::FindFullName(kClientSidePreviewsFieldTrial), - kEnabled, base::CompareCase::SENSITIVE); +std::string GetStringNameForType(PreviewsType type) { + switch (type) { + case PreviewsType::OFFLINE: + return "Offline"; + case PreviewsType::LOFI: + return "LoFi"; + case PreviewsType::LITE_PAGE: + return "LitePage"; + case PreviewsType::NONE: + case PreviewsType::LAST: + break; + } + NOTREACHED(); + return std::string(); } } // namespace previews diff --git a/chromium/components/previews/core/previews_experiments.h b/chromium/components/previews/core/previews_experiments.h index 7dd7bc2c42c..b7dd59954a5 100644 --- a/chromium/components/previews/core/previews_experiments.h +++ b/chromium/components/previews/core/previews_experiments.h @@ -5,6 +5,7 @@ #ifndef COMPONENTS_PREVIEWS_CORE_PREVIEWS_EXPERIMENTS_H_ #define COMPONENTS_PREVIEWS_CORE_PREVIEWS_EXPERIMENTS_H_ +#include <string> #include <utility> #include <vector> @@ -47,9 +48,9 @@ base::TimeDelta SingleOptOutDuration(); // shown as a preview. base::TimeDelta OfflinePreviewFreshnessDuration(); -// The threshold of EffectiveConnectionType above which offline previews should -// not be served. -net::EffectiveConnectionType EffectiveConnectionTypeThresholdForOffline(); +// The threshold of EffectiveConnectionType above which previews will trigger by +// default. +net::EffectiveConnectionType DefaultEffectiveConnectionTypeThreshold(); // Whether offline previews are enabled. bool IsOfflinePreviewsEnabled(); @@ -75,18 +76,21 @@ enum class PreviewsType { // The user is shown an offline page as a preview. OFFLINE = 1, - // Replace images with placeholders generated on the client. - CLIENT_LOFI = 2, + // Replace images with placeholders. + LOFI = 2, + + // The user is shown a server lite page. + LITE_PAGE = 3, // Insert new enum values here. Keep values sequential to allow looping // from NONE+1 to LAST-1. - LAST = 3, + LAST = 4, }; typedef std::vector<std::pair<PreviewsType, int>> PreviewsTypeList; -// Returns true if any client-side previews experiment is active. -bool IsIncludedInClientSidePreviewsExperimentsFieldTrial(); +// Gets the string representation of |type|. +std::string GetStringNameForType(PreviewsType type); } // namespace previews diff --git a/chromium/components/previews/core/previews_experiments_unittest.cc b/chromium/components/previews/core/previews_experiments_unittest.cc index be81b3d7587..bc5efb819cf 100644 --- a/chromium/components/previews/core/previews_experiments_unittest.cc +++ b/chromium/components/previews/core/previews_experiments_unittest.cc @@ -7,6 +7,8 @@ #include <map> #include <string> +#include "base/feature_list.h" +#include "base/memory/ptr_util.h" #include "base/metrics/field_trial.h" #include "base/metrics/field_trial_params.h" #include "base/strings/string_util.h" @@ -24,7 +26,6 @@ const char kEnabled[] = "Enabled"; // Verifies that we can enable offline previews via field trial. TEST(PreviewsExperimentsTest, TestFieldTrialOfflinePage) { - EXPECT_FALSE(IsIncludedInClientSidePreviewsExperimentsFieldTrial()); EXPECT_FALSE(params::IsOfflinePreviewsEnabled()); base::FieldTrialList field_trial_list(nullptr); @@ -36,11 +37,26 @@ TEST(PreviewsExperimentsTest, TestFieldTrialOfflinePage) { EXPECT_TRUE(base::FieldTrialList::CreateFieldTrial( kClientSidePreviewsFieldTrial, kEnabled)); - EXPECT_TRUE(IsIncludedInClientSidePreviewsExperimentsFieldTrial()); EXPECT_TRUE(params::IsOfflinePreviewsEnabled()); variations::testing::ClearAllVariationParams(); } +// Verifies that we can enable offline previews via comand line. +TEST(PreviewsExperimentsTest, TestCommandLineOfflinePage) { + EXPECT_FALSE(params::IsOfflinePreviewsEnabled()); + + std::unique_ptr<base::FeatureList> feature_list = + base::MakeUnique<base::FeatureList>(); + + // The feature is explicitly enabled on the command-line. + feature_list->InitializeFromCommandLine("OfflinePreviews", ""); + base::FeatureList::ClearInstanceForTesting(); + base::FeatureList::SetInstance(std::move(feature_list)); + + EXPECT_TRUE(params::IsOfflinePreviewsEnabled()); + base::FeatureList::ClearInstanceForTesting(); +} + // Verifies that the default params are correct, and that custom params can be // set, for both the previews blacklist and offline previews. TEST(PreviewsExperimentsTest, TestParamsForBlackListAndOffline) { @@ -58,7 +74,7 @@ TEST(PreviewsExperimentsTest, TestParamsForBlackListAndOffline) { EXPECT_EQ(base::TimeDelta::FromDays(7), params::OfflinePreviewFreshnessDuration()); EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G, - params::EffectiveConnectionTypeThresholdForOffline()); + params::DefaultEffectiveConnectionTypeThreshold()); EXPECT_EQ(0, params::OfflinePreviewsVersion()); base::FieldTrialList field_trial_list(nullptr); @@ -94,7 +110,7 @@ TEST(PreviewsExperimentsTest, TestParamsForBlackListAndOffline) { EXPECT_EQ(base::TimeDelta::FromDays(12), params::OfflinePreviewFreshnessDuration()); EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_4G, - params::EffectiveConnectionTypeThresholdForOffline()); + params::DefaultEffectiveConnectionTypeThreshold()); EXPECT_EQ(10, params::OfflinePreviewsVersion()); variations::testing::ClearAllVariationParams(); @@ -150,6 +166,22 @@ TEST(PreviewsExperimentsTest, TestEnableClientLoFiWithCustomParams) { variations::testing::ClearAllVariationParams(); } +// Verifies that we can enable offline previews via comand line. +TEST(PreviewsExperimentsTest, TestCommandLineClientLoFi) { + EXPECT_FALSE(params::IsClientLoFiEnabled()); + + std::unique_ptr<base::FeatureList> feature_list = + base::MakeUnique<base::FeatureList>(); + + // The feature is explicitly enabled on the command-line. + feature_list->InitializeFromCommandLine("ClientLoFi", ""); + base::FeatureList::ClearInstanceForTesting(); + base::FeatureList::SetInstance(std::move(feature_list)); + + EXPECT_TRUE(params::IsClientLoFiEnabled()); + base::FeatureList::ClearInstanceForTesting(); +} + } // namespace } // namespace previews diff --git a/chromium/components/previews/core/previews_features.cc b/chromium/components/previews/core/previews_features.cc new file mode 100644 index 00000000000..d88577c41f2 --- /dev/null +++ b/chromium/components/previews/core/previews_features.cc @@ -0,0 +1,19 @@ +// Copyright 2017 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/previews/core/previews_features.h" + +namespace previews { +namespace features { + +// Enables the Offline previews on Android. +const base::Feature kOfflinePreviews{"OfflinePreviews", + base::FEATURE_DISABLED_BY_DEFAULT}; + +// Enables the Client Lo-Fi previews on Android. +const base::Feature kClientLoFi{"ClientLoFi", + base::FEATURE_DISABLED_BY_DEFAULT}; + +} // namespace features +} // namespace previews diff --git a/chromium/components/previews/core/previews_features.h b/chromium/components/previews/core/previews_features.h new file mode 100644 index 00000000000..2225bfd5e41 --- /dev/null +++ b/chromium/components/previews/core/previews_features.h @@ -0,0 +1,19 @@ +// Copyright 2017 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_PREVIEWS_CORE_PREVIEWS_FEATURES_H_ +#define COMPONENTS_PREVIEWS_CORE_PREVIEWS_FEATURES_H_ + +#include "base/feature_list.h" + +namespace previews { +namespace features { + +extern const base::Feature kOfflinePreviews; +extern const base::Feature kClientLoFi; + +} // namespace features +} // namespace previews + +#endif // COMPONENTS_PREVIEWS_CORE_PREVIEWS_FEATURES_H_ diff --git a/chromium/components/previews/core/previews_io_data.cc b/chromium/components/previews/core/previews_io_data.cc index 67403a17c3d..d2f8ed1361d 100644 --- a/chromium/components/previews/core/previews_io_data.cc +++ b/chromium/components/previews/core/previews_io_data.cc @@ -9,10 +9,12 @@ #include "base/files/file_path.h" #include "base/location.h" #include "base/memory/ptr_util.h" -#include "base/metrics/histogram_macros.h" +#include "base/metrics/histogram.h" #include "base/sequenced_task_runner.h" +#include "base/strings/stringprintf.h" #include "base/time/default_clock.h" #include "components/previews/core/previews_black_list.h" +#include "components/previews/core/previews_experiments.h" #include "components/previews/core/previews_opt_out_store.h" #include "components/previews/core/previews_ui_service.h" #include "net/base/load_flags.h" @@ -27,35 +29,30 @@ namespace { void LogPreviewsEligibilityReason(PreviewsEligibilityReason status, PreviewsType type) { - switch (type) { - case PreviewsType::OFFLINE: - UMA_HISTOGRAM_ENUMERATION( - "Previews.EligibilityReason.Offline", static_cast<int>(status), - static_cast<int>(PreviewsEligibilityReason::LAST)); - break; - case PreviewsType::CLIENT_LOFI: - UMA_HISTOGRAM_ENUMERATION( - "Previews.EligibilityReason.ClientLoFi", static_cast<int>(status), - static_cast<int>(PreviewsEligibilityReason::LAST)); - break; - default: - NOTREACHED(); - } + int32_t max_limit = static_cast<int32_t>(PreviewsEligibilityReason::LAST); + base::LinearHistogram::FactoryGet( + base::StringPrintf("Previews.EligibilityReason.%s", + GetStringNameForType(type).c_str()), + 1, max_limit, max_limit + 1, + base::HistogramBase::kUmaTargetedHistogramFlag) + ->Add(static_cast<int>(status)); } -net::EffectiveConnectionType GetEffectiveConnectionTypeThresholdForPreviewsType( - PreviewsType type) { +bool AllowedOnReload(PreviewsType type) { switch (type) { + // These types return new content on refresh. + case PreviewsType::LITE_PAGE: + case PreviewsType::LOFI: + return true; + // Loading these types will always be stale when refreshed. case PreviewsType::OFFLINE: - return params::EffectiveConnectionTypeThresholdForOffline(); - case PreviewsType::CLIENT_LOFI: - return params::EffectiveConnectionTypeThresholdForClientLoFi(); + return false; case PreviewsType::NONE: case PreviewsType::LAST: break; } NOTREACHED(); - return net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN; + return false; } } // namespace @@ -110,6 +107,14 @@ void PreviewsIOData::ClearBlackList(base::Time begin_time, bool PreviewsIOData::ShouldAllowPreview(const net::URLRequest& request, PreviewsType type) const { + return ShouldAllowPreviewAtECT( + request, type, params::DefaultEffectiveConnectionTypeThreshold()); +} + +bool PreviewsIOData::ShouldAllowPreviewAtECT( + const net::URLRequest& request, + PreviewsType type, + net::EffectiveConnectionType effective_connection_type_threshold) const { if (is_enabled_callback_.is_null() || !previews_black_list_) { LogPreviewsEligibilityReason( PreviewsEligibilityReason::BLACKLIST_UNAVAILABLE, type); @@ -119,7 +124,7 @@ bool PreviewsIOData::ShouldAllowPreview(const net::URLRequest& request, return false; // The blacklist will disallow certain hosts for periods of time based on - // user's opting out of the preview + // user's opting out of the preview. PreviewsEligibilityReason status = previews_black_list_->IsLoadedAndAllowed(request.url(), type); if (status != PreviewsEligibilityReason::ALLOWED) { @@ -137,20 +142,21 @@ bool PreviewsIOData::ShouldAllowPreview(const net::URLRequest& request, } if (network_quality_estimator->GetEffectiveConnectionType() > - GetEffectiveConnectionTypeThresholdForPreviewsType(type)) { + effective_connection_type_threshold) { LogPreviewsEligibilityReason(PreviewsEligibilityReason::NETWORK_NOT_SLOW, type); return false; } // LOAD_VALIDATE_CACHE or LOAD_BYPASS_CACHE mean the user reloaded the page. // If this is a query for offline previews, reloads should be disallowed. - if (type == PreviewsType::OFFLINE && - (request.load_flags() & - (net::LOAD_VALIDATE_CACHE | net::LOAD_BYPASS_CACHE))) { - LogPreviewsEligibilityReason( - PreviewsEligibilityReason::RELOAD_DISALLOWED_FOR_OFFLINE, type); + if (!AllowedOnReload(type) && + request.load_flags() & + (net::LOAD_VALIDATE_CACHE | net::LOAD_BYPASS_CACHE)) { + LogPreviewsEligibilityReason(PreviewsEligibilityReason::RELOAD_DISALLOWED, + type); return false; } + LogPreviewsEligibilityReason(PreviewsEligibilityReason::ALLOWED, type); return true; } diff --git a/chromium/components/previews/core/previews_io_data.h b/chromium/components/previews/core/previews_io_data.h index f6e85331562..f44fa304736 100644 --- a/chromium/components/previews/core/previews_io_data.h +++ b/chromium/components/previews/core/previews_io_data.h @@ -16,6 +16,7 @@ #include "base/time/time.h" #include "components/previews/core/previews_decider.h" #include "components/previews/core/previews_experiments.h" +#include "net/nqe/effective_connection_type.h" class GURL; @@ -42,9 +43,10 @@ class PreviewsIOData : public PreviewsDecider { // Stores |previews_ui_service| as |previews_ui_service_| and posts a task to // InitializeOnIOThread on the IO thread. - void Initialize(base::WeakPtr<PreviewsUIService> previews_ui_service, - std::unique_ptr<PreviewsOptOutStore> previews_opt_out_store, - const PreviewsIsEnabledCallback& is_enabled_callback); + virtual void Initialize( + base::WeakPtr<PreviewsUIService> previews_ui_service, + std::unique_ptr<PreviewsOptOutStore> previews_opt_out_store, + const PreviewsIsEnabledCallback& is_enabled_callback); // Adds a navigation to |url| to the black list with result |opt_out|. void AddPreviewNavigation(const GURL& url, bool opt_out, PreviewsType type); @@ -59,6 +61,11 @@ class PreviewsIOData : public PreviewsDecider { // PreviewsDecider implementation: bool ShouldAllowPreview(const net::URLRequest& request, PreviewsType type) const override; + bool ShouldAllowPreviewAtECT( + const net::URLRequest& request, + PreviewsType type, + net::EffectiveConnectionType effective_connection_type_threshold) + const override; protected: // Posts a task to SetIOData for |previews_ui_service_| on the UI thread with diff --git a/chromium/components/previews/core/previews_io_data_unittest.cc b/chromium/components/previews/core/previews_io_data_unittest.cc index 604cb1aa060..2de0ade9039 100644 --- a/chromium/components/previews/core/previews_io_data_unittest.cc +++ b/chromium/components/previews/core/previews_io_data_unittest.cc @@ -24,6 +24,7 @@ #include "base/time/time.h" #include "components/previews/core/previews_black_list.h" #include "components/previews/core/previews_black_list_item.h" +#include "components/previews/core/previews_experiments.h" #include "components/previews/core/previews_opt_out_store.h" #include "components/previews/core/previews_ui_service.h" #include "components/variations/variations_associated_data.h" @@ -40,15 +41,16 @@ namespace previews { namespace { -// TODO(sclittle): Tests should be testing the actual prod code that checks if -// the appropriate field trial is enabled for the preview, instead of testing -// this function here. Consider moving that code out of -// chrome/browser/previews/previews_service.cc and into the previews/ component. +// This method simulates the actual behavior of the passed in callback, which is +// validated in other tests. For simplicity, offline, lite page, and server LoFi +// use the offline previews check. Client LoFi uses a seperate check to verify +// that types are treated differently. bool IsPreviewFieldTrialEnabled(PreviewsType type) { switch (type) { case PreviewsType::OFFLINE: + case PreviewsType::LITE_PAGE: return params::IsOfflinePreviewsEnabled(); - case PreviewsType::CLIENT_LOFI: + case PreviewsType::LOFI: return params::IsClientLoFiEnabled(); case PreviewsType::NONE: case PreviewsType::LAST: @@ -273,6 +275,25 @@ TEST_F(PreviewsIODataTest, TestDisallowOfflineWhenNetworkQualityUnavailable) { 1); } +TEST_F(PreviewsIODataTest, TestAllowLitePageWhenNetworkQualityFast) { + // LoFi and LitePage check NQE on their own. + InitializeUIService(); + CreateFieldTrialWithParams("ClientSidePreviews", "Enabled", + {{"show_offline_pages", "true"}, + {"max_allowed_effective_connection_type", "2G"}}); + + network_quality_estimator()->set_effective_connection_type( + net::EFFECTIVE_CONNECTION_TYPE_3G); + + base::HistogramTester histogram_tester; + EXPECT_TRUE(io_data()->ShouldAllowPreviewAtECT( + *CreateRequest(), PreviewsType::LITE_PAGE, + net::EFFECTIVE_CONNECTION_TYPE_4G)); + histogram_tester.ExpectUniqueSample( + "Previews.EligibilityReason.LitePage", + static_cast<int>(PreviewsEligibilityReason::ALLOWED), 1); +} + TEST_F(PreviewsIODataTest, TestDisallowOfflineWhenNetworkQualityFast) { InitializeUIService(); CreateFieldTrialWithParams("ClientSidePreviews", "Enabled", @@ -306,9 +327,7 @@ TEST_F(PreviewsIODataTest, TestDisallowOfflineOnReload) { EXPECT_FALSE(io_data()->ShouldAllowPreview(*request, PreviewsType::OFFLINE)); histogram_tester.ExpectUniqueSample( "Previews.EligibilityReason.Offline", - static_cast<int>( - PreviewsEligibilityReason::RELOAD_DISALLOWED_FOR_OFFLINE), - 1); + static_cast<int>(PreviewsEligibilityReason::RELOAD_DISALLOWED), 1); } TEST_F(PreviewsIODataTest, TestAllowOffline) { @@ -332,9 +351,10 @@ TEST_F(PreviewsIODataTest, ClientLoFiDisallowedByDefault) { InitializeUIService(); base::HistogramTester histogram_tester; - EXPECT_FALSE(io_data()->ShouldAllowPreview(*CreateRequest(), - PreviewsType::CLIENT_LOFI)); - histogram_tester.ExpectTotalCount("Previews.EligibilityReason.ClientLoFi", 0); + EXPECT_FALSE(io_data()->ShouldAllowPreviewAtECT( + *CreateRequest(), PreviewsType::LOFI, + params::EffectiveConnectionTypeThresholdForClientLoFi())); + histogram_tester.ExpectTotalCount("Previews.EligibilityReason.LoFi", 0); } TEST_F(PreviewsIODataTest, ClientLoFiDisallowedWhenFieldTrialDisabled) { @@ -342,9 +362,10 @@ TEST_F(PreviewsIODataTest, ClientLoFiDisallowedWhenFieldTrialDisabled) { CreateFieldTrialWithParams("PreviewsClientLoFi", "Disabled", {}); base::HistogramTester histogram_tester; - EXPECT_FALSE(io_data()->ShouldAllowPreview(*CreateRequest(), - PreviewsType::CLIENT_LOFI)); - histogram_tester.ExpectTotalCount("Previews.EligibilityReason.ClientLoFi", 0); + EXPECT_FALSE(io_data()->ShouldAllowPreviewAtECT( + *CreateRequest(), PreviewsType::LOFI, + params::EffectiveConnectionTypeThresholdForClientLoFi())); + histogram_tester.ExpectTotalCount("Previews.EligibilityReason.LoFi", 0); } TEST_F(PreviewsIODataTest, ClientLoFiDisallowedWhenNetworkQualityUnavailable) { @@ -355,10 +376,11 @@ TEST_F(PreviewsIODataTest, ClientLoFiDisallowedWhenNetworkQualityUnavailable) { net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN); base::HistogramTester histogram_tester; - EXPECT_FALSE(io_data()->ShouldAllowPreview(*CreateRequest(), - PreviewsType::CLIENT_LOFI)); + EXPECT_FALSE(io_data()->ShouldAllowPreviewAtECT( + *CreateRequest(), PreviewsType::LOFI, + params::EffectiveConnectionTypeThresholdForClientLoFi())); histogram_tester.ExpectUniqueSample( - "Previews.EligibilityReason.ClientLoFi", + "Previews.EligibilityReason.LoFi", static_cast<int>(PreviewsEligibilityReason::NETWORK_QUALITY_UNAVAILABLE), 1); } @@ -372,10 +394,11 @@ TEST_F(PreviewsIODataTest, ClientLoFiDisallowedWhenNetworkFast) { net::EFFECTIVE_CONNECTION_TYPE_3G); base::HistogramTester histogram_tester; - EXPECT_FALSE(io_data()->ShouldAllowPreview(*CreateRequest(), - PreviewsType::CLIENT_LOFI)); + EXPECT_FALSE(io_data()->ShouldAllowPreviewAtECT( + *CreateRequest(), PreviewsType::LOFI, + params::EffectiveConnectionTypeThresholdForClientLoFi())); histogram_tester.ExpectUniqueSample( - "Previews.EligibilityReason.ClientLoFi", + "Previews.EligibilityReason.LoFi", static_cast<int>(PreviewsEligibilityReason::NETWORK_NOT_SLOW), 1); } @@ -388,10 +411,11 @@ TEST_F(PreviewsIODataTest, ClientLoFiAllowed) { net::EFFECTIVE_CONNECTION_TYPE_2G); base::HistogramTester histogram_tester; - EXPECT_TRUE(io_data()->ShouldAllowPreview(*CreateRequest(), - PreviewsType::CLIENT_LOFI)); + EXPECT_TRUE(io_data()->ShouldAllowPreviewAtECT( + *CreateRequest(), PreviewsType::LOFI, + params::EffectiveConnectionTypeThresholdForClientLoFi())); histogram_tester.ExpectUniqueSample( - "Previews.EligibilityReason.ClientLoFi", + "Previews.EligibilityReason.LoFi", static_cast<int>(PreviewsEligibilityReason::ALLOWED), 1); } @@ -407,10 +431,11 @@ TEST_F(PreviewsIODataTest, ClientLoFiAllowedOnReload) { request->SetLoadFlags(net::LOAD_BYPASS_CACHE); base::HistogramTester histogram_tester; - EXPECT_TRUE( - io_data()->ShouldAllowPreview(*request, PreviewsType::CLIENT_LOFI)); + EXPECT_TRUE(io_data()->ShouldAllowPreviewAtECT( + *request, PreviewsType::LOFI, + params::EffectiveConnectionTypeThresholdForClientLoFi())); histogram_tester.ExpectUniqueSample( - "Previews.EligibilityReason.ClientLoFi", + "Previews.EligibilityReason.LoFi", static_cast<int>(PreviewsEligibilityReason::ALLOWED), 1); } diff --git a/chromium/components/previews/core/previews_opt_out_store_sql.cc b/chromium/components/previews/core/previews_opt_out_store_sql.cc index 4b9b97b67d9..1446b0a8bb1 100644 --- a/chromium/components/previews/core/previews_opt_out_store_sql.cc +++ b/chromium/components/previews/core/previews_opt_out_store_sql.cc @@ -17,6 +17,7 @@ #include "base/memory/ptr_util.h" #include "base/metrics/histogram_macros.h" #include "base/sequenced_task_runner.h" +#include "base/single_thread_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/threading/thread_task_runner_handle.h" #include "components/previews/core/previews_black_list.h" |