Don't underflow for background rendering. Ensure accurate counts.

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 1118 matching lines @@
     // Report the amount of time it took to leave the underflow state. Don't
     // bother to report this for MSE playbacks since it's out of our control.
     if (underflow_timer_ && data_source_) {
       UMA_HISTOGRAM_TIMES("Media.UnderflowDuration",
                           underflow_timer_->Elapsed());
       underflow_timer_.reset();
     }
   } else {
     // Buffering has underflowed.
     DCHECK_EQ(state, BUFFERING_HAVE_NOTHING);
+
+    // Report the number of times we've entered the underflow state. Only report
+    // for src= playback since for MSE it's out of our control. Ensure we only
+    // report the value when transitioning from HAVE_ENOUGH to HAVE_NOTHING.
+    if (data_source_ &&
+        ready_state_ == WebMediaPlayer::ReadyStateHaveEnoughData) {

[chcunningham, 2016/09/09 21:27:05]

I think this is fine, but I'm not sure it will have much impact. Looking at the
VideoRenderer guards, we only fire the event to transition to HAVE_NOTHING when
we are currently in HAVE_ENOUGH. Just want to be sure I'm not missing something.

[DaleCurtis, 2016/09/09 21:51:41]

Correct, that was my interpretation, but I wasn't able to verify it 100%.

+      UMA_HISTOGRAM_COUNTS_100("Media.UnderflowCount", ++underflow_count_);
+      underflow_timer_.reset(new base::ElapsedTimer());
+    }
+
     // It shouldn't be possible to underflow if we've not advanced past
     // HAVE_CURRENT_DATA.
     DCHECK_GT(highest_ready_state_, WebMediaPlayer::ReadyStateHaveCurrentData);
     SetReadyState(WebMediaPlayer::ReadyStateHaveCurrentData);
-
-    // Report the number of times we've entered the underflow state. Only report
-    // for src= playback since for MSE it's out of our control.
-    if (data_source_) {
-      UMA_HISTOGRAM_COUNTS_100("Media.UnderflowCount", ++underflow_count_);
-      underflow_timer_.reset(new base::ElapsedTimer());
-    }
   }
 
   UpdatePlayState();
 }
 
 void WebMediaPlayerImpl::OnDurationChange() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   // TODO(sandersd): We should call delegate_->DidPlay() with the new duration,
   // especially if it changed from  <5s to >5s.
@@ skipping 653 matching lines @@
       pipeline_metadata_.natural_size,
       base::Bind(&GetCurrentTimeInternal, this)));
   watch_time_reporter_->OnVolumeChange(volume_);
   if (delegate_ && delegate_->IsHidden())
     watch_time_reporter_->OnHidden();
   else
     watch_time_reporter_->OnShown();
 }
 
 }  // namespace media