Commit 2e8bced7 authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot

Put logic to disable Lite video on seek behind finch

This CL moves the logic to disable lite videos on seek
behind experiment parameter, and also disables that by default.

The param can be enabled again once we are reasonably confident
that it's not too aggressive and we can run an experiment to
verify that.

Bug: 1145646
Change-Id: Iedeb82abc1124568986f8d87cf92ef2f48c5eb2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524925
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: default avatarMichael Crouse <mcrouse@chromium.org>
Reviewed-by: default avatarrajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826172}
parent b577eeb6
...@@ -65,16 +65,19 @@ class LiteVideoBrowserTest : public InProcessBrowserTest { ...@@ -65,16 +65,19 @@ class LiteVideoBrowserTest : public InProcessBrowserTest {
public: public:
explicit LiteVideoBrowserTest(bool enable_lite_mode = true, explicit LiteVideoBrowserTest(bool enable_lite_mode = true,
bool enable_lite_video_feature = true, bool enable_lite_video_feature = true,
int max_rebuffers_before_stop = 1) int max_rebuffers_before_stop = 1,
bool should_disable_lite_videos_on_seek = false)
: enable_lite_mode_(enable_lite_mode) { : enable_lite_mode_(enable_lite_mode) {
std::vector<base::test::ScopedFeatureList::FeatureAndParams> std::vector<base::test::ScopedFeatureList::FeatureAndParams>
enabled_features; enabled_features;
if (enable_lite_video_feature) { base::FieldTrialParams params;
enabled_features.push_back( params["max_rebuffers_per_frame"] =
{features::kLiteVideo, base::NumberToString(max_rebuffers_before_stop);
{{"max_rebuffers_per_frame", params["disable_on_media_player_seek"] =
base::NumberToString(max_rebuffers_before_stop)}}}); should_disable_lite_videos_on_seek ? "true" : "false";
}
if (enable_lite_video_feature)
enabled_features.emplace_back(features::kLiteVideo, params);
std::vector<base::Feature> disabled_features = { std::vector<base::Feature> disabled_features = {
// Disable fallback after decode error to avoid unexpected test pass on // Disable fallback after decode error to avoid unexpected test pass on
...@@ -160,7 +163,8 @@ class LiteVideoBrowserTest : public InProcessBrowserTest { ...@@ -160,7 +163,8 @@ class LiteVideoBrowserTest : public InProcessBrowserTest {
// Need to make tests more reliable but feature only targeted // Need to make tests more reliable but feature only targeted
// for Android. Currently there are potential race conditions // for Android. Currently there are potential race conditions
// on throttle timing and counts // on throttle timing and counts
#if defined(OS_WIN) || defined(OS_MAC) || defined(OS_CHROMEOS) #if defined(OS_WIN) || defined(OS_MAC) || defined(OS_CHROMEOS) || \
defined(OS_LINUX)
#define DISABLE_ON_WIN_MAC_CHROMEOS(x) DISABLED_##x #define DISABLE_ON_WIN_MAC_CHROMEOS(x) DISABLED_##x
#else #else
#define DISABLE_ON_WIN_MAC_CHROMEOS(x) x #define DISABLE_ON_WIN_MAC_CHROMEOS(x) x
...@@ -308,8 +312,26 @@ IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest, ...@@ -308,8 +312,26 @@ IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest,
lite_video::LiteVideoThrottleResult::kThrottleStoppedOnRebuffer)); lite_video::LiteVideoThrottleResult::kThrottleStoppedOnRebuffer));
} }
class LiteVideoStopThrottlingOnPlaybackSeekBrowserTest
: public ::testing::WithParamInterface<bool>,
public LiteVideoBrowserTest {
public:
LiteVideoStopThrottlingOnPlaybackSeekBrowserTest()
: LiteVideoBrowserTest(true /*enable_lite_mode*/,
true /*enable_lite_video_feature*/,
1 /* max_rebuffers_before_stop */,
should_disable_lite_videos_on_seek()) {}
bool should_disable_lite_videos_on_seek() const { return GetParam(); }
};
INSTANTIATE_TEST_SUITE_P(,
LiteVideoStopThrottlingOnPlaybackSeekBrowserTest,
::testing::Bool());
// When video is seeked throttling is stopped. // When video is seeked throttling is stopped.
IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest, StopsThrottlingOnPlaybackSeek) { IN_PROC_BROWSER_TEST_P(LiteVideoStopThrottlingOnPlaybackSeekBrowserTest,
StopsThrottlingOnPlaybackSeek) {
ukm::TestAutoSetUkmRecorder ukm_recorder; ukm::TestAutoSetUkmRecorder ukm_recorder;
TestMSEPlayback("bear-vp9.webm", "2000", "2000", false); TestMSEPlayback("bear-vp9.webm", "2000", "2000", false);
...@@ -324,13 +346,17 @@ IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest, StopsThrottlingOnPlaybackSeek) { ...@@ -324,13 +346,17 @@ IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest, StopsThrottlingOnPlaybackSeek) {
"document.querySelector('video').currentTime = 1")); "document.querySelector('video').currentTime = 1"));
// Verify some responses were throttled and then throttling is stopped. // Verify some responses were throttled and then throttling is stopped.
RetryForHistogramUntilCountReached(histogram_tester(), if (should_disable_lite_videos_on_seek()) {
"LiteVideo.HintsAgent.StopThrottling", 1); RetryForHistogramUntilCountReached(
histogram_tester(), "LiteVideo.MediaPlayerSeek.StopThrottling", 1);
}
EXPECT_GE(1U, histogram_tester() EXPECT_GE(1U, histogram_tester()
.GetAllSamples("LiteVideo.URLLoader.ThrottleLatency") .GetAllSamples("LiteVideo.URLLoader.ThrottleLatency")
.size()); .size());
EXPECT_EQ(1U, histogram_tester() EXPECT_EQ(should_disable_lite_videos_on_seek() ? 1U : 0U,
.GetAllSamples("LiteVideo.HintsAgent.StopThrottling") histogram_tester()
.GetAllSamples("LiteVideo.MediaPlayerSeek.StopThrottling")
.size()); .size());
EXPECT_GE( EXPECT_GE(
1U, histogram_tester() 1U, histogram_tester()
......
...@@ -140,5 +140,10 @@ int GetMaxRebuffersPerFrame() { ...@@ -140,5 +140,10 @@ int GetMaxRebuffersPerFrame() {
"max_rebuffers_per_frame", 1); "max_rebuffers_per_frame", 1);
} }
bool DisableLiteVideoOnMediaPlayerSeek() {
return GetFieldTrialParamByFeatureAsBool(
::features::kLiteVideo, "disable_on_media_player_seek", false);
}
} // namespace features } // namespace features
} // namespace lite_video } // namespace lite_video
...@@ -79,6 +79,8 @@ bool IsLiteVideoNotAllowedForPageTransition(ui::PageTransition page_transition); ...@@ -79,6 +79,8 @@ bool IsLiteVideoNotAllowedForPageTransition(ui::PageTransition page_transition);
// should be stopped. // should be stopped.
int GetMaxRebuffersPerFrame(); int GetMaxRebuffersPerFrame();
bool DisableLiteVideoOnMediaPlayerSeek();
} // namespace features } // namespace features
} // namespace lite_video } // namespace lite_video
......
...@@ -278,6 +278,9 @@ void LiteVideoObserver::MediaBufferUnderflow(const content::MediaPlayerId& id) { ...@@ -278,6 +278,9 @@ void LiteVideoObserver::MediaBufferUnderflow(const content::MediaPlayerId& id) {
void LiteVideoObserver::MediaPlayerSeek(const content::MediaPlayerId& id) { void LiteVideoObserver::MediaPlayerSeek(const content::MediaPlayerId& id) {
content::RenderFrameHost* render_frame_host = id.render_frame_host; content::RenderFrameHost* render_frame_host = id.render_frame_host;
if (!lite_video::features::DisableLiteVideoOnMediaPlayerSeek())
return;
if (!render_frame_host || !render_frame_host->GetProcess()) if (!render_frame_host || !render_frame_host->GetProcess())
return; return;
...@@ -297,6 +300,8 @@ void LiteVideoObserver::MediaPlayerSeek(const content::MediaPlayerId& id) { ...@@ -297,6 +300,8 @@ void LiteVideoObserver::MediaPlayerSeek(const content::MediaPlayerId& id) {
render_frame_host->GetRemoteAssociatedInterfaces()->GetInterface( render_frame_host->GetRemoteAssociatedInterfaces()->GetInterface(
&loading_hints_agent); &loading_hints_agent);
LOCAL_HISTOGRAM_BOOLEAN("LiteVideo.MediaPlayerSeek.StopThrottling", true);
loading_hints_agent->StopThrottlingMediaRequests(); loading_hints_agent->StopThrottlingMediaRequests();
} }
......
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