0

Return surface record from AcquireJavaSurface method.

As per current implementation method returns a java surface variant and
fills in `can_be_used_with_surface_control` argument passed to it. This
change modifies it to return a surface record which contains both
surface variant and `can_be_used_with_surface_control`.

This also simplifies future changes we are planning to make for the
linked bug.

Bug: b/364201006
Change-Id: I9700823fd3a241c50b12c42dee792d35786f4510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5870133
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Kartar Singh <kartarsingh@google.com>
Cr-Commit-Position: refs/heads/main@{#1357040}
This commit is contained in:
Kartar Singh
2024-09-18 13:01:59 +00:00
committed by Chromium LUCI CQ
parent 63d4d2d0ac
commit 169f17570b
16 changed files with 94 additions and 110 deletions

@ -116,14 +116,14 @@ bool SkiaOutputDeviceDawn::Initialize(gpu::SurfaceHandle surface_handle) {
#endif
#if BUILDFLAG(IS_ANDROID)
bool can_be_used_with_surface_control = false;
auto surface_variant =
gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(
surface_handle, &can_be_used_with_surface_control);
auto surface_record =
gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(surface_handle);
// 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));
auto& scoped_java_surface = absl::get<gl::ScopedJavaSurface>(surface_variant);
CHECK(absl::holds_alternative<gl::ScopedJavaSurface>(
surface_record.surface_variant));
auto& scoped_java_surface =
absl::get<gl::ScopedJavaSurface>(surface_record.surface_variant);
android_native_window_ = gl::ScopedANativeWindow(scoped_java_surface);
wgpu::SurfaceSourceAndroidNativeWindow android_native_window_desc;

@ -277,15 +277,14 @@ void SkiaOutputDeviceVulkan::EndPaint() {
bool SkiaOutputDeviceVulkan::Initialize() {
gfx::AcceleratedWidget accelerated_widget = gfx::kNullAcceleratedWidget;
#if BUILDFLAG(IS_ANDROID)
bool can_be_used_with_surface_control = false;
auto surface_variant =
gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(
surface_handle_, &can_be_used_with_surface_control);
auto surface_record =
gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(surface_handle_);
// 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));
CHECK(absl::holds_alternative<gl::ScopedJavaSurface>(
surface_record.surface_variant));
gl::ScopedJavaSurface& scoped_java_surface =
absl::get<gl::ScopedJavaSurface>(surface_variant);
absl::get<gl::ScopedJavaSurface>(surface_record.surface_variant);
gl::ScopedANativeWindow window(scoped_java_surface);
accelerated_widget = window.a_native_window();
#else

@ -125,9 +125,8 @@ void XrSessionCoordinator::OnDrawingSurfaceReady(
gl::ScopedANativeWindow window(scoped_surface);
gpu::SurfaceHandle surface_handle =
gpu::GpuSurfaceTracker::Get()->AddSurfaceForNativeWidget(
gpu::GpuSurfaceTracker::SurfaceRecord(
std::move(scoped_surface),
/*can_be_used_with_surface_control=*/false));
gpu::SurfaceRecord(std::move(scoped_surface),
/*can_be_used_with_surface_control=*/false));
ui::WindowAndroid* root_window =
ui::WindowAndroid::FromJavaWindowAndroid(java_root_window);
display::Display::Rotation display_rotation =

@ -217,10 +217,9 @@ void MailboxToSurfaceBridgeImpl::BindContextProviderToCurrentThread() {
void MailboxToSurfaceBridgeImpl::CreateSurface(
gl::SurfaceTexture* surface_texture) {
gpu::GpuSurfaceTracker* tracker = gpu::GpuSurfaceTracker::Get();
surface_handle_ =
tracker->AddSurfaceForNativeWidget(gpu::GpuSurfaceTracker::SurfaceRecord(
gl::ScopedJavaSurface(surface_texture),
false /* can_be_used_with_surface_control */));
surface_handle_ = tracker->AddSurfaceForNativeWidget(
gpu::SurfaceRecord(gl::ScopedJavaSurface(surface_texture),
false /* can_be_used_with_surface_control */));
// Unregistering happens in the destructor.
}

@ -66,17 +66,16 @@ class ChildProcessSurfaceManager : public gpu::ScopedSurfaceRequestConduit,
}
// Overridden from GpuSurfaceLookup:
JavaSurfaceVariant AcquireJavaSurface(
int surface_id,
bool* can_be_used_with_surface_control) override {
gpu::SurfaceRecord AcquireJavaSurface(int surface_id) override {
JNIEnv* env = base::android::AttachCurrentThread();
base::android::ScopedJavaLocalRef<jobject> surface_wrapper =
content::Java_ContentChildProcessServiceDelegate_getViewSurface(
env, service_impl_, surface_id);
if (!surface_wrapper)
return gl::ScopedJavaSurface();
return gpu::SurfaceRecord(gl::ScopedJavaSurface(),
/*can_be_used_with_surface_control=*/false);
*can_be_used_with_surface_control =
bool can_be_used_with_surface_control =
JNI_SurfaceWrapper_canBeUsedWithSurfaceControl(env, surface_wrapper);
bool wraps_surface =
JNI_SurfaceWrapper_getWrapsSurface(env, surface_wrapper);
@ -86,11 +85,13 @@ class ChildProcessSurfaceManager : public gpu::ScopedSurfaceRequestConduit,
content::JNI_SurfaceWrapper_takeSurface(env, surface_wrapper),
/*auto_release=*/true);
DCHECK(!surface.j_surface().is_null());
return surface;
return gpu::SurfaceRecord(std::move(surface),
can_be_used_with_surface_control);
} else {
return gl::ScopedJavaSurfaceControl(
gl::ScopedJavaSurfaceControl surface_control(
JNI_SurfaceWrapper_takeSurfaceControl(env, surface_wrapper),
/*release_on_destroy=*/true);
return gpu::SurfaceRecord(std::move(surface_control));
}
}

@ -327,9 +327,8 @@ static jint JNI_DialogOverlayImpl_RegisterSurface(
const JavaParamRef<jobject>& surface) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
return gpu::GpuSurfaceTracker::Get()->AddSurfaceForNativeWidget(
gpu::GpuSurfaceTracker::SurfaceRecord(
gl::ScopedJavaSurface(surface, /*auto_release=*/false),
/*can_be_used_with_surface_control=*/false));
gpu::SurfaceRecord(gl::ScopedJavaSurface(surface, /*auto_release=*/false),
/*can_be_used_with_surface_control=*/false));
}
static void JNI_DialogOverlayImpl_UnregisterSurface(
@ -343,14 +342,15 @@ static ScopedJavaLocalRef<jobject>
JNI_DialogOverlayImpl_LookupSurfaceForTesting(
JNIEnv* env,
jint surfaceId) {
bool can_be_used_with_surface_control = false;
auto surface_variant = gpu::GpuSurfaceTracker::Get()->AcquireJavaSurface(
surfaceId, &can_be_used_with_surface_control);
if (!absl::holds_alternative<gl::ScopedJavaSurface>(surface_variant)) {
auto surface_record =
gpu::GpuSurfaceTracker::Get()->AcquireJavaSurface(surfaceId);
if (!absl::holds_alternative<gl::ScopedJavaSurface>(
surface_record.surface_variant)) {
return nullptr;
}
return ScopedJavaLocalRef<jobject>(
absl::get<gl::ScopedJavaSurface>(surface_variant).j_surface());
absl::get<gl::ScopedJavaSurface>(surface_record.surface_variant)
.j_surface());
}
} // namespace content

@ -36,16 +36,14 @@ JNI_GpuProcessCallback_GetViewSurface(
JNIEnv* env,
jint surface_id) {
base::android::ScopedJavaLocalRef<jobject> j_surface_wrapper;
bool can_be_used_with_surface_control = false;
auto surface_variant =
gpu::GpuSurfaceTracker::GetInstance()->AcquireJavaSurface(
surface_id, &can_be_used_with_surface_control);
auto surface_record =
gpu::GpuSurfaceTracker::GetInstance()->AcquireJavaSurface(surface_id);
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);
surface_record.can_be_used_with_surface_control);
}
},
[&](gl::ScopedJavaSurfaceControl&& surface_control) {
@ -53,7 +51,7 @@ JNI_GpuProcessCallback_GetViewSurface(
JNI_SurfaceWrapper_createFromSurfaceControl(
env, std::move(surface_control));
}},
std::move(surface_variant));
std::move(surface_record.surface_variant));
return j_surface_wrapper;
}

