Improve transition between opaque and translucent compositor views.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java
new file mode 100644
index 0000000000000000000000000000000000000000..1e660210a88fa24f9c6cbd63aa7be606db6e2cd8
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManager.java
@@ -0,0 +1,318 @@
+// Copyright 2017 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.
+
+package org.chromium.chrome.browser.compositor;
+
+import android.content.Context;
+import android.graphics.PixelFormat;
+import android.graphics.drawable.Drawable;
+import android.os.Build;
+import android.view.SurfaceHolder;
+import android.view.SurfaceView;
+import android.view.View;
+import android.view.ViewGroup;
+import android.widget.FrameLayout;
+
+/**
+ * Manage multiple SurfaceViews for the compositor, so that transitions between
+ * surfaces with and without an alpha channel can be visually smooth.
+ *
+ * This class allows a client to request a 'translucent' or 'opaque' surface, and we will signal via
+ * SurfaceHolder.Callback when it's ready.  We guarantee that the client will receive surfaceCreated
+ * / surfaceDestroyed only for a surface that represents the most recently requested PixelFormat.
+ *
+ * Internally, we maintain two SurfaceViews, since calling setFormat() to change the PixelFormat
+ * results in a visual glitch as the surface is torn down.  crbug.com/679902
+ *
+ * The client has the responsibility to call doneWithUnownedSurface() at some point between when we
+ * call back its surfaceCreated, when it is safe for us to hide the SurfaceView with the wrong
+ * format.
+ *
+ * We do not guarantee that the surface will always actually have that format, though, if we're set
+ * up to always provide a translucent surface.  This helps low-memory devices, but not requiring two
+ * SurfaceViews.  However, it does require that the compositor use the EGL config to turn off alpha
+ * blending.  For low memory devices, this already happens, since it selects between 565 and 8888
+ * EGL configs.  In this case, we will still provide synthetic surfaceCreated / surfaceDestroyed
+ * messages to the client, even though the underlying surface isn't changing.
+ *
+ * The full design doc is at https://goo.gl/aAmQzR .
+ */
+class CompositorSurfaceManager implements SurfaceHolder.Callback {
+    private static class SurfaceState {
+        public SurfaceView surfaceView;
+
+        // Have we started destroying |surfaceView|, but haven't been notified of it yet?
+        public boolean destroyPending;
+
+        public SurfaceState(Context context, int format, SurfaceHolder.Callback callback) {
+            surfaceView = new SurfaceView(context);
+            surfaceView.setZOrderMediaOverlay(true);
+            surfaceView.setVisibility(View.VISIBLE);
+            surfaceHolder().setFormat(format);
+            surfaceHolder().addCallback(callback);
+        }
+
+        public SurfaceHolder surfaceHolder() {
+            return surfaceView.getHolder();
+        }
+
+        public boolean isValid() {
+            return surfaceHolder().getSurface().isValid();
+        }
+    }
+
+    // SurfaceView with a translucent PixelFormat.
+    private SurfaceState mTranslucent;

[David Trainor- moved to gerrit, 2017/01/19 22:08:22]

Should these two be final?

[liberato (no reviews please), 2017/01/20 17:50:46]

Done.

+
+    // SurfaceView with an opaque PixelFormat.
+    private SurfaceState mOpaque;
+
+    // Surface that we last gave to the client with surfaceCreated.  Cleared when we call
+    // surfaceDestroyed.
+    private SurfaceState mOwnedByClient;
+
+    // Surface that was most recently requested by the client.
+    private SurfaceState mRequestedByClient;
+
+    // Client that we notify about surface change events.
+    private SurfaceHolder.Callback mClient;
+
+    // View to which we'll attach the SurfaceView.
+    private final ViewGroup mParentView;
+
+    public CompositorSurfaceManager(ViewGroup parentView, SurfaceHolder.Callback client) {
+        mParentView = parentView;
+        mClient = client;
+
+        mTranslucent = new SurfaceState(parentView.getContext(), PixelFormat.TRANSLUCENT, this);
+        mOpaque = new SurfaceState(mParentView.getContext(), PixelFormat.OPAQUE, this);
+    }
+
+    /**
+     * Turn off everything.
+     */
+    public void shutDown() {
+        mTranslucent.surfaceHolder().removeCallback(this);
+        if (mOpaque != null) mOpaque.surfaceHolder().removeCallback(this);

[David Trainor- moved to gerrit, 2017/01/19 22:08:22]

It looks like mOpaque always gets built now.  Could these null checks be
removed?

[liberato (no reviews please), 2017/01/20 17:50:46]

Done.

+    }
+
+    /**
+     * Called by the client to request a surface.  Once called, we guarantee that the next call to
+     * onSurfaceAvailable will match the most recent value of |format|.  If the surface is already
+     * available for use, then we'll elide the created callback.  Note that |format| must be either
+     * OPAQUE or TRANSLUCENT.
+     */
+    public void requestSurface(int format) {
+        mRequestedByClient = (format == PixelFormat.TRANSLUCENT) ? mTranslucent : mOpaque;
+
+        // Is the requested surface ready?  "Ready" means that it's valid, and that we don't have a
+        // destroy in-flight for it.
+        if (mRequestedByClient.isValid() && !mRequestedByClient.destroyPending) {
+            // The client has requested a surface that's already valid and isn't being torn down.
+            // It might be requesting the same surface it already has, or it might have toggled
+            // back to one it had before without notifying doneWithUnownedSurface.  In either case,
+            // we elide the surface created / destroyed callbacks.  We might want to return whether
+            // we've elided them, but CompositorView doesn't care.
+            mOwnedByClient = mRequestedByClient;
+            return;
+        }
+
+        // Surface is not valid, or it'll be destroyed soon.  If it's not valid now, then start
+        // construction.  Once it's ready, we'll signal the client if it still wants this surface.
+        // If destruction is pending, then we must wait for it to complete.  When we're notified
+        // that it is destroyed, we'll re-start construction if the client still wants this surface.
+        // Note that we could send a surfaceDestroyed for the owned surface, if there is one, but we
+        // defer it until later so that the compositor can still use it.
+        if (!mRequestedByClient.isValid()) startSurfaceConstruction(mRequestedByClient);
+    }
+
+    /**
+     * Called to notify us that the client no longer needs the surface that it doesn't own.  This
+     * tells us that we may destroy it.  Note that it's okay if it never had an unowned surface.
+     */
+    public void doneWithUnownedSurface() {
+        if (mOwnedByClient == null) return;
+
+        SurfaceState unowned = (mOwnedByClient == mTranslucent) ? mOpaque : mTranslucent;
+
+        if (mRequestedByClient == unowned) {
+            // Client is giving us back a surface that it's since requested but hasn't gotten yet.
+            // Do nothing.  It will be notified when the new surface is ready, and it can call us
+            // again.
+            return;
+        }
+
+        // Start destruction of this surface.
+        postSurfaceDestruction(unowned);
+    }
+
+    /**
+     * Return the currently owned SurfaceHolder, if any.
+     */
+    public SurfaceHolder getHolder() {
+        return mOwnedByClient != null ? mOwnedByClient.surfaceHolder() : null;
+    }
+
+    /**
+     * Destroy and re-create the surface.  Useful for a JB workaround needed by CompositorView.
+     */
+    public void recreateSurfaceForJellyBean() {
+        assert Build.VERSION.SDK_INT <= Build.VERSION_CODES.JELLY_BEAN_MR2;
+
+        // If they don't have a surface, then they'll get a new one anyway.
+        if (mOwnedByClient == null) return;
+
+        // Notify the client that it no longer owns this surface, then destroy it.  When destruction
+        // completes, we will recreate it automatically, since it will look like the client since
+        // re-requested it.  That's why we send surfaceDestroyed here rather than letting our
+        // surfaceDestroyed do it when destruction completes.  If we just started destruction while
+        // the client still owns the surface, then our surfaceDestroyed would assume that android
+        // initiated the destruction since |mSurfaceDestroyed| would imply that the client didn't.
+
+        mParentView.post(new Runnable() {
+            @Override
+            public void run() {
+                if (mOwnedByClient == null) return;
+                SurfaceState owned = mOwnedByClient;
+                mClient.surfaceDestroyed(mOwnedByClient.surfaceHolder());
+                mOwnedByClient = null;
+                startSurfaceDestruction(owned);
+            }
+        });
+    }
+
+    @Override
+    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
+        SurfaceState state = getStateForHolder(holder);
+        assert state != null;
+
+        // If this is the surface that the client currently cares about, then notify the client.
+        // Note that surfaceChanged is guaranteed to come only after surfaceCreated.  Also, if the
+        // client has requested a different surface but hasn't gotten it yet, then skip this.
+        if (state == mOwnedByClient && state == mRequestedByClient) {
+            mClient.surfaceChanged(holder, format, width, height);
+        }
+    }
+
+    @Override
+    public void surfaceCreated(SurfaceHolder holder) {
+        SurfaceState state = getStateForHolder(holder);
+        assert state != null;
+
+        if (state != mRequestedByClient) {
+            // Surface is created, but it's not the one that's been requested most recently.  Just
+            // destroy it again.
+            postSurfaceDestruction(state);
+            return;
+        }
+
+        // The client requested a surface, and it's now available.  If the client owns a
+        // surface, then notify it that it doesn't.  Note that the client can't own |state| at
+        // this point, since we would have removed ownership when we got surfaceDestroyed.
+        if (mOwnedByClient != null) mClient.surfaceDestroyed(mOwnedByClient.surfaceHolder());
+
+        mOwnedByClient = mRequestedByClient;
+        mClient.surfaceCreated(mOwnedByClient.surfaceHolder());
+    }
+
+    @Override
+    public void surfaceDestroyed(SurfaceHolder holder) {
+        SurfaceState state = getStateForHolder(holder);
+        assert state != null;
+
+        // We might not have requested destruction, but clear the flag in case we did.
+        state.destroyPending = false;
+
+        // If the client owns this surface, then notify it synchronously that it no longer does.
+        // This can happen if Android destroys the surface on its own.
+        if (state == mOwnedByClient) {
+            mClient.surfaceDestroyed(holder);
+            mOwnedByClient = null;
+
+            // Do not re-request the surface here.  If android gives the surface back, then we'll
+            // re-signal the client about construction.
+            return;
+        }
+
+        // The client doesn't own this surface, but might want it.
+        // If the client has requested this surface, then start construction on it.  The client will
+        // be notified when it completes.  This can happen if the client re-requests a surface after
+        // we start destruction on it from a previous request, for example.  We post this for later,
+        // since we might be called while removing |state| from the view tree. In general, posting
+        // from here is good.
+        if (state == mRequestedByClient) {
+            postSurfaceConstruction(state);
+        } else {
+            // This isn't the requested surface.  If android destroyed it, then also unhook it so
+            // that it isn't recreated later.  If we did, then it's already not attached, and this
+            // will do nothing.
+            postSurfaceDestruction(state);
+        }
+    }
+
+    /**
+     * Update the background drawable on all surfaces.
+     */
+    public void setBackgroundDrawable(Drawable background) {
+        mTranslucent.surfaceView.setBackgroundDrawable(background);
+        if (mOpaque != null) mOpaque.surfaceView.setBackgroundDrawable(background);
+    }
+
+    /**
+     * Set |willNotDraw| on all surfaces.
+     */
+    public void setWillNotDraw(boolean willNotDraw) {
+        mTranslucent.surfaceView.setWillNotDraw(willNotDraw);
+        if (mOpaque != null) mOpaque.surfaceView.setWillNotDraw(willNotDraw);
+    }
+
+    /**
+     * Return the SurfaceState for |holder|, or null if it isn't either.
+     */
+    private SurfaceState getStateForHolder(SurfaceHolder holder) {
+        if (mTranslucent.surfaceHolder() == holder) return mTranslucent;
+
+        if (mOpaque != null && mOpaque.surfaceHolder() == holder) return mOpaque;
+
+        return null;
+    }
+
+    private void startSurfaceConstruction(SurfaceState state) {
+        if (state.surfaceView.getParent() != null) return;
+
+        FrameLayout.LayoutParams lp = new FrameLayout.LayoutParams(
+                ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT);
+        mParentView.addView(state.surfaceView, lp);
+        mParentView.bringChildToFront(state.surfaceView);
+        mParentView.postInvalidateOnAnimation();
+    }
+
+    private void postSurfaceConstruction(final SurfaceState state) {
+        mParentView.post(new Runnable() {
+            @Override
+            public void run() {
+                startSurfaceConstruction(state);
+            }
+        });
+    }
+
+    private void startSurfaceDestruction(SurfaceState state) {
+        // If we're called while we're not attached, then do nothing.  This makes it easier for the
+        // client, since it doesn't have to keep track of whether the outgoing surface has been
+        // destroyed or not.
+        if (state.surfaceView.getParent() == null) return;
+
+        state.destroyPending = true;
+        mParentView.removeView(state.surfaceView);
+    }
+
+    private void postSurfaceDestruction(final SurfaceState state) {
+        mParentView.post(new Runnable() {
+            @Override
+            public void run() {
+                startSurfaceDestruction(state);
+            }
+        });
+    }
+}