Android: Consolidate and simplify VSync logic

content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java

Index: content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
index 2e591a8c8e3b12c7aefefe39ba8ffb097c5d810f..550b738e086a0c8ae0daa663b14d65b4061a3103 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewRenderView.java
@@ -9,7 +9,6 @@ import android.graphics.Bitmap;
 import android.graphics.Canvas;
 import android.graphics.Color;
 import android.graphics.PixelFormat;
-import android.os.Build;
 import android.os.Handler;
 import android.view.Surface;
 import android.view.SurfaceHolder;
@@ -18,8 +17,6 @@ import android.widget.FrameLayout;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
-import org.chromium.base.ObserverList;
-import org.chromium.base.ObserverList.RewindableIterator;
 import org.chromium.base.TraceEvent;
 import org.chromium.ui.base.WindowAndroid;
 
@@ -30,7 +27,7 @@ import org.chromium.ui.base.WindowAndroid;
  * Note that only one ContentViewCore can be shown at a time.
  */
 @JNINamespace("content")
-public class ContentViewRenderView extends FrameLayout {
+public class ContentViewRenderView extends FrameLayout implements WindowAndroid.VSyncClient {

[David Trainor- moved to gerrit, 2014/04/16 06:18:20]

Indent +8

[no sievers, 2014/04/25 22:23:33]

there's actually no line break here :)

     private static final int MAX_SWAP_BUFFER_COUNT = 2;
 
     // The native side of this object.
@@ -38,7 +35,7 @@ public class ContentViewRenderView extends FrameLayout {
     private final SurfaceHolder.Callback mSurfaceCallback;
 
     private final SurfaceView mSurfaceView;
-    private final VSyncAdapter mVSyncAdapter;
+    private final WindowAndroid mRootWindow;
 
     private int mPendingRenders;
     private int mPendingSwapBuffers;
@@ -64,6 +61,8 @@ public class ContentViewRenderView extends FrameLayout {
         mNativeContentViewRenderView = nativeInit(rootWindow.getNativePointer());
         assert mNativeContentViewRenderView != 0;
 
+        mRootWindow = rootWindow;
+        rootWindow.setVSyncClient(this);
         mSurfaceView = createSurfaceView(getContext());
         mSurfaceView.setZOrderMediaOverlay(true);
         mSurfaceCallback = new SurfaceHolder.Callback() {
@@ -99,86 +98,23 @@ public class ContentViewRenderView extends FrameLayout {
         };
         mSurfaceView.getHolder().addCallback(mSurfaceCallback);
 
-        mVSyncAdapter = new VSyncAdapter(getContext());
         addView(mSurfaceView,
                 new FrameLayout.LayoutParams(
                         FrameLayout.LayoutParams.MATCH_PARENT,
                         FrameLayout.LayoutParams.MATCH_PARENT));
     }
 
-    private class VSyncAdapter implements VSyncManager.Provider, VSyncMonitor.Listener {
-        private final VSyncMonitor mVSyncMonitor;
-        private boolean mVSyncNotificationEnabled;
-        private VSyncManager.Listener mVSyncListener;
-        private final ObserverList<VSyncManager.Listener> mCurrentVSyncListeners;
-        private final RewindableIterator<VSyncManager.Listener> mCurrentVSyncListenersIterator;
-
-        // The VSyncMonitor gives the timebase for the actual vsync, but we don't want render until
-        // we have had a chance for input events to propagate to the compositor thread. This takes
-        // 3 ms typically, so we adjust the vsync timestamps forward by a bit to give input events a
-        // chance to arrive.
-        private static final long INPUT_EVENT_LAG_FROM_VSYNC_MICROSECONDS = 3200;

[Sami, 2014/04/16 17:26:54]

Should probably verify with a trace, but does the fact that we now buffer input
events until vsync mean that we no longer need this hack for ICS?

[no sievers, 2014/04/25 22:23:33]

I thought that the deadline scheduler and retroactive BeginFrame eliminate the
need for this. But now that I look again, I'm actually not sure what it does :)

Note that this is used on JB and up in the code below but only when there are no
vsync subscribers (?).

You introduced it in 'Change-Id: I3df344b8a0fecda7c9c9a49be892875c01afeea8'
(patch set 2).

-
-        VSyncAdapter(Context context) {
-            mVSyncMonitor = new VSyncMonitor(context, this);
-            mCurrentVSyncListeners = new ObserverList<VSyncManager.Listener>();
-            mCurrentVSyncListenersIterator = mCurrentVSyncListeners.rewindableIterator();
-        }
-
-        @Override
-        public void onVSync(VSyncMonitor monitor, long vsyncTimeMicros) {
-            if (mNeedToRender) {
-                if (mPendingSwapBuffers + mPendingRenders <= MAX_SWAP_BUFFER_COUNT) {
-                    mNeedToRender = false;
-                    mPendingRenders++;
-                    render();
-                } else {
-                    TraceEvent.instant("ContentViewRenderView:bail");
-                }
-            }
-
-            if (mVSyncListener != null) {
-                if (mVSyncNotificationEnabled) {
-                    for (mCurrentVSyncListenersIterator.rewind();
-                            mCurrentVSyncListenersIterator.hasNext();) {
-                        mCurrentVSyncListenersIterator.next().onVSync(vsyncTimeMicros);
-                    }
-                    mVSyncMonitor.requestUpdate();
-                } else {
-                    // Compensate for input event lag. Input events are delivered immediately on
-                    // pre-JB releases, so this adjustment is only done for later versions.
-                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
-                        vsyncTimeMicros += INPUT_EVENT_LAG_FROM_VSYNC_MICROSECONDS;
-                    }
-                    mVSyncListener.updateVSync(vsyncTimeMicros,
-                            mVSyncMonitor.getVSyncPeriodInMicroseconds());
-                }
-            }
-        }
-
-        @Override
-        public void registerVSyncListener(VSyncManager.Listener listener) {
-            if (!mVSyncNotificationEnabled) mVSyncMonitor.requestUpdate();
-            mCurrentVSyncListeners.addObserver(listener);
-            mVSyncNotificationEnabled = true;
-        }
-
-        @Override
-        public void unregisterVSyncListener(VSyncManager.Listener listener) {
-            mCurrentVSyncListeners.removeObserver(listener);
-            if (mCurrentVSyncListeners.isEmpty()) {
-                mVSyncNotificationEnabled = false;
-            }
-        }
-
-        void setVSyncListener(VSyncManager.Listener listener) {
-            mVSyncListener = listener;
-            if (mVSyncListener != null) mVSyncMonitor.requestUpdate();
-        }
-
-        void requestUpdate() {
-            mVSyncMonitor.requestUpdate();
+    @Override
+    public void onVSync(long vsyncTimeMicros) {
+      if (mNeedToRender) {
+        if (mPendingSwapBuffers + mPendingRenders <= MAX_SWAP_BUFFER_COUNT) {
+          mNeedToRender = false;
+          mPendingRenders++;
+          render();
+        } else {
+          TraceEvent.instant("ContentViewRenderView:bail");
         }
+      }
     }
 
     /**
@@ -198,6 +134,7 @@ public class ContentViewRenderView extends FrameLayout {
      * native resource can be freed.
      */
     public void destroy() {
+        mRootWindow.setVSyncClient(null);
         mSurfaceView.getHolder().removeCallback(mSurfaceCallback);
         nativeDestroy(mNativeContentViewRenderView);
         mNativeContentViewRenderView = 0;
@@ -219,7 +156,6 @@ public class ContentViewRenderView extends FrameLayout {
 
         if (mContentViewCore != null) {
             mContentViewCore.onPhysicalBackingSizeChanged(getWidth(), getHeight());
-            mVSyncAdapter.setVSyncListener(mContentViewCore.getVSyncListener(mVSyncAdapter));
             nativeSetCurrentContentViewCore(mNativeContentViewRenderView,
                                             mContentViewCore.getNativeContentViewCore());
         } else {
@@ -291,12 +227,11 @@ public class ContentViewRenderView extends FrameLayout {
             } else {
                 post(mRenderRunnable);
             }
-            mVSyncAdapter.requestUpdate();
         } else if (mPendingRenders <= 0) {
             assert mPendingRenders == 0;
             TraceEvent.instant("requestRender:later");
             mNeedToRender = true;
-            mVSyncAdapter.requestUpdate();
+            mRootWindow.requestVSyncUpdate();
         }
     }