CrOS: Look for consistency between successive vsync intervals.

Currently we report computed vsync intervals based on if they are in
an expected range and silently drop otherwise. The disadvantage with this
is that we're probably reporting noise caused by configuration changes
(monitor reconfiguration or moving a window between monitors). We may
also be papering over driver bugs.

Instead, we look for consistency between successive computed intervals
as a way of validating that there hasn't been a configuration change. In
this case, we can be more aggressive in error reporting to find driver bugs
(crash instead of silently drop).

BUG=221701


Review URL: https://chromiumcodereview.appspot.com/13861016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193386 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 47 insertions, 18 deletions

diff --git a/ui/gl/vsync_provider.cc b/ui/gl/vsync_provider.cc
index e117647..91d64e0 100644
--- a/ui/gl/vsync_provider.cc
+++ b/ui/gl/vsync_provider.cc
@@ -4,6 +4,8 @@
 
 #include "ui/gl/vsync_provider.h"
 
+#include <math.h>
+
 #include "base/logging.h"
 #include "base/time.h"
 
@@ -14,6 +16,11 @@ namespace {
 const int64 kMinVsyncIntervalUs = base::Time::kMicrosecondsPerSecond / 400;
 const int64 kMaxVsyncIntervalUs = base::Time::kMicrosecondsPerSecond / 10;
 
+// How much noise we'll tolerate between successive computed intervals before
+// we think the latest computed interval is invalid (noisey due to
+// monitor configuration change, moving a window between monitors, etc.).
+const double kRelativeIntervalDifferenceThreshold = 0.05;
+
 }  // namespace
 
 namespace gfx {
@@ -94,32 +101,46 @@ void SyncControlVSyncProvider::GetVSyncParameters(
 
   timebase = base::TimeTicks::FromInternalValue(system_time);
 
+  // Only need the previous calculated interval for our filtering.
+  while (last_computed_intervals_.size() > 1)
+    last_computed_intervals_.pop();
+
   int32 numerator, denominator;
-  base::TimeDelta new_interval;
   if (GetMscRate(&numerator, &denominator)) {
-    new_interval =
-        base::TimeDelta::FromSeconds(denominator) / numerator;
+    last_computed_intervals_.push(
+        base::TimeDelta::FromSeconds(denominator) / numerator);
   } else if (!last_timebase_.is_null()) {
     base::TimeDelta timebase_diff = timebase - last_timebase_;
     uint64 counter_diff = media_stream_counter -
         last_media_stream_counter_;
     if (counter_diff > 0 && timebase > last_timebase_)
-      new_interval = timebase_diff / counter_diff;
+      last_computed_intervals_.push(timebase_diff / counter_diff);
   }
-  if (new_interval.InMicroseconds() < kMinVsyncIntervalUs ||
-      new_interval.InMicroseconds() > kMaxVsyncIntervalUs) {
-    LOG(ERROR) << "Calculated bogus refresh interval of "
-               << new_interval.InMicroseconds() << " us. "
-               << "Last time base of "
-               << last_timebase_.ToInternalValue() << " us. "
-               << "Current time base of "
-               << timebase.ToInternalValue() << " us. "
-               << "Last media stream count of "
-               << last_media_stream_counter_ << ". "
-               << "Current media stream count of "
-               << media_stream_counter << ".";
-  } else {
-    last_good_interval_ = new_interval;
+
+  if (last_computed_intervals_.size() == 2) {
+    const base::TimeDelta& old_interval = last_computed_intervals_.front();
+    const base::TimeDelta& new_interval = last_computed_intervals_.back();
+
+    double relative_change =
+        fabs(old_interval.InMillisecondsF() - new_interval.InMillisecondsF()) /
+        new_interval.InMillisecondsF();
+    if (relative_change < kRelativeIntervalDifferenceThreshold) {
+      if (new_interval.InMicroseconds() < kMinVsyncIntervalUs ||
+          new_interval.InMicroseconds() > kMaxVsyncIntervalUs) {
+        LOG(FATAL) << "Calculated bogus refresh interval of "
+                   << new_interval.InMicroseconds() << " us. "
+                   << "Last time base of "
+                   << last_timebase_.ToInternalValue() << " us. "
+                   << "Current time base of "
+                   << timebase.ToInternalValue() << " us. "
+                   << "Last media stream count of "
+                   << last_media_stream_counter_ << ". "
+                   << "Current media stream count of "
+                   << media_stream_counter << ".";
+      } else {
+        last_good_interval_ = new_interval;
+      }
+    }
   }
 
   last_timebase_ = timebase;
diff --git a/ui/gl/vsync_provider.h b/ui/gl/vsync_provider.h
index 5c7b2a4..bec065e 100644
--- a/ui/gl/vsync_provider.h
+++ b/ui/gl/vsync_provider.h
@@ -5,6 +5,8 @@
 #ifndef UI_GL_VSYNC_PROVIDER_H_
 #define UI_GL_VSYNC_PROVIDER_H_
 
+#include <queue>
+
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/time.h"
@@ -54,6 +56,12 @@ class SyncControlVSyncProvider : public VSyncProvider {
   uint64 last_media_stream_counter_;
   base::TimeDelta last_good_interval_;
 
+  // A short history of the last few computed intervals.
+  // We use this to filter out the noise in the computation resulting
+  // from configuration change (monitor reconfiguration, moving windows
+  // between monitors, suspend and resume, etc.).
+  std::queue<base::TimeDelta> last_computed_intervals_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncControlVSyncProvider);
 };