android: Support passing SurfaceControl over aidl

Make GpuSurfaceLookup::AcquireJavaSurface return a variant of either ScopedJavaSurface or ScopedJavaSurfaceControl. Update all getters (callers of AcquireJavaSurface) to handle both (or CHECK only one is possible). No change to all setters (callers of AddSurfaceForNativeWidget), so they all still pass in Surface for now. Bug: 1399283 Change-Id: Ia86e9c0992d5e891ac9b6d46d7723af61e7e261e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4104520 Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org> Reviewed-by: Andrew Grieve <agrieve@chromium.org> Commit-Queue: Bo Liu <boliu@chromium.org> Reviewed-by: Joe Mason <joenotcharles@google.com> Cr-Commit-Position: refs/heads/main@{#1084557}
chromium · Dec 16, 2022 · 30286a8 · 30286a8
1 parent 473043a
commit 30286a8
Show file tree

Hide file tree

Showing 12 changed files with 235 additions and 53 deletions.
diff --git a/components/viz/service/display_embedder/skia_output_device_vulkan.cc b/components/viz/service/display_embedder/skia_output_device_vulkan.cc
@@ -262,9 +262,14 @@ bool SkiaOutputDeviceVulkan::Initialize() {
   gfx::AcceleratedWidget accelerated_widget = gfx::kNullAcceleratedWidget;
 #if BUILDFLAG(IS_ANDROID)
   bool can_be_used_with_surface_control = false;
-  gl::ScopedJavaSurface scoped_java_surface =
+  auto surface_variant =
       gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(
           surface_handle_, &can_be_used_with_surface_control);
+  // Should only reach here if surface control is disabled. In which case
+  // browser should not be sending ScopedJavaSurfaceControl variant.
+  CHECK(absl::holds_alternative<gl::ScopedJavaSurface>(surface_variant));
+  gl::ScopedJavaSurface& scoped_java_surface =
+      absl::get<gl::ScopedJavaSurface>(surface_variant);
   gl::ScopedANativeWindow window(scoped_java_surface);
   accelerated_widget = window.a_native_window();
 #else

diff --git a/content/app/android/content_child_process_service_delegate.cc b/content/app/android/content_child_process_service_delegate.cc
@@ -23,6 +23,7 @@
 #include "gpu/ipc/common/android/scoped_surface_request_conduit.h"
 #include "gpu/ipc/common/gpu_surface_lookup.h"
 #include "ui/gl/android/scoped_java_surface.h"
+#include "ui/gl/android/scoped_java_surface_control.h"
 #include "ui/gl/android/surface_texture.h"
 
 using base::android::AttachCurrentThread;
@@ -65,7 +66,7 @@ class ChildProcessSurfaceManager : public gpu::ScopedSurfaceRequestConduit,
   }
 
   // Overridden from GpuSurfaceLookup:
-  gl::ScopedJavaSurface AcquireJavaSurface(
+  JavaSurfaceVariant AcquireJavaSurface(
       int surface_id,
       bool* can_be_used_with_surface_control) override {
     JNIEnv* env = base::android::AttachCurrentThread();
@@ -75,15 +76,22 @@ class ChildProcessSurfaceManager : public gpu::ScopedSurfaceRequestConduit,
     if (!surface_wrapper)
       return gl::ScopedJavaSurface();
 
-    gl::ScopedJavaSurface surface(
-        content::JNI_SurfaceWrapper_takeSurface(env, surface_wrapper),
-        /*auto_release=*/true);
-    DCHECK(!surface.j_surface().is_null());
-
     *can_be_used_with_surface_control =
-        content::JNI_SurfaceWrapper_canBeUsedWithSurfaceControl(
-            env, surface_wrapper);
-    return surface;
+        JNI_SurfaceWrapper_canBeUsedWithSurfaceControl(env, surface_wrapper);
+    bool wraps_surface =
+        JNI_SurfaceWrapper_getWrapsSurface(env, surface_wrapper);
+
+    if (wraps_surface) {
+      gl::ScopedJavaSurface surface(
+          content::JNI_SurfaceWrapper_takeSurface(env, surface_wrapper),
+          /*auto_release=*/true);
+      DCHECK(!surface.j_surface().is_null());
+      return surface;
+    } else {
+      return gl::ScopedJavaSurfaceControl(
+          JNI_SurfaceWrapper_takeSurfaceControl(env, surface_wrapper),
+          /*release_on_destroy=*/true);
+    }
   }
 
  private:

diff --git a/content/browser/android/dialog_overlay_impl.cc b/content/browser/android/dialog_overlay_impl.cc
@@ -16,6 +16,7 @@
 #include "gpu/ipc/common/gpu_surface_tracker.h"
 #include "media/mojo/mojom/android_overlay.mojom.h"
 #include "mojo/public/cpp/bindings/sync_call_restrictions.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 #include "ui/android/view_android_observer.h"
 #include "ui/android/window_android.h"
 
@@ -343,10 +344,13 @@ JNI_DialogOverlayImpl_LookupSurfaceForTesting(
     JNIEnv* env,
     jint surfaceId) {
   bool can_be_used_with_surface_control = false;
-  gl::ScopedJavaSurface surface =
-      gpu::GpuSurfaceTracker::Get()->AcquireJavaSurface(
-          surfaceId, &can_be_used_with_surface_control);
-  return ScopedJavaLocalRef<jobject>(surface.j_surface());
+  auto surface_variant = gpu::GpuSurfaceTracker::Get()->AcquireJavaSurface(
+      surfaceId, &can_be_used_with_surface_control);
+  if (!absl::holds_alternative<gl::ScopedJavaSurface>(surface_variant)) {
+    return nullptr;
+  }
+  return ScopedJavaLocalRef<jobject>(
+      absl::get<gl::ScopedJavaSurface>(surface_variant).j_surface());
 }
 
 }  // namespace content
diff --git a/content/browser/android/gpu_process_callback.cc b/content/browser/android/gpu_process_callback.cc
@@ -2,14 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <utility>
+
 #include "base/android/scoped_java_ref.h"
 #include "base/android/unguessable_token_android.h"
+#include "base/functional/overloaded.h"
 #include "content/browser/android/scoped_surface_request_manager.h"
 #include "content/common/android/surface_wrapper.h"
+#include "content/public/android/content_jni_headers/GpuProcessCallback_jni.h"
 #include "content/public/browser/browser_thread.h"
 #include "gpu/ipc/common/gpu_surface_tracker.h"
-
-#include "content/public/android/content_jni_headers/GpuProcessCallback_jni.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 
 namespace content {
 
@@ -37,14 +40,26 @@ base::android::ScopedJavaLocalRef<jobject>
 JNI_GpuProcessCallback_GetViewSurface(
     JNIEnv* env,
     jint surface_id) {
+  base::android::ScopedJavaLocalRef<jobject> j_surface_wrapper;
   bool can_be_used_with_surface_control = false;
-  gl::ScopedJavaSurface surface_view =
+  auto surface_variant =
       gpu::GpuSurfaceTracker::GetInstance()->AcquireJavaSurface(
           surface_id, &can_be_used_with_surface_control);
-  if (surface_view.IsEmpty())
-    return nullptr;
-  return JNI_SurfaceWrapper_create(env, surface_view.j_surface(),
-                                   can_be_used_with_surface_control);
+  absl::visit(
+      base::Overloaded{[&](gl::ScopedJavaSurface&& scoped_java_surface) {
+                         if (!scoped_java_surface.IsEmpty()) {
+                           j_surface_wrapper = JNI_SurfaceWrapper_create(
+                               env, scoped_java_surface.j_surface(),
+                               can_be_used_with_surface_control);
+                         }
+                       },
+                       [&](gl::ScopedJavaSurfaceControl&& surface_control) {
+                         j_surface_wrapper =
+                             JNI_SurfaceWrapper_createFromSurfaceControl(
+                                 env, std::move(surface_control));
+                       }},
+      std::move(surface_variant));
+  return j_surface_wrapper;
 }
 
 }  // namespace content
diff --git a/content/common/android/surface_wrapper.cc b/content/common/android/surface_wrapper.cc
@@ -4,6 +4,7 @@
 
 #include "content/common/android/surface_wrapper.h"
 
+#include "base/check.h"
 #include "content/public/android/content_jni_headers/SurfaceWrapper_jni.h"
 
 namespace content {
@@ -15,16 +16,41 @@ base::android::ScopedJavaLocalRef<jobject> JNI_SurfaceWrapper_create(
   return Java_SurfaceWrapper_create(env, surface, canBeUsedWithSurfaceControl);
 }
 
+base::android::ScopedJavaLocalRef<jobject>
+JNI_SurfaceWrapper_createFromSurfaceControl(
+    JNIEnv* env,
+    gl::ScopedJavaSurfaceControl surface_control) {
+  bool release_on_destroy = false;
+  auto java_surface_control =
+      surface_control.TakeJavaSurfaceControl(release_on_destroy);
+  CHECK(!release_on_destroy);
+  return Java_SurfaceWrapper_createFromSurfaceControl(env,
+                                                      java_surface_control);
+}
+
 jboolean JNI_SurfaceWrapper_canBeUsedWithSurfaceControl(
     JNIEnv* env,
     const base::android::JavaRef<jobject>& obj) {
   return Java_SurfaceWrapper_canBeUsedWithSurfaceControl(env, obj);
 }
 
+jboolean JNI_SurfaceWrapper_getWrapsSurface(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& obj) {
+  return Java_SurfaceWrapper_getWrapsSurface(env, obj);
+}
+
 base::android::ScopedJavaLocalRef<jobject> JNI_SurfaceWrapper_takeSurface(
     JNIEnv* env,
     const base::android::JavaRef<jobject>& obj) {
   return Java_SurfaceWrapper_takeSurface(env, obj);
 }
 
+base::android::ScopedJavaLocalRef<jobject>
+JNI_SurfaceWrapper_takeSurfaceControl(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& obj) {
+  return Java_SurfaceWrapper_takeSurfaceControl(env, obj);
+}
+
 }  // namespace content.
diff --git a/content/common/android/surface_wrapper.h b/content/common/android/surface_wrapper.h
@@ -6,6 +6,7 @@
 #define CONTENT_COMMON_ANDROID_SURFACE_WRAPPER_H_
 
 #include "base/android/scoped_java_ref.h"
+#include "ui/gl/android/scoped_java_surface_control.h"
 
 namespace content {
 
@@ -14,14 +15,28 @@ base::android::ScopedJavaLocalRef<jobject> JNI_SurfaceWrapper_create(
     const base::android::JavaRef<jobject>& surface,
     jboolean canBeUsedWithSurfaceControl);
 
+base::android::ScopedJavaLocalRef<jobject>
+JNI_SurfaceWrapper_createFromSurfaceControl(
+    JNIEnv* env,
+    gl::ScopedJavaSurfaceControl surface_control);
+
 jboolean JNI_SurfaceWrapper_canBeUsedWithSurfaceControl(
     JNIEnv* env,
     const base::android::JavaRef<jobject>& obj);
 
+jboolean JNI_SurfaceWrapper_getWrapsSurface(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& obj);
+
 base::android::ScopedJavaLocalRef<jobject> JNI_SurfaceWrapper_takeSurface(
     JNIEnv* env,
     const base::android::JavaRef<jobject>& obj);
 
+base::android::ScopedJavaLocalRef<jobject>
+JNI_SurfaceWrapper_takeSurfaceControl(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& obj);
+
 }  // namespace content
 
 #endif  // CONTENT_COMMON_ANDROID_SURFACE_WRAPPER_H_
diff --git a/content/public/android/java/src/org/chromium/content/common/SurfaceWrapper.java b/content/public/android/java/src/org/chromium/content/common/SurfaceWrapper.java
@@ -4,9 +4,13 @@
 
 package org.chromium.content.common;
 
+import android.os.Build;
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.view.Surface;
+import android.view.SurfaceControl;
+
+import androidx.annotation.RequiresApi;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
@@ -18,16 +22,34 @@
 @JNINamespace("content")
 @MainDex
 public class SurfaceWrapper implements Parcelable {
+    private final boolean mWrapsSurface;
     private Surface mSurface;
     private final boolean mCanBeUsedWithSurfaceControl;
+    private SurfaceControl mSurfaceControl;
 
     private SurfaceWrapper(Surface surface, boolean canBeUsedWithSurfaceControl) {
+        mWrapsSurface = true;
         mSurface = surface;
         mCanBeUsedWithSurfaceControl = canBeUsedWithSurfaceControl;
+        mSurfaceControl = null;
+    }
+
+    @RequiresApi(Build.VERSION_CODES.Q)
+    private SurfaceWrapper(SurfaceControl surfaceControl) {
+        mWrapsSurface = false;
+        mSurface = null;
+        mCanBeUsedWithSurfaceControl = true;
+        mSurfaceControl = surfaceControl;
+    }
+
+    @CalledByNative
+    private boolean getWrapsSurface() {
+        return mWrapsSurface;
     }
 
     @CalledByNative
     private Surface takeSurface() {
+        assert mWrapsSurface;
         Surface surface = mSurface;
         mSurface = null;
         return surface;
@@ -38,30 +60,58 @@ private boolean canBeUsedWithSurfaceControl() {
         return mCanBeUsedWithSurfaceControl;
     }
 
+    @CalledByNative
+    private SurfaceControl takeSurfaceControl() {
+        assert !mWrapsSurface;
+        SurfaceControl sc = mSurfaceControl;
+        mSurfaceControl = null;
+        return sc;
+    }
+
     @Override
     public int describeContents() {
         return 0;
     }
 
     @Override
     public void writeToParcel(Parcel out, int flags) {
-        // Ignore flags so that the Surface won't call release()
-        mSurface.writeToParcel(out, 0);
-        out.writeInt(mCanBeUsedWithSurfaceControl ? 1 : 0);
+        out.writeInt(mWrapsSurface ? 1 : 0);
+        if (mWrapsSurface) {
+            // Ignore flags so that the Surface won't call release()
+            mSurface.writeToParcel(out, 0);
+            out.writeInt(mCanBeUsedWithSurfaceControl ? 1 : 0);
+        } else {
+            // Ignore flags so that SurfaceControl won't call release().
+            mSurfaceControl.writeToParcel(out, 0);
+        }
     }
 
     @CalledByNative
     private static SurfaceWrapper create(Surface surface, boolean canBeUsedWithSurfaceControl) {
         return new SurfaceWrapper(surface, canBeUsedWithSurfaceControl);
     }
 
+    @RequiresApi(Build.VERSION_CODES.Q)
+    @CalledByNative
+    private static SurfaceWrapper createFromSurfaceControl(SurfaceControl surfaceControl) {
+        return new SurfaceWrapper(surfaceControl);
+    }
+
     public static final Parcelable.Creator<SurfaceWrapper> CREATOR =
             new Parcelable.Creator<SurfaceWrapper>() {
                 @Override
                 public SurfaceWrapper createFromParcel(Parcel in) {
-                    Surface surface = Surface.CREATOR.createFromParcel(in);
-                    boolean canBeUsedWithSurfaceControl = (in.readInt() == 1);
-                    return new SurfaceWrapper(surface, canBeUsedWithSurfaceControl);
+                    final boolean wrapsSurface = (in.readInt() == 1);
+                    if (wrapsSurface) {
+                        Surface surface = Surface.CREATOR.createFromParcel(in);
+                        boolean canBeUsedWithSurfaceControl = (in.readInt() == 1);
+                        return new SurfaceWrapper(surface, canBeUsedWithSurfaceControl);
+                    } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+                        SurfaceControl surfaceControl = SurfaceControl.CREATOR.createFromParcel(in);
+                        return new SurfaceWrapper(surfaceControl);
+                    } else {
+                        throw new RuntimeException("not reached");
+                    }
                 }
 
                 @Override

diff --git a/gpu/ipc/common/gpu_surface_lookup.h b/gpu/ipc/common/gpu_surface_lookup.h
@@ -7,8 +7,10 @@
 
 #include "gpu/gpu_export.h"
 #include "gpu/ipc/common/surface_handle.h"
+#include "third_party/abseil-cpp/absl/types/variant.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gl/android/scoped_java_surface.h"
+#include "ui/gl/android/scoped_java_surface_control.h"
 
 namespace gpu {
 
@@ -26,7 +28,9 @@ class GPU_EXPORT GpuSurfaceLookup {
   static GpuSurfaceLookup* GetInstance();
   static void InitInstance(GpuSurfaceLookup* lookup);
 
-  virtual gl::ScopedJavaSurface AcquireJavaSurface(
+  using JavaSurfaceVariant =
+      absl::variant<gl::ScopedJavaSurface, gl::ScopedJavaSurfaceControl>;
+  virtual JavaSurfaceVariant AcquireJavaSurface(
       int surface_id,
       bool* can_be_used_with_surface_control) = 0;
 };