From aab443b709be482b746ffa47c3a04e69fade3e83 Mon Sep 17 00:00:00 2001
From: jdduke <jdduke@chromium.org>
Date: Tue, 9 Jun 2015 14:51:16 -0700
Subject: [PATCH] [Android] Use the UI thread for VSyncMonitorTest vsync
 requests

VSyncMonitor is not thread-safe, and is not intended to be. When a vsync
request is made from a non-UI thread, there is a race where both a
synthetic and frame-based callback can fire for the same request. Avoid
this by making sure VSyncMonitorTest only requests updates from the UI
thread, representative of production code behavior.

BUG=498007

Review URL: https://codereview.chromium.org/1166223003

Cr-Commit-Position: refs/heads/master@{#333580}
---
 .../chromium/chrome/browser/autofill/AutofillTest.java   |  2 +-
 .../org/chromium/content/browser/VSyncMonitorTest.java   | 16 ++++++++++++++--
 ui/android/java/src/org/chromium/ui/VSyncMonitor.java    |  5 ++++-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/chrome/android/javatests_shell/src/org/chromium/chrome/browser/autofill/AutofillTest.java b/chrome/android/javatests_shell/src/org/chromium/chrome/browser/autofill/AutofillTest.java
index c360ad13ca55..b1447bdbb4a8 100644
--- a/chrome/android/javatests_shell/src/org/chromium/chrome/browser/autofill/AutofillTest.java
+++ b/chrome/android/javatests_shell/src/org/chromium/chrome/browser/autofill/AutofillTest.java
@@ -45,13 +45,13 @@ public class AutofillTest extends ChromeShellTestBase {
         waitForActiveShellToBeDoneLoading();
 
         mMockAutofillCallback = new MockAutofillCallback();
-        mWindowAndroid = new ActivityWindowAndroid(activity);
         final ViewAndroidDelegate viewDelegate =
                 activity.getActiveContentViewCore().getViewAndroidDelegate();
 
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
             @Override
             public void run() {
+                mWindowAndroid = new ActivityWindowAndroid(activity);
                 mAutofillPopup = new AutofillPopup(activity, viewDelegate, mMockAutofillCallback);
                 mAutofillPopup.filterAndShow(new AutofillSuggestion[0], false);
                 mAutofillPopup.setAnchorRect(50, 500, 500, 50);
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 61b9dfeaa669..610ba0b716bc 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
@@ -43,6 +43,7 @@ public class VSyncMonitorTest extends InstrumentationTestCase {
 
         @Override
         public void onVSync(VSyncMonitor monitor, long vsyncTimeMicros) {
+            ThreadUtils.assertOnUiThread();
             mLastVSyncCpuTimeMillis = SystemClock.uptimeMillis();
             if (mPreviousVSyncTimeMicros == 0) {
                 mPreviousVSyncTimeMicros = vsyncTimeMicros;
@@ -81,6 +82,17 @@ public class VSyncMonitorTest extends InstrumentationTestCase {
         });
     }
 
+    // Vsync requests should be made on the same thread as that used to create the VSyncMonitor (the
+    // UI thread).
+    private void requestVSyncMonitorUpdate(final VSyncMonitor monitor) {
+        ThreadUtils.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                monitor.requestUpdate();
+            }
+        });
+    }
+
     // Check that the vsync period roughly matches the timestamps that the monitor generates.
     @MediumTest
     public void testVSyncPeriod() throws InterruptedException {
@@ -92,7 +104,7 @@ public class VSyncMonitorTest extends InstrumentationTestCase {
         assertTrue(reportedFramePeriod > 0);
 
         assertFalse(collector.isDone());
-        monitor.requestUpdate();
+        requestVSyncMonitorUpdate(monitor);
         collector.waitTillDone();
         assertTrue(collector.isDone());
 
@@ -122,7 +134,7 @@ public class VSyncMonitorTest extends InstrumentationTestCase {
         VSyncDataCollector collector = new VSyncDataCollector(1);
         VSyncMonitor monitor = createVSyncMonitor(collector);
 
-        monitor.requestUpdate();
+        requestVSyncMonitorUpdate(monitor);
         collector.waitTillDone();
         assertTrue(collector.isDone());
 
diff --git a/ui/android/java/src/org/chromium/ui/VSyncMonitor.java b/ui/android/java/src/org/chromium/ui/VSyncMonitor.java
index c0745013b4e6..039613568739 100644
--- a/ui/android/java/src/org/chromium/ui/VSyncMonitor.java
+++ b/ui/android/java/src/org/chromium/ui/VSyncMonitor.java
@@ -6,6 +6,7 @@ package org.chromium.ui;
 
 import android.content.Context;
 import android.os.Handler;
+import android.os.Looper;
 import android.view.Choreographer;
 import android.view.WindowManager;
 
@@ -108,10 +109,12 @@ public class VSyncMonitor {
     }
 
     /**
-     * Request to be notified of the closest display vsync events.
+     * Request to be notified of the closest display vsync events. This should
+     * always be called on the same thread used to create the VSyncMonitor.
      * Listener.onVSync() will be called soon after the upcoming vsync pulses.
      */
     public void requestUpdate() {
+        assert mHandler.getLooper() == Looper.myLooper();
         postCallback();
     }
 
-- 
2.11.4.GIT