Commit 37c7a793 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.

Change-Id: I950b6d42750f11156ba0f0afad5989ac19832a7b
Bug: 1145646
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519719
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@{#824767}
parent 8ca79b5d
......@@ -65,15 +65,19 @@ class LiteVideoBrowserTest : public InProcessBrowserTest {
public:
explicit LiteVideoBrowserTest(bool enable_lite_mode = 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) {
std::vector<base::test::ScopedFeatureList::FeatureAndParams>
enabled_features;
base::FieldTrialParams params;
params["max_rebuffers_per_frame"] =
base::NumberToString(max_rebuffers_before_stop);
params["disable_on_media_player_seek"] =
should_disable_lite_videos_on_seek ? "true" : "false";
if (enable_lite_video_feature) {
enabled_features.push_back(
{features::kLiteVideo,
{{"max_rebuffers_per_frame",
base::NumberToString(max_rebuffers_before_stop)}}});
enabled_features.push_back({features::kLiteVideo, params});
}
std::vector<base::Feature> disabled_features = {
......@@ -308,8 +312,26 @@ IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest,
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.
IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest, StopsThrottlingOnPlaybackSeek) {
IN_PROC_BROWSER_TEST_P(LiteVideoStopThrottlingOnPlaybackSeekBrowserTest,
StopsThrottlingOnPlaybackSeek) {
ukm::TestAutoSetUkmRecorder ukm_recorder;
TestMSEPlayback("bear-vp9.webm", "2000", "2000", false);
......@@ -324,14 +346,18 @@ IN_PROC_BROWSER_TEST_F(LiteVideoBrowserTest, StopsThrottlingOnPlaybackSeek) {
"document.querySelector('video').currentTime = 1"));
// Verify some responses were throttled and then throttling is stopped.
RetryForHistogramUntilCountReached(histogram_tester(),
"LiteVideo.HintsAgent.StopThrottling", 1);
if (should_disable_lite_videos_on_seek()) {
RetryForHistogramUntilCountReached(
histogram_tester(), "LiteVideo.HintsAgent.StopThrottling", 1);
}
EXPECT_GE(1U, histogram_tester()
.GetAllSamples("LiteVideo.URLLoader.ThrottleLatency")
.size());
EXPECT_EQ(1U, histogram_tester()
.GetAllSamples("LiteVideo.HintsAgent.StopThrottling")
.size());
EXPECT_EQ(should_disable_lite_videos_on_seek() ? 1U : 0U,
histogram_tester()
.GetAllSamples("LiteVideo.HintsAgent.StopThrottling")
.size());
EXPECT_GE(
1U, histogram_tester()
.GetAllSamples("LiteVideo.NavigationMetrics.FrameRebufferMapSize")
......
......@@ -140,5 +140,10 @@ int GetMaxRebuffersPerFrame() {
"max_rebuffers_per_frame", 1);
}
bool DisableLiteVideoOnMediaPlayerSeek() {
return GetFieldTrialParamByFeatureAsBool(
::features::kLiteVideo, "disable_on_media_player_seek", false);
}
} // namespace features
} // namespace lite_video
......@@ -79,6 +79,8 @@ bool IsLiteVideoNotAllowedForPageTransition(ui::PageTransition page_transition);
// should be stopped.
int GetMaxRebuffersPerFrame();
bool DisableLiteVideoOnMediaPlayerSeek();
} // namespace features
} // namespace lite_video
......
......@@ -278,6 +278,9 @@ void LiteVideoObserver::MediaBufferUnderflow(const content::MediaPlayerId& id) {
void LiteVideoObserver::MediaPlayerSeek(const content::MediaPlayerId& id) {
content::RenderFrameHost* render_frame_host = id.render_frame_host;
if (!lite_video::features::DisableLiteVideoOnMediaPlayerSeek())
return;
if (!render_frame_host || !render_frame_host->GetProcess())
return;
......
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