Improve transition between opaque and translucent compositor views.

chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java

Index: chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
index 323781006ed24c7a9a8a9d55a05cfd984668e57a..8612f5255f2aab03243ba304be76a6f0d3d6723b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorView.java
@@ -1,4 +1,4 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2016 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.
 
@@ -9,6 +9,7 @@ import android.content.Context;
 import android.graphics.Color;
 import android.graphics.PixelFormat;
 import android.graphics.Rect;
+import android.graphics.drawable.Drawable;
 import android.os.Build;
 import android.view.Display;
 import android.view.MotionEvent;
@@ -17,9 +18,9 @@ import android.view.SurfaceHolder;
 import android.view.SurfaceView;
 import android.view.View;
 import android.view.WindowManager;
+import android.widget.FrameLayout;
 
 import org.chromium.base.CommandLine;
-import org.chromium.base.Log;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
@@ -43,8 +44,7 @@ import org.chromium.ui.resources.ResourceManager;
  * The is the {@link View} displaying the ui compositor results; including webpages and tabswitcher.
  */
 @JNINamespace("android")
-public class CompositorView
-        extends SurfaceView implements SurfaceHolder.Callback {
+public class CompositorView extends FrameLayout implements SurfaceHolder.Callback {
     private static final String TAG = "CompositorView";
     private static final long NANOSECONDS_PER_MILLISECOND = 1000000;
 
@@ -52,6 +52,19 @@ public class CompositorView
     private final Rect mCacheAppRect = new Rect();
     private final int[] mCacheViewPosition = new int[2];
 
+    private CompositorSurfaceManager mCompositorSurfaceManager;
+    private boolean mOverlayVideoEnabled;
+    private boolean mLowmMemoryDevice;
+    private boolean mAlwaysTranslucent;
+    private int mCurrentPixelFormat;
+
+    // Are we waiting to hide the outgoing surface until the foreground has something to display?
+    // If == 0, then no.  If > 0, then yes.  We'll hide when it transitions from one to zero.
+    private int mFramesUntilHideBackground;
+
+    // Cache of setWillNotDraw, which we keep up to date if we switch SVs.
+    private boolean mWillNotDraw;
+
     private long mNativeCompositorView;
     private final LayoutRenderHost mRenderHost;
     private boolean mEnableTabletTabStack;
@@ -74,12 +87,9 @@ public class CompositorView
     private int mSurfaceHeight;
     private boolean mPreloadedResources;
 
-    // The current SurfaceView pixel format. Defaults to OPAQUE.
-    private int mCurrentPixelFormat = PixelFormat.OPAQUE;
-
     /**
-     * Creates a {@link CompositorView}. This can be called only after the native library is
-     * properly loaded.
+     * Creates a {@link CompositorView}. This can be called only after the
+     * native library is properly loaded.
      * @param c        The Context to create this {@link CompositorView} in.
      * @param host     The renderer host owning this view.
      */
@@ -87,8 +97,8 @@ public class CompositorView
         super(c);
         mRenderHost = host;
         resetFlags();
+
         setVisibility(View.INVISIBLE);
-        setZOrderMediaOverlay(true);
     }
 
     /**
@@ -157,7 +167,7 @@ public class CompositorView
      * Should be called for cleanup when the CompositorView instance is no longer used.
      */
     public void shutDown() {
-        getHolder().removeCallback(this);
+        mCompositorSurfaceManager.shutDown();
         if (mNativeCompositorView != 0) nativeDestroy(mNativeCompositorView);
         mNativeCompositorView = 0;
     }
@@ -178,9 +188,27 @@ public class CompositorView
         mNativeCompositorView = nativeInit(lowMemDevice,
                 windowAndroid.getNativePointer(), layerTitleCache, tabContentManager);
 
-        assert !getHolder().getSurface().isValid()
-            : "Surface created before native library loaded.";
-        getHolder().addCallback(this);
+        // compositor_impl_android.cc will use 565 EGL surfaces if and only if  we're using a low
+        // memory device, and no alpha channel is desired.  Otherwise, it will use 8888.  Since
+        // SurfaceFlinger doesn't need the eOpaque flag to optimize out alpha blending during
+        // composition if the buffer has no alpha channel, we can avoid using the extra background
+        // surface (and the memory it requires) in the low memory case.  The output buffer will
+        // either have an alpha channel or not, depending on whether the compositor needs it.  We
+        // can keep the surface translucent all the times without worrying about the impact on power
+        // usage during SurfaceFlinger composition. We might also want to set |mAlwaysTranslucent|
+        // on non-low memory devices , if we are running on hardware that implements efficient alpha
+        // blending.
+        mAlwaysTranslucent = lowMemDevice;
+
+        mCompositorSurfaceManager = new CompositorSurfaceManager(this, this);
+        // In case the background drawable was set before now, update it.
+        mCompositorSurfaceManager.setBackgroundDrawable(getBackground());
+        mCompositorSurfaceManager.setWillNotDraw(mWillNotDraw);
+        // Make sure that the current pixel format does not match what we're requesting, so that we
+        // actually request a surface.
+        int initialFormat = getSurfacePixelFormat();
+        mCurrentPixelFormat = initialFormat + 1;

[aelias_OOO_until_Jul13, 2017/01/12 21:46:05]

This seems weird, I'd rather we define an invalid format value like -1 instead.

[liberato (no reviews please), 2017/01/19 18:12:34]

yeah -- no idea why i did that.  we have PixelFormat.UNKNOWN.

+        mCompositorSurfaceManager.requestSurface(initialFormat);
 
         // Cover the black surface before it has valid content.
         setBackgroundColor(Color.WHITE);
@@ -208,18 +236,52 @@ public class CompositorView
     }
 
     /**
+     * @see SurfaceView#getHolder
+     */
+    SurfaceHolder getHolder() {
+        return mCompositorSurfaceManager.getHolder();
+    }
+
+    /**
      * Enables/disables overlay video mode. Affects alpha blending on this view.
      * @param enabled Whether to enter or leave overlay video mode.
      */
     public void setOverlayVideoMode(boolean enabled) {
-        mCurrentPixelFormat = enabled ? PixelFormat.TRANSLUCENT : PixelFormat.OPAQUE;
-        getHolder().setFormat(mCurrentPixelFormat);
         nativeSetOverlayVideoMode(mNativeCompositorView, enabled);
+
+        mOverlayVideoEnabled = enabled;
+        int newFormat = getSurfacePixelFormat();
+        if (newFormat == mCurrentPixelFormat) {
+            // Don't bother to request a surface, but instead re-use this one.
+            if (mNativeCompositorView != 0) {
+                // Restart the compositor so that it knows about the new format.  Otherwise, nothing
+                // will update the EGL surface.
+                nativeSurfaceDestroyed(mNativeCompositorView);
+                nativeSurfaceCreated(mNativeCompositorView);
+            }
+            return;
+        }
+
+        // We're actually switching formats.
+        mCurrentPixelFormat = newFormat;
+        mCompositorSurfaceManager.requestSurface(newFormat);
+        // Note that we don't know if we'll get a surfaceCreated / surfaceDestoyed for this surface.
+        // We do know that if we do get one, then it will be for the surface that we just requested.
+        // However, they might be elided entirely if we switch fast enough that we never destroy the
+        // first surface before we request the other, then request the first back.  In that case,
+        // the compositor will be back in the right mode too, and it's fine to elide these.  Of
+        // course, we have to work whether it calls us back or not.
+    }
+
+    private int getSurfacePixelFormat() {
+        return (mOverlayVideoEnabled || mAlwaysTranslucent) ? PixelFormat.TRANSLUCENT
+                                                            : PixelFormat.OPAQUE;
     }
 
     @Override
     public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
         if (mNativeCompositorView == 0) return;
