 Chromium Code Reviews
 Chromium Code Reviews Issue 2856783002:
  Autoplay time metric  (Closed)
    
  
    Issue 2856783002:
  Autoplay time metric  (Closed) 
  | Index: third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp | 
| diff --git a/third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp b/third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp | 
| index 410dcc50e41ceff3d8ab904d199a1d7c02fe58e0..f87dd197fd788b88831f8e3efb6f564a5d9eb26b 100644 | 
| --- a/third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp | 
| +++ b/third_party/WebKit/Source/core/html/media/AutoplayUmaHelper.cpp | 
| @@ -21,6 +21,8 @@ namespace { | 
| const int32_t kMaxOffscreenDurationUmaMS = 60 * 60 * 1000; | 
| const int32_t kOffscreenDurationUmaBucketCount = 50; | 
| +const int32_t kMaxWaitTimeUmaMS = 30 * 1000; | 
| +const int32_t kWaitTimeBucketCount = 50; | 
| } // namespace | 
| @@ -36,7 +38,8 @@ AutoplayUmaHelper::AutoplayUmaHelper(HTMLMediaElement* element) | 
| muted_video_autoplay_offscreen_start_time_ms_(0), | 
| muted_video_autoplay_offscreen_duration_ms_(0), | 
| is_visible_(false), | 
| - muted_video_offscreen_duration_visibility_observer_(nullptr) {} | 
| + muted_video_offscreen_duration_visibility_observer_(nullptr), | 
| + load_start_time_ms_(0.0) {} | 
| AutoplayUmaHelper::~AutoplayUmaHelper() = default; | 
| @@ -44,7 +47,17 @@ bool AutoplayUmaHelper::operator==(const EventListener& other) const { | 
| return this == &other; | 
| } | 
| +void AutoplayUmaHelper::OnLoadStarted() { | 
| 
DaleCurtis
2017/05/02 21:05:28
Probably should take the type. We only care about
 
hubbe
2017/05/02 22:33:26
Done.
 | 
| + load_start_time_ms_ = MonotonicallyIncreasingTimeMS(); | 
| +} | 
| + | 
| void AutoplayUmaHelper::OnAutoplayInitiated(AutoplaySource source) { | 
| + int32_t autoplay_wait_time_ms = -1; | 
| + if (load_start_time_ms_ != 0.0) { | 
| + autoplay_wait_time_ms = static_cast<int32_t>( | 
| + std::min<int64_t>(MonotonicallyIncreasingTimeMS() - load_start_time_ms_, | 
| 
DaleCurtis
2017/05/02 21:05:28
Clamp to kMaxWaitTimeUmaMS?
 
hubbe
2017/05/02 22:33:26
Doesn't the histogram already do that?
 
hubbe
2017/05/03 20:37:24
Checked, it does, here: https://cs.chromium.org/ch
 | 
| + std::numeric_limits<int32_t>::max())); | 
| + } | 
| DEFINE_STATIC_LOCAL(EnumerationHistogram, video_histogram, | 
| ("Media.Video.Autoplay", | 
| static_cast<int>(AutoplaySource::kNumberOfUmaSources))); | 
| @@ -58,6 +71,19 @@ void AutoplayUmaHelper::OnAutoplayInitiated(AutoplaySource source) { | 
| EnumerationHistogram, blocked_muted_video_histogram, | 
| ("Media.Video.Autoplay.Muted.Blocked", kAutoplayBlockedReasonMax)); | 
| + DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_video_attrib_histogram, | 
| 
DaleCurtis
2017/05/02 21:05:28
I don't think we really care about the difference
 
Zhiqiang Zhang (Slow)
2017/05/02 21:21:22
I think the experiment would only affect autoplay
 
DaleCurtis
2017/05/02 21:36:38
Ah, I didn't realize the behavior was different. H
 
hubbe
2017/05/02 22:33:26
The thing we're changing actually changes the read
 | 
| + ("Media.Video.Autoplay.Video.Attribute.WaitTimeMS", 1, | 
| + kMaxWaitTimeUmaMS, kWaitTimeBucketCount)); | 
| + DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_audio_attrib_histogram, | 
| + ("Media.Video.Autoplay.Audio.Attribute.WaitTimeMS", 1, | 
| + kMaxWaitTimeUmaMS, kWaitTimeBucketCount)); | 
| + DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_video_play_histogram, | 
| + ("Media.Video.Autoplay.Video.PlayMethod.WaitTimeMS", 1, | 
| + kMaxWaitTimeUmaMS, kWaitTimeBucketCount)); | 
| + DEFINE_STATIC_LOCAL(CustomCountHistogram, wait_time_audio_play_histogram, | 
| + ("Media.Video.Autoplay.Audio.PlayMethod.WaitTimeMS", 1, | 
| + kMaxWaitTimeUmaMS, kWaitTimeBucketCount)); | 
| + | 
| // Autoplay already initiated | 
| if (sources_.count(source)) | 
| return; | 
| @@ -69,8 +95,22 @@ void AutoplayUmaHelper::OnAutoplayInitiated(AutoplaySource source) { | 
| video_histogram.Count(static_cast<int>(source)); | 
| if (element_->muted()) | 
| muted_video_histogram.Count(static_cast<int>(source)); | 
| + if (autoplay_wait_time_ms >= 0) { | 
| + if (source == AutoplaySource::kAttribute) { | 
| + wait_time_video_attrib_histogram.Count(autoplay_wait_time_ms); | 
| + } else if (source == AutoplaySource::kMethod) { | 
| + wait_time_video_play_histogram.Count(autoplay_wait_time_ms); | 
| + } | 
| + } | 
| } else { | 
| audio_histogram.Count(static_cast<int>(source)); | 
| + if (autoplay_wait_time_ms >= 0) { | 
| + if (source == AutoplaySource::kAttribute) { | 
| + wait_time_audio_attrib_histogram.Count(autoplay_wait_time_ms); | 
| + } else if (source == AutoplaySource::kMethod) { | 
| + wait_time_audio_play_histogram.Count(autoplay_wait_time_ms); | 
| + } | 
| + } | 
| } | 
| // Record dual source. |