media: Report initial video height to UMA

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 172541b83589e380d6e5e3fc661da93561247df7..8384a527c1c289888d2e75f20d6f65c9f910b789 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -1406,20 +1406,18 @@ void WebMediaPlayerImpl::OnVideoNaturalSizeChange(const gfx::Size& size) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK_NE(ready_state_, WebMediaPlayer::kReadyStateHaveNothing);
 
+  TRACE_EVENT0("media", "WebMediaPlayerImpl::OnNaturalSizeChanged");
+
   // The input |size| is from the decoded video frame, which is the original
   // natural size and need to be rotated accordingly.
   gfx::Size rotated_size =
       GetRotatedVideoSize(pipeline_metadata_.video_rotation, size);
 
-  if (rotated_size == pipeline_metadata_.natural_size)
-    return;
-
-  TRACE_EVENT0("media", "WebMediaPlayerImpl::OnNaturalSizeChanged");
-  media_log_->AddEvent(media_log_->CreateVideoSizeSetEvent(
-      rotated_size.width(), rotated_size.height()));
+  RecordVideoNaturalSize(rotated_size);
 
-  if (overlay_enabled_ && surface_manager_)
-    surface_manager_->NaturalSizeChanged(rotated_size);
+  gfx::Size old_size = pipeline_metadata_.natural_size;
+  if (rotated_size == old_size)
+    return;
 
   pipeline_metadata_.natural_size = rotated_size;
 
@@ -1428,6 +1426,9 @@ void WebMediaPlayerImpl::OnVideoNaturalSizeChange(const gfx::Size& size) {
   if (!watch_time_reporter_->IsSizeLargeEnoughToReportWatchTime())
     CreateWatchTimeReporter();
 
+  if (overlay_enabled_ && surface_manager_)
+    surface_manager_->NaturalSizeChanged(rotated_size);
+
   client_->SizeChanged();
 
   if (observer_)
@@ -2365,4 +2366,32 @@ void WebMediaPlayerImpl::RecordUnderflowDuration(base::TimeDelta duration) {
     UMA_HISTOGRAM_TIMES("Media.UnderflowDuration.MSE", duration);
 }
 
+#define UMA_HISTOGRAM_VIDEO_HEIGHT(name, sample) \
+  UMA_HISTOGRAM_CUSTOM_COUNTS(name, sample, 100, 10000, 50)
+
+void WebMediaPlayerImpl::RecordVideoNaturalSize(const gfx::Size& natural_size) {
+  // Always report video natural size to MediaLog.
+  media_log_->AddEvent(media_log_->CreateVideoSizeSetEvent(
+      natural_size.width(), natural_size.height()));
+
+  if (initial_video_height_recorded_)
+    return;
+
+  initial_video_height_recorded_ = true;
+
+  int height = natural_size.height();
+
+  if (load_type_ == kLoadTypeURL)
+    UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.SRC", height);
+  else if (load_type_ == kLoadTypeMediaSource)
+    UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.MSE", height);
+
+  if (is_encrypted_)
+    UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.EME", height);
+
+  UMA_HISTOGRAM_VIDEO_HEIGHT("Media.VideoHeight.Initial.All", height);
+}
+
+#undef UMA_HISTOGRAM_VIDEO_HEIGHT

[Ilya Sherman, 2017/04/19 05:52:45]

Optional nit: It seems a bit weird to #undef in an implementation file, since no
other files should ever include this file.  But, you're welcome to keep it if
you really like it.

[xhwang, 2017/04/19 06:03:50]

Thanks for catching the weirdness. I am okay either way. But Dale suggested this
so I'll just keep it.

+
 }  // namespace media