summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorenne <enne@chromium.org>2016-03-14 15:58:35 -0700
committerCommit bot <commit-bot@chromium.org>2016-03-14 23:00:13 +0000
commit4a7c4f1f0ea5708d7a166a103d85d162ef3ef4c7 (patch)
tree556327d4ec2c3d82ef852a33c7bb100fa7e58997
parent56047cef021402a70701ca7c81897db542be731d (diff)
downloadchromium_src-4a7c4f1f0ea5708d7a166a103d85d162ef3ef4c7.zip
chromium_src-4a7c4f1f0ea5708d7a166a103d85d162ef3ef4c7.tar.gz
chromium_src-4a7c4f1f0ea5708d7a166a103d85d162ef3ef4c7.tar.bz2
Remove synthetic vsync from Android VsyncMonitor
This logic is from the ancient past and apparently is not needed anymore. This vsync can cause BeginFrames with frame times in the past to occur. As a part of testing https://codereview.chromium.org/1765723002, this caused AwContentsGarbageCollectionTest#testCreateAndGcManyTimes to crash because of this. It would enter a situation where a synthetic vsync would be sent that tried to predict the last vsync time, but the next time the choreographer sent a frame time it would be *before* the synthetic frame time. This causes assertions later on in the pipeline where it's assumed that all begin frames are monotonically increasing in frame time. These invalid begin frames used to be dropped silently by the begin frame source multiplexer, but that logic is removed in the above patch. R=skyostil@chromium.org,sievers@chromium.org Review URL: https://codereview.chromium.org/1789803003 Cr-Commit-Position: refs/heads/master@{#381105}
-rw-r--r--content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java17
-rw-r--r--ui/android/java/src/org/chromium/ui/VSyncMonitor.java39
2 files changed, 0 insertions, 56 deletions
diff --git a/content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java b/content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java
index 7b34461..65153f7 100644
--- a/content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java
+++ b/content/public/android/javatests/src/org/chromium/content/browser/VSyncMonitorTest.java
@@ -127,21 +127,4 @@ public class VSyncMonitorTest extends InstrumentationTestCase {
assertTrue(monitor.getVSyncPeriodInMicroseconds() < 1000000 / 30);
}
}
-
- @MediumTest
- public void testVSyncActivationFromIdle() throws InterruptedException {
- // Check that the vsync period roughly matches the timestamps that the monitor generates.
- VSyncDataCollector collector = new VSyncDataCollector(1);
- VSyncMonitor monitor = createVSyncMonitor(collector);
-
- requestVSyncMonitorUpdate(monitor);
- collector.waitTillDone();
- assertTrue(collector.isDone());
-
- long period = monitor.getVSyncPeriodInMicroseconds() / 1000;
- long delay = SystemClock.uptimeMillis() - collector.mLastVSyncCpuTimeMillis;
-
- // The VSync should have activated immediately instead of at the next real vsync.
- assertTrue(delay < period);
- }
}
diff --git a/ui/android/java/src/org/chromium/ui/VSyncMonitor.java b/ui/android/java/src/org/chromium/ui/VSyncMonitor.java
index 0396135..281ed14 100644
--- a/ui/android/java/src/org/chromium/ui/VSyncMonitor.java
+++ b/ui/android/java/src/org/chromium/ui/VSyncMonitor.java
@@ -51,8 +51,6 @@ public class VSyncMonitor {
// If the monitor is activated after having been idle, we synthesize the first vsync to reduce
// latency.
private final Handler mHandler = new Handler();
- private final Runnable mSyntheticVSyncRunnable;
- private long mLastVSyncCpuTimeNano;
/**
* Constructs a VSyncMonitor
@@ -73,7 +71,6 @@ public class VSyncMonitor {
@Override
public void doFrame(long frameTimeNanos) {
TraceEvent.begin("VSync");
- mHandler.removeCallbacks(mSyntheticVSyncRunnable);
if (useEstimatedRefreshPeriod && mConsecutiveVSync) {
// Display.getRefreshRate() is unreliable on some platforms.
// Adjust refresh period- initial value is based on Display.getRefreshRate()
@@ -88,16 +85,6 @@ public class VSyncMonitor {
TraceEvent.end("VSync");
}
};
- mSyntheticVSyncRunnable = new Runnable() {
- @Override
- public void run() {
- TraceEvent.begin("VSyncSynthetic");
- mChoreographer.removeFrameCallback(mVSyncFrameCallback);
- final long currentTime = getCurrentNanoTime();
- onVSyncCallback(estimateLastVSyncTime(currentTime), currentTime);
- TraceEvent.end("VSyncSynthetic");
- }
- };
mGoodStartingPointNano = getCurrentNanoTime();
}
@@ -137,7 +124,6 @@ public class VSyncMonitor {
assert mHaveRequestInFlight;
mInsideVSync = true;
mHaveRequestInFlight = false;
- mLastVSyncCpuTimeNano = currentTimeNanos;
try {
if (mListener != null) {
mListener.onVSync(this, frameTimeNanos / NANOSECONDS_PER_MICROSECOND);
@@ -151,31 +137,6 @@ public class VSyncMonitor {
if (mHaveRequestInFlight) return;
mHaveRequestInFlight = true;
mConsecutiveVSync = mInsideVSync;
- // There's no way to tell if we're currently in the scope of a
- // choregrapher frame callback, which might in turn allow us to honor
- // the vsync callback in the current frame. Thus, we eagerly post the
- // frame callback even when we post a synthetic frame callback. If the
- // frame callback is honored before the synthetic callback, we simply
- // remove the synthetic callback.
- postSyntheticVSyncIfNecessary();
mChoreographer.postFrameCallback(mVSyncFrameCallback);
}
-
- private void postSyntheticVSyncIfNecessary() {
- // TODO(jdduke): Consider removing synthetic vsyncs altogether if
- // they're found to be no longer necessary.
- final long currentTime = getCurrentNanoTime();
- // Only trigger a synthetic vsync if we've been idle for long enough and the upcoming real
- // vsync is more than half a frame away.
- if (currentTime - mLastVSyncCpuTimeNano < 2 * mRefreshPeriodNano) return;
- if (currentTime - estimateLastVSyncTime(currentTime) > mRefreshPeriodNano / 2) return;
- mHandler.post(mSyntheticVSyncRunnable);
- }
-
- private long estimateLastVSyncTime(long currentTime) {
- final long lastRefreshTime = mGoodStartingPointNano
- + ((currentTime - mGoodStartingPointNano) / mRefreshPeriodNano)
- * mRefreshPeriodNano;
- return lastRefreshTime;
- }
}