ThinWebView : SurfaceView zOrder issue

This CL addresses the surface view transparent issue after screen off/on.
This is one of the approaches to fix this. When screen is turned off, in
versions >=P, we recreate the compositor surface. Also in Q, we use
surface control, and use a Translucent surface. For translucent surfaces,
we use zOrderMediaOverly. This CL fixes this issue by not setting
zOrderMediaOverlay for the main compositor view when surface control
is enabled. Another alternative is to recreate the ThinWebView surface
after screen off/on, and order both of them correctly so that TWV
surface is created after the main compositor. This CL follows the former approach.

This CL also reverts back the setZOrderOnTop added recently in CL
https://chromium-review.googlesource.com/c/chromium/src/+/1939892

Bug: 1024622
Change-Id: I889a98aa18943bfbcd45797c031196640a822b88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1954452
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723608}

chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrCompositorSurfaceManager.java

@@ -97,6 +97,9 @@ public class VrCompositorSurfaceManager implements CompositorSurfaceManager {
     @Override
     public void recreateSurface() {}
 
+    @Override
+    public void recreateTranslucentSurfaceForSurfaceControl() {}
+
     @Override
     public void setBackgroundDrawable(Drawable background) {}
 

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

@@ -51,6 +51,14 @@ public interface CompositorSurfaceManager {
      */
     void recreateSurface();
 
+    /**
+     * Called by client to re-create the translucent surface after surface control is enabled. For
+     * surface control mode, the translucent surface should have a different z-order and should be
+     * recreated if the surface control mode is changed.
+     * TODO(crbug.com/1031636): Remove this method once SurfaceControl is always enabled on Q.
+     */
+    void recreateTranslucentSurfaceForSurfaceControl();
+
     /**
      * Update the background drawable on all surfaces.
      */

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

@@ -55,7 +55,8 @@ class CompositorSurfaceManagerImpl implements SurfaceHolder.Callback2, Composito
         // Parent ViewGroup, or null.
         private ViewGroup mParent;
 
-        public SurfaceState(Context context, int format, SurfaceHolder.Callback2 callback) {
+        public SurfaceState(Context context, int format, boolean useSurfaceControl,
+                SurfaceHolder.Callback2 callback) {
             surfaceView = new SurfaceView(context);
 
             // Media overlays require a translucent surface for the compositor which should be
@@ -65,7 +66,9 @@ class CompositorSurfaceManagerImpl implements SurfaceHolder.Callback2, Composito
             // stacked on top of the translucent one, the framework doesn't draw any content
             // underneath it and shows its background instead when it has no content during the
             // transition.
-            if (format == PixelFormat.TRANSLUCENT) surfaceView.setZOrderMediaOverlay(true);
+            if (format == PixelFormat.TRANSLUCENT && !useSurfaceControl) {
+                surfaceView.setZOrderMediaOverlay(true);
+            }
             surfaceView.setVisibility(View.INVISIBLE);
             surfaceHolder().setFormat(format);
             surfaceHolder().addCallback(callback);
@@ -107,7 +110,7 @@ class CompositorSurfaceManagerImpl implements SurfaceHolder.Callback2, Composito
     private static final String TAG = "CompositorSurfaceMgr";
 
     // SurfaceView with a translucent PixelFormat.
-    private final SurfaceState mTranslucent;
+    private SurfaceState mTranslucent;
 
     // SurfaceView with an opaque PixelFormat.
     private final SurfaceState mOpaque;
@@ -127,12 +130,15 @@ class CompositorSurfaceManagerImpl implements SurfaceHolder.Callback2, Composito
     // View to which we'll attach the SurfaceView.
     private final ViewGroup mParentView;
 
-    public CompositorSurfaceManagerImpl(ViewGroup parentView, SurfaceManagerCallbackTarget client) {
+    public CompositorSurfaceManagerImpl(
+            ViewGroup parentView, SurfaceManagerCallbackTarget client, boolean useSurfaceControl) {
         mParentView = parentView;
         mClient = client;
 
-        mTranslucent = new SurfaceState(parentView.getContext(), PixelFormat.TRANSLUCENT, this);
-        mOpaque = new SurfaceState(mParentView.getContext(), PixelFormat.OPAQUE, this);
+        mTranslucent = new SurfaceState(
+                parentView.getContext(), PixelFormat.TRANSLUCENT, useSurfaceControl, this);
+        mOpaque = new SurfaceState(
+                mParentView.getContext(), PixelFormat.OPAQUE, useSurfaceControl, this);
     }
 
     /**
@@ -239,6 +245,18 @@ class CompositorSurfaceManagerImpl implements SurfaceHolder.Callback2, Composito
         });
     }
 
+    @Override
+    public void recreateTranslucentSurfaceForSurfaceControl() {
+        // Recreate the translucent surface only if it hasn't been used or requested by client yet.
+        // This should be very early in the flow and until now the opaque one should be the one
+        // being used.
+        if (mTranslucent.isAttached() || mTranslucent == mRequestedByClient) return;
+
+        mTranslucent.surfaceHolder().removeCallback(this);
+        mTranslucent =
+                new SurfaceState(mParentView.getContext(), PixelFormat.TRANSLUCENT, true, this);
+    }
+
     @Override
     public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
         SurfaceState state = getStateForHolder(holder);

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

@@ -134,7 +134,7 @@ public class CompositorView
             return;
         }
 
-        mCompositorSurfaceManager = new CompositorSurfaceManagerImpl(this, this);
+        mCompositorSurfaceManager = new CompositorSurfaceManagerImpl(this, this, false);
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
             mScreenStateReceiver = new ScreenStateReceiverWorkaround();
         }
@@ -487,6 +487,7 @@ public class CompositorView
     @CalledByNative
     private void notifyWillUseSurfaceControl() {
         mIsSurfaceControlEnabled = true;
+        mCompositorSurfaceManager.recreateTranslucentSurfaceForSurfaceControl();
     }
 
     /**
@@ -602,7 +603,8 @@ public class CompositorView
     }
 
     private void createCompositorSurfaceManager() {
-        mCompositorSurfaceManager = new CompositorSurfaceManagerImpl(this, this);
+        mCompositorSurfaceManager =
+                new CompositorSurfaceManagerImpl(this, this, mIsSurfaceControlEnabled);
         mCompositorSurfaceManager.requestSurface(getSurfacePixelFormat());
         CompositorViewJni.get().setNeedsComposite(mNativeCompositorView, CompositorView.this);
         mCompositorSurfaceManager.setVisibility(getVisibility());

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/ephemeraltab/EphemeralTabSheetContent.java

@@ -94,10 +94,8 @@ public class EphemeralTabSheetContent implements BottomSheetContent {
      * bottom sheet.
      */
     private void createThinWebView(int maxSheetHeight) {
-        ThinWebViewConstraints thinWebViewConstraints = new ThinWebViewConstraints();
-        thinWebViewConstraints.zOrderOnTop = true;
         mThinWebView = ThinWebViewFactory.create(
-                mContext, new ActivityWindowAndroid(mContext), thinWebViewConstraints);
+                mContext, new ActivityWindowAndroid(mContext), new ThinWebViewConstraints());
 
         mSheetContentView = new FrameLayout(mContext);
         mThinWebView.getView().setLayoutParams(new FrameLayout.LayoutParams(

chrome/android/java/src/org/chromium/chrome/browser/payments/handler/PaymentHandlerView.java

@@ -46,10 +46,8 @@ import org.chromium.ui.base.ActivityWindowAndroid;
         mContentView = (FrameLayout) LayoutInflater.from(activity).inflate(
                 R.layout.payment_handler_content, null);
 
-        ThinWebViewConstraints thinWebViewConstraints = new ThinWebViewConstraints();
-        thinWebViewConstraints.zOrderOnTop = true;
         mThinWebView = ThinWebViewFactory.create(
-                activity, new ActivityWindowAndroid(activity), thinWebViewConstraints);
+                activity, new ActivityWindowAndroid(activity), new ThinWebViewConstraints());
         initContentView(activity, mThinWebView, webContents, webContentView);
     }
 

chrome/android/junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImplTest.java

@@ -129,7 +129,7 @@ public class CompositorSurfaceManagerImplTest {
         MockitoAnnotations.initMocks(this);
         Activity activity = Robolectric.buildActivity(Activity.class).setup().get();
         mLayout = new FrameLayout(activity);
-        mManager = new CompositorSurfaceManagerImpl(mLayout, mCallback);
+        mManager = new CompositorSurfaceManagerImpl(mLayout, mCallback, false);
     }
 
     private void runDelayedTasks() {

chrome/browser/android/thin_webview/internal/java/src/org/chromium/chrome/browser/thinwebview/internal/CompositorViewImpl.java

@@ -80,7 +80,7 @@ public class CompositorViewImpl implements CompositorView {
 
     private SurfaceView createSurfaceView() {
         SurfaceView surfaceView = new SurfaceView(mContext);
-        if (mViewConstraints.zOrderOnTop) surfaceView.setZOrderOnTop(true);
+        surfaceView.setZOrderMediaOverlay(true);
         surfaceView.getHolder().addCallback(new SurfaceHolder.Callback() {
             @Override
             public void surfaceCreated(SurfaceHolder surfaceHolder) {

chrome/browser/android/thin_webview/java/src/org/chromium/chrome/browser/thinwebview/ThinWebViewConstraints.java

@@ -6,13 +6,6 @@ package org.chromium.chrome.browser.thinwebview;
 
 /** Various constraints associated with the thin webview based on the usage. */
 public class ThinWebViewConstraints implements Cloneable {
-    /**
-     * Whether this view should be placed on the top of the window. Set this to true only when this
-     * view will be used on top of another surface view underneath. Note, don't add any views on the
-     * top of this surface in the same window, as they would be invisible.
-     */
-    public boolean zOrderOnTop;
-
     /**
      * Whether this view will support opacity.
      */
@@ -21,7 +14,6 @@ public class ThinWebViewConstraints implements Cloneable {
     @Override
     public ThinWebViewConstraints clone() {
         ThinWebViewConstraints clone = new ThinWebViewConstraints();
-        clone.zOrderOnTop = zOrderOnTop;
         clone.supportsOpacity = supportsOpacity;
         return clone;
     }