+
         nativeSurfaceChanged(mNativeCompositorView, format, width, height, holder.getSurface());
         mRenderHost.onPhysicalBackingSizeChanged(width, height);
         mSurfaceWidth = width;
@@ -229,13 +291,16 @@ public class CompositorView
     @Override
     public void surfaceCreated(SurfaceHolder holder) {
         if (mNativeCompositorView == 0) return;
+
         nativeSurfaceCreated(mNativeCompositorView);
+        mFramesUntilHideBackground = 2;
         mRenderHost.onSurfaceCreated();
     }
 
     @Override
     public void surfaceDestroyed(SurfaceHolder holder) {
         if (mNativeCompositorView == 0) return;
+
         nativeSurfaceDestroyed(mNativeCompositorView);
     }
 
@@ -264,29 +329,7 @@ public class CompositorView
      */
     @CalledByNative
     private void onJellyBeanSurfaceDisconnectWorkaround(boolean inOverlayMode) {
-        // There is a bug in JellyBean because of which we will not be able to
-        // reconnect to the existing Surface after we launch a new GPU process.
-        // We simply trick the JB Android code to allocate a new Surface.
-        // It does a strict comparison between the current format and the requested
-        // one, even if they are the same in practice. Furthermore, the format
-        // does not matter here since the producer-side EGL config overwrites it
-        // (but transparency might matter).
-        switch (mCurrentPixelFormat) {
-            case PixelFormat.OPAQUE:
-                mCurrentPixelFormat = PixelFormat.RGBA_8888;
-                break;
-            case PixelFormat.RGBA_8888:
-                mCurrentPixelFormat = inOverlayMode
-                        ? PixelFormat.TRANSLUCENT : PixelFormat.OPAQUE;
-                break;
-            case PixelFormat.TRANSLUCENT:
-                mCurrentPixelFormat = PixelFormat.RGBA_8888;
-                break;
-            default:
-                assert false;
-                Log.e(TAG, "Unknown current pixel format.");
-        }
-        getHolder().setFormat(mCurrentPixelFormat);
+        mCompositorSurfaceManager.recreateSurfaceForJellyBean();
     }
 
     /**
@@ -308,6 +351,20 @@ public class CompositorView
             }, mFramePresentationDelay);
         }
 
+        // If we're in the middle of a surface swap, then see if we've received a new frame yet for
+        // the new surface before hiding the outgoing surface.
+        if (mFramesUntilHideBackground > 1) {
+            // We need at least one more frame before we hide the outgoing surface.  Make sure that
+            // there will be a frame.
+            mFramesUntilHideBackground--;
+            requestRender();
+        } else if (mFramesUntilHideBackground == 1) {
+            // We can hide the outgoing surface, since the incoming one has a frame.  It's okay if
+            // we've don't have an unowned surface.
+            mFramesUntilHideBackground = 0;
+            mCompositorSurfaceManager.doneWithUnownedSurface();
+        }
+
         mRenderHost.onSwapBuffersCompleted(pendingSwapBuffersCount);
     }
 
@@ -351,6 +408,25 @@ public class CompositorView
         TraceEvent.end("CompositorView:finalizeLayers");
     }
 
+    @Override
+    public void setWillNotDraw(boolean willNotDraw) {
+        mWillNotDraw = willNotDraw;
+        if (mCompositorSurfaceManager != null) {
+            mCompositorSurfaceManager.setWillNotDraw(mWillNotDraw);
+        }
+    }
+
+    @Override
+    public void setBackgroundDrawable(Drawable background) {
+        // We override setBackgroundDrawable since that's the common entry point from all the
+        // setBackground* calls in View.  We still call to setBackground on the SurfaceView because
+        // SetBackgroundDrawable is depricated, and the semantics are the same I think.
+        super.setBackgroundDrawable(background);
+        if (mCompositorSurfaceManager != null) {
+            mCompositorSurfaceManager.setBackgroundDrawable(background);
+        }
+    }
+
     // Implemented in native
     private native long nativeInit(boolean lowMemDevice, long nativeWindowAndroid,
             LayerTitleCache layerTitleCache, TabContentManager tabContentManager);