@ -72,9 +72,8 @@ static jlong JNI_MagnifierSurfaceControl_Create(
gl::ScopedJavaSurfaceControl scoped_java_surface_control(j_surface_control,
release_on_destroy);
gpu::GpuSurfaceTracker* tracker = gpu::GpuSurfaceTracker::Get();
gpu::SurfaceHandle surface_handle =
tracker->AddSurfaceForNativeWidget(gpu::GpuSurfaceTracker::SurfaceRecord(
std::move(scoped_java_surface_control)));
gpu::SurfaceHandle surface_handle = tracker->AddSurfaceForNativeWidget(
gpu::SurfaceRecord(std::move(scoped_java_surface_control)));
return reinterpret_cast<jlong>(new MagnifierSurfaceControl(
web_contents, surface_handle, device_scale, width, height, corner_radius,

@ -350,9 +350,8 @@ void CompositorImpl::SetSurface(const base::android::JavaRef<jobject>& surface,
if (window) {
window_ = std::move(window);
// Register first, SetVisible() might create a LayerTreeFrameSink.
surface_handle_ = tracker->AddSurfaceForNativeWidget(
gpu::GpuSurfaceTracker::SurfaceRecord(
std::move(scoped_surface), can_be_used_with_surface_control));
surface_handle_ = tracker->AddSurfaceForNativeWidget(gpu::SurfaceRecord(
std::move(scoped_surface), can_be_used_with_surface_control));
SetVisible(true);
}
}

@ -12,6 +12,18 @@ namespace {
GpuSurfaceLookup* g_instance = nullptr;
} // anonymous namespace
SurfaceRecord::SurfaceRecord(gl::ScopedJavaSurface surface,
bool can_be_used_with_surface_control)
: surface_variant(std::move(surface)),
can_be_used_with_surface_control(can_be_used_with_surface_control) {}
SurfaceRecord::SurfaceRecord(gl::ScopedJavaSurfaceControl surface_control)
: surface_variant(std::move(surface_control)),
can_be_used_with_surface_control(true) {}
SurfaceRecord::~SurfaceRecord() = default;
SurfaceRecord::SurfaceRecord(SurfaceRecord&&) = default;
// static
GpuSurfaceLookup* GpuSurfaceLookup::GetInstance() {
DCHECK(g_instance);

@ -14,6 +14,21 @@
namespace gpu {
using JavaSurfaceVariant =
absl::variant<gl::ScopedJavaSurface, gl::ScopedJavaSurfaceControl>;
struct GPU_EXPORT SurfaceRecord {
SurfaceRecord(gl::ScopedJavaSurface surface,
bool can_be_used_with_surface_control);
explicit SurfaceRecord(gl::ScopedJavaSurfaceControl surface_control);
~SurfaceRecord();
SurfaceRecord(SurfaceRecord&&);
SurfaceRecord(const SurfaceRecord&) = delete;
JavaSurfaceVariant surface_variant;
bool can_be_used_with_surface_control = false;
};
// This class provides an interface to look up window surface handles
// that cannot be sent through the IPC channel.
class GPU_EXPORT GpuSurfaceLookup {
@ -28,11 +43,7 @@ class GPU_EXPORT GpuSurfaceLookup {
static GpuSurfaceLookup* GetInstance();
static void InitInstance(GpuSurfaceLookup* lookup);
using JavaSurfaceVariant =
absl::variant<gl::ScopedJavaSurface, gl::ScopedJavaSurfaceControl>;
virtual JavaSurfaceVariant AcquireJavaSurface(
int surface_id,
bool* can_be_used_with_surface_control) = 0;
virtual SurfaceRecord AcquireJavaSurface(int surface_id) = 0;
};
} // namespace gpu

@ -14,20 +14,6 @@
namespace gpu {
GpuSurfaceTracker::SurfaceRecord::SurfaceRecord(
gl::ScopedJavaSurface surface,
bool can_be_used_with_surface_control)
: surface_variant(std::move(surface)),
can_be_used_with_surface_control(can_be_used_with_surface_control) {}
GpuSurfaceTracker::SurfaceRecord::SurfaceRecord(
gl::ScopedJavaSurfaceControl surface_control)
: surface_variant(std::move(surface_control)),
can_be_used_with_surface_control(true) {}
GpuSurfaceTracker::SurfaceRecord::~SurfaceRecord() = default;
GpuSurfaceTracker::SurfaceRecord::SurfaceRecord(SurfaceRecord&&) = default;
GpuSurfaceTracker::GpuSurfaceTracker()
: next_surface_handle_(1) {
gpu::GpuSurfaceLookup::InitInstance(this);
@ -60,24 +46,23 @@ void GpuSurfaceTracker::RemoveSurface(gpu::SurfaceHandle surface_handle) {
surface_map_.erase(surface_handle);
}
GpuSurfaceTracker::JavaSurfaceVariant GpuSurfaceTracker::AcquireJavaSurface(
gpu::SurfaceHandle surface_handle,
bool* can_be_used_with_surface_control) {
SurfaceRecord GpuSurfaceTracker::AcquireJavaSurface(
gpu::SurfaceHandle surface_handle) {
base::AutoLock lock(surface_map_lock_);
SurfaceMap::const_iterator it = surface_map_.find(surface_handle);
if (it == surface_map_.end())
return gl::ScopedJavaSurface();
return SurfaceRecord(gl::ScopedJavaSurface(),
/*can_be_used_with_surface_control=*/false);
*can_be_used_with_surface_control =
it->second.can_be_used_with_surface_control;
return absl::visit(
base::Overloaded{
[&](const gl::ScopedJavaSurface& surface) {
DCHECK(surface.IsValid());
return JavaSurfaceVariant(surface.CopyRetainOwnership());
return SurfaceRecord(surface.CopyRetainOwnership(),
it->second.can_be_used_with_surface_control);
},
[&](const gl::ScopedJavaSurfaceControl& surface_control) {
return JavaSurfaceVariant(surface_control.CopyRetainOwnership());
return SurfaceRecord(surface_control.CopyRetainOwnership());
}},
it->second.surface_variant);
}

@ -31,22 +31,7 @@ namespace gpu {
// This class is thread safe.
class GPU_EXPORT GpuSurfaceTracker : public gpu::GpuSurfaceLookup {
public:
struct SurfaceRecord {
SurfaceRecord(gl::ScopedJavaSurface surface,
bool can_be_used_with_surface_control);
explicit SurfaceRecord(gl::ScopedJavaSurfaceControl surface_control);
~SurfaceRecord();
SurfaceRecord(SurfaceRecord&&);
SurfaceRecord(const SurfaceRecord&) = delete;
JavaSurfaceVariant surface_variant;
bool can_be_used_with_surface_control = false;
};
JavaSurfaceVariant AcquireJavaSurface(
gpu::SurfaceHandle surface_handle,
bool* can_be_used_with_surface_control) override;
SurfaceRecord AcquireJavaSurface(gpu::SurfaceHandle surface_handle) override;
// Gets the global instance of the surface tracker.
static GpuSurfaceTracker* Get() { return GetInstance(); }

@ -45,11 +45,10 @@ scoped_refptr<gl::Presenter> ImageTransportSurface::CreatePresenter(
DCHECK_NE(surface_handle, kNullSurfaceHandle);
// On Android, the surface_handle is the id of the surface in the
// GpuSurfaceTracker/GpuSurfaceLookup
bool can_be_used_with_surface_control = false;
auto surface_variant = GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(
surface_handle, &can_be_used_with_surface_control);
auto surface_record =
GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(surface_handle);
if (!can_be_used_with_surface_control) {
if (!surface_record.can_be_used_with_surface_control) {
return nullptr;
}
@ -70,7 +69,7 @@ scoped_refptr<gl::Presenter> ImageTransportSurface::CreatePresenter(
std::move(surface_control),
base::SingleThreadTaskRunner::GetCurrentDefault());
}},
std::move(surface_variant));
std::move(surface_record.surface_variant));
return presenter;
}
@ -102,15 +101,15 @@ scoped_refptr<gl::GLSurface> ImageTransportSurface::CreateNativeGLSurface(
// On Android, the surface_handle is the id of the surface in the
// GpuSurfaceTracker/GpuSurfaceLookup
bool can_be_used_with_surface_control = false;
auto surface_variant = GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(
surface_handle, &can_be_used_with_surface_control);
if (!absl::holds_alternative<gl::ScopedJavaSurface>(surface_variant)) {
auto surface_record =
GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(surface_handle);
if (!absl::holds_alternative<gl::ScopedJavaSurface>(
surface_record.surface_variant)) {
LOG(WARNING) << "Expected Java Surface";
return nullptr;
}
gl::ScopedJavaSurface& scoped_java_surface =
absl::get<gl::ScopedJavaSurface>(surface_variant);
absl::get<gl::ScopedJavaSurface>(surface_record.surface_variant);
gl::ScopedANativeWindow window(scoped_java_surface);
if (!window) {

@ -55,19 +55,18 @@ void MojoAndroidOverlay::OnSurfaceReady(uint64_t surface_key) {
received_surface_ = true;
// Get the surface and notify our client.
bool can_be_used_with_surface_control = false;
auto surface_variant =
gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(
surface_key, &can_be_used_with_surface_control);
DCHECK(!can_be_used_with_surface_control);
if (!absl::holds_alternative<gl::ScopedJavaSurface>(surface_variant)) {
auto surface_record =
gpu::GpuSurfaceLookup::GetInstance()->AcquireJavaSurface(surface_key);
DCHECK(!surface_record.can_be_used_with_surface_control);
if (!absl::holds_alternative<gl::ScopedJavaSurface>(
surface_record.surface_variant)) {
config_.is_failed(this);
// |this| may be deleted.
return;
}
surface_ =
std::move(absl::get<gl::ScopedJavaSurface>(std::move(surface_variant)));
surface_ = std::move(absl::get<gl::ScopedJavaSurface>(
std::move(surface_record.surface_variant)));
// If no surface was returned, then fail instead.
if (surface_.IsEmpty()) {

@ -137,9 +137,8 @@ class MojoAndroidOverlayTest : public ::testing::Test {
surface_texture_ = gl::SurfaceTexture::Create(0);
surface_ = gl::ScopedJavaSurface(surface_texture_.get());
surface_key_ = gpu::GpuSurfaceTracker::Get()->AddSurfaceForNativeWidget(
gpu::GpuSurfaceTracker::SurfaceRecord(
surface_.CopyRetainOwnership(),
false /* can_be_used_with_surface_control */));
gpu::SurfaceRecord(surface_.CopyRetainOwnership(),
false /* can_be_used_with_surface_control */));
mock_provider_.client_->OnSurfaceReady(surface_key_);
base::RunLoop().RunUntilIdle();