Commit 6f9ca96c authored by Michael Crouse's avatar Michael Crouse Committed by Commit Bot

Reland "[LiteVideo] Do not attempt LiteVideos on Reloads/Forward-Back."

This is a reland of 6ea86d08

The issue was the network condition checks which are now override by
a switch for these tests (see cl/2284978).

Original change's description:
> [LiteVideo] Do not attempt LiteVideos on Reloads/Forward-Back.
>
> This change prevents LiteVideos from being attempted when the
> navigation is a reload or forward-back page transition. It also
> adds an entry for host of the navigation to the user blocklist to
> allow users to soft opt-out of the optimization if they reload
> frequently on a host.
>
> A future change will clear the blocklist when the browser history
> is wiped.
>
> Bug: 1096796
> Change-Id: I687d4c9cd0844d7475f6a3baedacebcf73a48845
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2276681
> Commit-Queue: Michael Crouse <mcrouse@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Auto-Submit: Michael Crouse <mcrouse@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Sophie Chang <sophiechang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#786127}

TBR=tbansal@chromium.org

Bug: 1096796
Change-Id: I5164e510bfb06fc97e45ab9c82c482dc25529e1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2287380Reviewed-by: default avatarMichael Crouse <mcrouse@chromium.org>
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786432}
parent 00eaeb3f
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "content/public/browser/network_service_instance.h" #include "content/public/browser/network_service_instance.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "net/nqe/effective_connection_type.h" #include "net/nqe/effective_connection_type.h"
#include "ui/base/page_transition_types.h"
namespace { namespace {
...@@ -113,7 +114,7 @@ LiteVideoDecider::~LiteVideoDecider() { ...@@ -113,7 +114,7 @@ LiteVideoDecider::~LiteVideoDecider() {
} }
base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo( base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo(
content::NavigationHandle* navigation_handle) const { content::NavigationHandle* navigation_handle) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsLiteVideoAllowedForUser(Profile::FromBrowserContext( if (!IsLiteVideoAllowedForUser(Profile::FromBrowserContext(
...@@ -133,14 +134,26 @@ base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo( ...@@ -133,14 +134,26 @@ base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo(
return base::nullopt; return base::nullopt;
} }
// TODO(crbug/1096796): Add checks on the page transition and update
// the blocklist if needed page transition.
GURL url = navigation_handle->GetURL(); GURL url = navigation_handle->GetURL();
if (!url.SchemeIsHTTPOrHTTPS()) if (!url.SchemeIsHTTPOrHTTPS())
return base::nullopt; return base::nullopt;
// Reloads and Forward-Back navigations are considered opt-outs and are added
// to the blocklist so that a host that is frequently reloaded on does not get
// LiteVideos.
bool is_reload = PageTransitionCoreTypeIs(
navigation_handle->GetPageTransition(), ui::PAGE_TRANSITION_RELOAD);
if (is_reload || (navigation_handle->GetPageTransition() &
ui::PAGE_TRANSITION_FORWARD_BACK)) {
user_blocklist_->AddNavigationToBlocklist(navigation_handle, true);
ScopedLiteVideoDecisionRecorder scoped_decision_recorder(
is_reload ? LiteVideoBlocklistReason::kNavigationReload
: LiteVideoBlocklistReason::kNavigationForwardBack,
navigation_handle->IsInMainFrame());
return base::nullopt;
}
auto blocklist_reason = auto blocklist_reason =
user_blocklist_->IsLiteVideoAllowedOnNavigation(navigation_handle); user_blocklist_->IsLiteVideoAllowedOnNavigation(navigation_handle);
ScopedLiteVideoDecisionRecorder scoped_decision_recorder( ScopedLiteVideoDecisionRecorder scoped_decision_recorder(
...@@ -154,6 +167,9 @@ base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo( ...@@ -154,6 +167,9 @@ base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo(
if (blocklist_reason != LiteVideoBlocklistReason::kAllowed || !hint) if (blocklist_reason != LiteVideoBlocklistReason::kAllowed || !hint)
return base::nullopt; return base::nullopt;
// The navigation will have the LiteVideo optimization triggered so
// update the navigation blocklist.
user_blocklist_->AddNavigationToBlocklist(navigation_handle, false);
return hint; return hint;
} }
......
...@@ -36,8 +36,10 @@ class LiteVideoDecider ...@@ -36,8 +36,10 @@ class LiteVideoDecider
// Determine if the navigation can have the LiteVideo optimization // Determine if the navigation can have the LiteVideo optimization
// applied and returns the LiteVideoHint to use for throttling if one exists. // applied and returns the LiteVideoHint to use for throttling if one exists.
// This also updates the blocklist based on the navigation provided and should
// be limited to one call per navigation.
base::Optional<LiteVideoHint> CanApplyLiteVideo( base::Optional<LiteVideoHint> CanApplyLiteVideo(
content::NavigationHandle* navigation_handle) const; content::NavigationHandle* navigation_handle);
// Override the blocklist used by |this| for testing. // Override the blocklist used by |this| for testing.
void SetUserBlocklistForTesting( void SetUserBlocklistForTesting(
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "content/public/test/test_renderer_host.h" #include "content/public/test/test_renderer_host.h"
#include "content/public/test/web_contents_tester.h" #include "content/public/test/web_contents_tester.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/page_transition_types.h"
#include "url/gurl.h" #include "url/gurl.h"
class TestLiteVideoUserBlocklist : public lite_video::LiteVideoUserBlocklist { class TestLiteVideoUserBlocklist : public lite_video::LiteVideoUserBlocklist {
...@@ -142,6 +143,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyOnNonHTTPOrHTTPSURL) { ...@@ -142,6 +143,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyOnNonHTTPOrHTTPSURL) {
content::MockNavigationHandle navigation_handle(web_contents()); content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(GURL("chrome:://about")); navigation_handle.set_url(GURL("chrome:://about"));
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_TYPED);
base::Optional<lite_video::LiteVideoHint> hint = base::Optional<lite_video::LiteVideoHint> hint =
lite_video_decider()->CanApplyLiteVideo(&navigation_handle); lite_video_decider()->CanApplyLiteVideo(&navigation_handle);
EXPECT_FALSE(hint); EXPECT_FALSE(hint);
...@@ -159,6 +161,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyNoHintAndHostBlocklisted) { ...@@ -159,6 +161,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyNoHintAndHostBlocklisted) {
lite_video::LiteVideoBlocklistReason::kNavigationBlocklisted); lite_video::LiteVideoBlocklistReason::kNavigationBlocklisted);
content::MockNavigationHandle navigation_handle(web_contents()); content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(GURL("https://NoVideo.com")); navigation_handle.set_url(GURL("https://NoVideo.com"));
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_TYPED);
base::Optional<lite_video::LiteVideoHint> hint = base::Optional<lite_video::LiteVideoHint> hint =
lite_video_decider()->CanApplyLiteVideo(&navigation_handle); lite_video_decider()->CanApplyLiteVideo(&navigation_handle);
EXPECT_FALSE(hint); EXPECT_FALSE(hint);
...@@ -175,6 +178,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyAllowedButNoHint) { ...@@ -175,6 +178,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyAllowedButNoHint) {
content::MockNavigationHandle navigation_handle(web_contents()); content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(GURL("https://NoVideo.com")); navigation_handle.set_url(GURL("https://NoVideo.com"));
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_TYPED);
base::Optional<lite_video::LiteVideoHint> hint = base::Optional<lite_video::LiteVideoHint> hint =
lite_video_decider()->CanApplyLiteVideo(&navigation_handle); lite_video_decider()->CanApplyLiteVideo(&navigation_handle);
...@@ -193,6 +197,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyLiteVideo) { ...@@ -193,6 +197,7 @@ TEST_F(LiteVideoDeciderTest, CanApplyLiteVideo) {
GURL url("https://LiteVideo.com"); GURL url("https://LiteVideo.com");
content::MockNavigationHandle navigation_handle(web_contents()); content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(url); navigation_handle.set_url(url);
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_TYPED);
lite_video::LiteVideoHint seeded_hint( lite_video::LiteVideoHint seeded_hint(
/*target_downlink_bandwidth_kbps=*/123, /*target_downlink_bandwidth_kbps=*/123,
/*target_downlink_rtt_latency_ms=*/2500, /*target_downlink_rtt_latency_ms=*/2500,
...@@ -225,6 +230,7 @@ TEST_F(LiteVideoDeciderTest, LiteVideoDisabled) { ...@@ -225,6 +230,7 @@ TEST_F(LiteVideoDeciderTest, LiteVideoDisabled) {
GURL url("https://LiteVideo.com"); GURL url("https://LiteVideo.com");
content::MockNavigationHandle navigation_handle(web_contents()); content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(url); navigation_handle.set_url(url);
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_TYPED);
lite_video::LiteVideoHint seeded_hint( lite_video::LiteVideoHint seeded_hint(
/*target_downlink_bandwidth_kbps=*/123, /*target_downlink_bandwidth_kbps=*/123,
/*target_downlink_rtt_latency_ms=*/2500, /*target_downlink_rtt_latency_ms=*/2500,
...@@ -249,6 +255,7 @@ TEST_F(LiteVideoDeciderTest, LiteVideoCanApplyOnSubframeNavigation) { ...@@ -249,6 +255,7 @@ TEST_F(LiteVideoDeciderTest, LiteVideoCanApplyOnSubframeNavigation) {
GURL url("https://LiteVideo.com"); GURL url("https://LiteVideo.com");
content::MockNavigationHandle navigation_handle(web_contents()); content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(url); navigation_handle.set_url(url);
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_TYPED);
lite_video::LiteVideoHint seeded_hint( lite_video::LiteVideoHint seeded_hint(
/*target_downlink_bandwidth_kbps=*/123, /*target_downlink_bandwidth_kbps=*/123,
/*target_downlink_rtt_latency_ms=*/2500, /*target_downlink_rtt_latency_ms=*/2500,
...@@ -273,3 +280,57 @@ TEST_F(LiteVideoDeciderTest, LiteVideoCanApplyOnSubframeNavigation) { ...@@ -273,3 +280,57 @@ TEST_F(LiteVideoDeciderTest, LiteVideoCanApplyOnSubframeNavigation) {
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.HintCache.HasHint", true, 1); "LiteVideo.CanApplyLiteVideo.HintCache.HasHint", true, 1);
} }
TEST_F(LiteVideoDeciderTest, CanApplyOnReload) {
base::HistogramTester histogram_tester;
SetBlocklistReason(lite_video::LiteVideoBlocklistReason::kAllowed);
GURL url("https://LiteVideo.com");
content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(url);
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_RELOAD);
lite_video::LiteVideoHint seeded_hint(
/*target_downlink_bandwidth_kbps=*/123,
/*target_downlink_rtt_latency_ms=*/2500,
/*kilobytes_to_buffer_before_throttle=*/500);
SeedLiteVideoHintCache(url, seeded_hint);
base::Optional<lite_video::LiteVideoHint> hint =
lite_video_decider()->CanApplyLiteVideo(&navigation_handle);
EXPECT_FALSE(hint);
histogram_tester.ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kNavigationReload, 1);
histogram_tester.ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
histogram_tester.ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.HintCache.HasHint", false, 1);
}
TEST_F(LiteVideoDeciderTest, CanApplyOnBackForwardNavigation) {
base::HistogramTester histogram_tester;
SetBlocklistReason(lite_video::LiteVideoBlocklistReason::kAllowed);
GURL url("https://LiteVideo.com");
content::MockNavigationHandle navigation_handle(web_contents());
navigation_handle.set_url(url);
navigation_handle.set_page_transition(ui::PAGE_TRANSITION_FORWARD_BACK);
lite_video::LiteVideoHint seeded_hint(
/*target_downlink_bandwidth_kbps=*/123,
/*target_downlink_rtt_latency_ms=*/2500,
/*kilobytes_to_buffer_before_throttle=*/500);
SeedLiteVideoHintCache(url, seeded_hint);
base::Optional<lite_video::LiteVideoHint> hint =
lite_video_decider()->CanApplyLiteVideo(&navigation_handle);
EXPECT_FALSE(hint);
histogram_tester.ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kNavigationForwardBack, 1);
histogram_tester.ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
histogram_tester.ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.HintCache.HasHint", false, 1);
}
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "net/nqe/effective_connection_type.h" #include "net/nqe/effective_connection_type.h"
#include "services/network/public/mojom/network_change_manager.mojom-shared.h" #include "services/network/public/mojom/network_change_manager.mojom-shared.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/page_transition_types.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace { namespace {
...@@ -106,7 +107,8 @@ class LiteVideoKeyedServiceBrowserTest ...@@ -106,7 +107,8 @@ class LiteVideoKeyedServiceBrowserTest
void SetUp() override { void SetUp() override {
scoped_feature_list_.InitAndEnableFeatureWithParameters( scoped_feature_list_.InitAndEnableFeatureWithParameters(
{::features::kLiteVideo}, {::features::kLiteVideo},
{{"lite_video_origin_hints", "{\"litevideo.com\": 123}"}}); {{"lite_video_origin_hints", "{\"litevideo.com\": 123}"},
{"user_blocklist_opt_out_history_threshold", "1"}});
InProcessBrowserTest::SetUp(); InProcessBrowserTest::SetUp();
} }
...@@ -225,6 +227,124 @@ IN_PROC_BROWSER_TEST_F(LiteVideoKeyedServiceBrowserTest, ...@@ -225,6 +227,124 @@ IN_PROC_BROWSER_TEST_F(LiteVideoKeyedServiceBrowserTest,
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0); "LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
} }
IN_PROC_BROWSER_TEST_F(LiteVideoKeyedServiceBrowserTest,
LiteVideoCanApplyLiteVideo_Reload) {
WaitForBlocklistToBeLoaded();
EXPECT_TRUE(
LiteVideoKeyedServiceFactory::GetForProfile(browser()->profile()));
// Navigate metrics get recorded.
GURL url("https://testserver.com");
NavigateParams params(browser(), url, ui::PAGE_TRANSITION_RELOAD);
ui_test_utils::NavigateToURL(&params);
EXPECT_GT(RetryForHistogramUntilCountReached(
*histogram_tester(), "LiteVideo.Navigation.HasHint", 1),
0);
histogram_tester()->ExpectUniqueSample("LiteVideo.Navigation.HasHint", false,
1);
histogram_tester()->ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kNavigationReload, 1);
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
// Navigate to confirm that the host is blocklisted due to a reload. This
// happens after one such navigation due to overriding the blocklist
// parameters for testing.
NavigateParams params_blocklisted(browser(), url, ui::PAGE_TRANSITION_TYPED);
ui_test_utils::NavigateToURL(&params_blocklisted);
EXPECT_GT(RetryForHistogramUntilCountReached(
*histogram_tester(), "LiteVideo.Navigation.HasHint", 2),
0);
histogram_tester()->ExpectUniqueSample("LiteVideo.Navigation.HasHint", false,
2);
histogram_tester()->ExpectBucketCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kNavigationBlocklisted, 1);
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
}
IN_PROC_BROWSER_TEST_F(LiteVideoKeyedServiceBrowserTest,
LiteVideoCanApplyLiteVideo_ForwardBack) {
WaitForBlocklistToBeLoaded();
EXPECT_TRUE(
LiteVideoKeyedServiceFactory::GetForProfile(browser()->profile()));
// Navigate metrics get recorded.
GURL url("https://testserver.com");
NavigateParams params(browser(), url, ui::PAGE_TRANSITION_FORWARD_BACK);
ui_test_utils::NavigateToURL(&params);
EXPECT_GT(RetryForHistogramUntilCountReached(
*histogram_tester(), "LiteVideo.Navigation.HasHint", 1),
0);
histogram_tester()->ExpectUniqueSample("LiteVideo.Navigation.HasHint", false,
1);
histogram_tester()->ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kNavigationForwardBack, 1);
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
// Navigate to confirm that the host is blocklisted due to the Forward-Back
// navigation. This happens after one such navigation due to overriding the
// blocklist parameters for testing.
NavigateParams params_blocklisted(browser(), url, ui::PAGE_TRANSITION_TYPED);
ui_test_utils::NavigateToURL(&params_blocklisted);
EXPECT_GT(RetryForHistogramUntilCountReached(
*histogram_tester(), "LiteVideo.Navigation.HasHint", 2),
0);
histogram_tester()->ExpectUniqueSample("LiteVideo.Navigation.HasHint", false,
2);
histogram_tester()->ExpectBucketCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kNavigationBlocklisted, 1);
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
}
IN_PROC_BROWSER_TEST_F(LiteVideoKeyedServiceBrowserTest,
MultipleNavigationsNotBlocklisted) {
WaitForBlocklistToBeLoaded();
EXPECT_TRUE(
LiteVideoKeyedServiceFactory::GetForProfile(browser()->profile()));
GURL url("https://litevideo.com");
// Navigate metrics get recorded.
NavigateParams params(browser(), url, ui::PAGE_TRANSITION_TYPED);
ui_test_utils::NavigateToURL(&params);
EXPECT_GT(RetryForHistogramUntilCountReached(
*histogram_tester(), "LiteVideo.Navigation.HasHint", 1),
0);
histogram_tester()->ExpectUniqueSample("LiteVideo.Navigation.HasHint", true,
1);
histogram_tester()->ExpectUniqueSample(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kAllowed, 1);
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
// Navigate again to ensure that it was not blocklisted.
ui_test_utils::NavigateToURL(&params);
EXPECT_GT(RetryForHistogramUntilCountReached(
*histogram_tester(), "LiteVideo.Navigation.HasHint", 2),
0);
histogram_tester()->ExpectBucketCount("LiteVideo.Navigation.HasHint", true,
2);
histogram_tester()->ExpectBucketCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.MainFrame",
lite_video::LiteVideoBlocklistReason::kAllowed, 2);
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
}
class LiteVideoNetworkConnectionBrowserTest class LiteVideoNetworkConnectionBrowserTest
: public LiteVideoKeyedServiceBrowserTest { : public LiteVideoKeyedServiceBrowserTest {
public: public:
......
...@@ -10,6 +10,18 @@ ...@@ -10,6 +10,18 @@
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
namespace {
// Determine whether the provided navigation is valid and can be queried or
// added to the blocklist.
bool IsNavigationValidForBlocklist(
content::NavigationHandle* navigation_handle) {
GURL navigation_url = navigation_handle->GetURL();
return navigation_url.SchemeIsHTTPOrHTTPS() && navigation_url.has_host();
}
} // namespace
namespace lite_video { namespace lite_video {
// Separator between hosts for the rebuffer blocklist type. // Separator between hosts for the rebuffer blocklist type.
...@@ -18,10 +30,10 @@ constexpr char kLiteVideoBlocklistKeySeparator[] = "_"; ...@@ -18,10 +30,10 @@ constexpr char kLiteVideoBlocklistKeySeparator[] = "_";
// static // static
base::Optional<std::string> LiteVideoUserBlocklist::GetRebufferBlocklistKey( base::Optional<std::string> LiteVideoUserBlocklist::GetRebufferBlocklistKey(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
const GURL url = navigation_handle->GetURL(); if (!IsNavigationValidForBlocklist(navigation_handle))
if (!url.SchemeIsHTTPOrHTTPS() || !url.has_host())
return base::nullopt; return base::nullopt;
const GURL url = navigation_handle->GetURL();
if (navigation_handle->IsInMainFrame()) if (navigation_handle->IsInMainFrame())
return url.host() + kLiteVideoBlocklistKeySeparator; return url.host() + kLiteVideoBlocklistKeySeparator;
...@@ -45,8 +57,7 @@ LiteVideoUserBlocklist::~LiteVideoUserBlocklist() = default; ...@@ -45,8 +57,7 @@ LiteVideoUserBlocklist::~LiteVideoUserBlocklist() = default;
LiteVideoBlocklistReason LiteVideoUserBlocklist::IsLiteVideoAllowedOnNavigation( LiteVideoBlocklistReason LiteVideoUserBlocklist::IsLiteVideoAllowedOnNavigation(
content::NavigationHandle* navigation_handle) const { content::NavigationHandle* navigation_handle) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
GURL navigation_url = navigation_handle->GetURL(); if (!IsNavigationValidForBlocklist(navigation_handle))
if (!navigation_url.SchemeIsHTTPOrHTTPS() || !navigation_url.has_host())
return LiteVideoBlocklistReason::kNavigationNotEligibile; return LiteVideoBlocklistReason::kNavigationNotEligibile;
std::vector<blocklist::BlocklistReason> passed_reasons; std::vector<blocklist::BlocklistReason> passed_reasons;
...@@ -113,4 +124,14 @@ LiteVideoUserBlocklist::GetAllowedTypes() const { ...@@ -113,4 +124,14 @@ LiteVideoUserBlocklist::GetAllowedTypes() const {
features::LiteVideoBlocklistVersion()}}; features::LiteVideoBlocklistVersion()}};
} }
void LiteVideoUserBlocklist::AddNavigationToBlocklist(
content::NavigationHandle* navigation_handle,
bool opt_out) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsNavigationValidForBlocklist(navigation_handle))
return;
AddEntry(navigation_handle->GetURL().host(), opt_out,
static_cast<int>(LiteVideoBlocklistType::kNavigationBlocklist));
}
} // namespace lite_video } // namespace lite_video
...@@ -52,9 +52,13 @@ enum class LiteVideoBlocklistReason { ...@@ -52,9 +52,13 @@ enum class LiteVideoBlocklistReason {
// LiteVideos were blocked because the host was on the // LiteVideos were blocked because the host was on the
// NavigationBlocklist. // NavigationBlocklist.
kNavigationBlocklisted, kNavigationBlocklisted,
// LiteVideo not shown because it was a reloaded navigation.
kNavigationReload,
// LiteVideo not shown because it was a forward-back navigation.
kNavigationForwardBack,
// Insert new values before this line. // Insert new values before this line.
kMaxValue = kNavigationBlocklisted, kMaxValue = kNavigationForwardBack,
}; };
// The LiteVideoUserBlocklist maintains information about hosts the user // The LiteVideoUserBlocklist maintains information about hosts the user
...@@ -77,6 +81,11 @@ class LiteVideoUserBlocklist : public blocklist::OptOutBlocklist { ...@@ -77,6 +81,11 @@ class LiteVideoUserBlocklist : public blocklist::OptOutBlocklist {
virtual LiteVideoBlocklistReason IsLiteVideoAllowedOnNavigation( virtual LiteVideoBlocklistReason IsLiteVideoAllowedOnNavigation(
content::NavigationHandle* navigation_handle) const; content::NavigationHandle* navigation_handle) const;
// Update the entry within the NavigationBlocklistType for the
// |navigation_handle| based on whether it was an opt-out or not.
void AddNavigationToBlocklist(content::NavigationHandle* navigation_handle,
bool opt_out);
protected: protected:
// OptOutBlocklist: // OptOutBlocklist:
bool ShouldUseSessionPolicy(base::TimeDelta* duration, bool ShouldUseSessionPolicy(base::TimeDelta* duration,
......
...@@ -137,6 +137,8 @@ class LiteVideoUserBlocklistTest : public ChromeRenderViewHostTestHarness { ...@@ -137,6 +137,8 @@ class LiteVideoUserBlocklistTest : public ChromeRenderViewHostTestHarness {
TestLiteVideoUserBlocklist* blocklist() { return blocklist_.get(); } TestLiteVideoUserBlocklist* blocklist() { return blocklist_.get(); }
base::SimpleTestClock* test_clock() { return &test_clock_; }
private: private:
EmptyOptOutBlocklistDelegate blocklist_delegate_; EmptyOptOutBlocklistDelegate blocklist_delegate_;
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
...@@ -154,11 +156,28 @@ TEST_F(LiteVideoUserBlocklistTest, NavigationNotEligibile) { ...@@ -154,11 +156,28 @@ TEST_F(LiteVideoUserBlocklistTest, NavigationNotEligibile) {
TEST_F(LiteVideoUserBlocklistTest, NavigationBlocklistedByNavigationBlocklist) { TEST_F(LiteVideoUserBlocklistTest, NavigationBlocklistedByNavigationBlocklist) {
ConfigBlocklistParamsForTesting(); ConfigBlocklistParamsForTesting();
GURL url("https://test.com"); GURL url("https://test.com");
SeedBlocklist(url.host(), LiteVideoBlocklistType::kNavigationBlocklist); content::MockNavigationHandle nav_handle;
nav_handle.set_url(url);
blocklist()->AddNavigationToBlocklist(&nav_handle, true);
EXPECT_EQ(CheckBlocklistForMainframeNavigation(url), EXPECT_EQ(CheckBlocklistForMainframeNavigation(url),
LiteVideoBlocklistReason::kNavigationBlocklisted); LiteVideoBlocklistReason::kNavigationBlocklisted);
} }
TEST_F(LiteVideoUserBlocklistTest, NavigationUnblocklistedByNavigation) {
ConfigBlocklistParamsForTesting();
GURL url("https://test.com");
content::MockNavigationHandle nav_handle;
nav_handle.set_url(url);
blocklist()->AddNavigationToBlocklist(&nav_handle, true);
test_clock()->Advance(base::TimeDelta::FromSeconds(1));
EXPECT_EQ(CheckBlocklistForMainframeNavigation(url),
LiteVideoBlocklistReason::kNavigationBlocklisted);
blocklist()->AddNavigationToBlocklist(&nav_handle, false);
test_clock()->Advance(base::TimeDelta::FromSeconds(1));
EXPECT_EQ(CheckBlocklistForMainframeNavigation(url),
LiteVideoBlocklistReason::kAllowed);
}
TEST_F(LiteVideoUserBlocklistTest, TEST_F(LiteVideoUserBlocklistTest,
MainframeNavigationBlocklistedByRebufferBlocklist) { MainframeNavigationBlocklistedByRebufferBlocklist) {
ConfigBlocklistParamsForTesting(); ConfigBlocklistParamsForTesting();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment