Skip to content

cobalt: Use activity Window surface for UI rendering #11538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kjyoun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

cobalt: Use activity Window surface for UI rendering #11538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cobalt/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ protected void createContent(final Bundle savedInstanceState) {
// Set up the animation placeholder to be the SurfaceView. This disables the
// SurfaceView's 'hole' clipping during animations that are notified to the window.
mWindowAndroid.setAnimationPlaceholderView(
mShellManager.getContentViewRenderView().getSurfaceView());
mShellManager.getContentViewRenderView().getAnchorView());
mA11yHelper =
new CobaltA11yHelper(this, mShellManager.getContentViewRenderView().getSurfaceView());
new CobaltA11yHelper(this, mShellManager.getContentViewRenderView().getAnchorView());

@borongc borongc Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check if this breaks a11y.


if (mStartupUrl == null || mStartupUrl.isEmpty()) {
String[] args = getStarboardBridge().getArgs();
Expand Down
190 changes: 158 additions & 32 deletions cobalt/shell/android/java/src/dev/cobalt/shell/ContentViewRenderView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@

package dev.cobalt.shell;

import android.app.Activity;
import android.content.Context;
import android.graphics.PixelFormat;
import android.view.Surface;
import android.view.SurfaceHolder;
import android.view.MotionEvent;
import android.view.SurfaceView;
import android.view.View;
import android.view.Window;
import android.widget.FrameLayout;

import org.chromium.base.CommandLine;
import org.chromium.base.Log;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.EventForwarder;
import org.chromium.ui.base.WindowAndroid;
Expand All @@ -28,6 +32,8 @@
*/
@JNINamespace("cobalt")
public class ContentViewRenderView extends FrameLayout {
private static final String TAG = "cobalt";

@borongc borongc Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use import static dev.cobalt.util.Log.TAG;


// The native side of this object.
private long mNativeContentViewRenderView;
private WindowAndroid mWindowAndroid;
Expand All @@ -53,7 +59,12 @@ public ContentViewRenderView(Context context) {
}

protected SurfaceBridge createSurfaceBridge() {
return new SurfaceBridge();
if (CommandLine.getInstance().hasSwitch("use-window-surface-for-ui")) {

@jasonzhangxx jasonzhangxx Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is called very early in CobaltActivity.createContent() right after CommandLine is initialized. My understanding is we have to use kimono gcl to run the experiments. And please double check if the change can be properly enabled via the experiments.

Log.i(TAG, "ContentViewRenderView: created using WindowSurfaceBridge");
return new WindowSurfaceBridge();
}
Log.i(TAG, "ContentViewRenderView: created with SurfaceView");
return new SurfaceViewBridge();
}

/**
Expand All @@ -62,7 +73,8 @@ protected SurfaceBridge createSurfaceBridge() {
* @param rootWindow The {@link WindowAndroid} this render view should be linked to.
*/
public void onNativeLibraryLoaded(WindowAndroid rootWindow) {
assert !getSurfaceView().getHolder().getSurface().isValid()
assert mSurfaceBridge.getSurfaceView() == null
|| !mSurfaceBridge.getSurfaceView().getHolder().getSurface().isValid()
: "Surface created before native library loaded.";
assert rootWindow != null;
mNativeContentViewRenderView =
Expand Down Expand Up @@ -96,7 +108,10 @@ public void surfaceCreated(SurfaceHolder holder) {
// devices, where a relayout never happens. This bug is out of Chromium's
// control, but can be worked around by forcibly re-setting the visibility of
// the surface view. Otherwise, the screen stays black, and some tests fail.
getSurfaceView().setVisibility(getSurfaceView().getVisibility());
SurfaceView surfaceView = mSurfaceBridge.getSurfaceView();
if (surfaceView != null) {
surfaceView.setVisibility(surfaceView.getVisibility());
}

onReadyToRender();
}
Expand All @@ -108,7 +123,7 @@ public void surfaceDestroyed(SurfaceHolder holder) {
mNativeContentViewRenderView, ContentViewRenderView.this);
}
};
mSurfaceBridge.connect(surfaceCallback);
mSurfaceBridge.connect(surfaceCallback, rootWindow);
}

@Override
Expand All @@ -135,22 +150,12 @@ protected void onWindowVisibilityChanged(int visibility) {
}

/**
* Sets the background color of the surface view. This method is necessary because the
* background color of ContentViewRenderView itself is covered by the background of
* SurfaceView.
* @param color The color of the background.
*/
public void setSurfaceViewBackgroundColor(int color) {
if (getSurfaceView() != null) {
getSurfaceView().setBackgroundColor(color);
}
}

/**
* Gets the SurfaceView for this ContentViewRenderView
* Gets the View used for layout anchoring, animation placeholder, or accessibility (child
* SurfaceView in SurfaceView mode, or this host View in Window Surface mode).
*/
public SurfaceView getSurfaceView() {
return mSurfaceBridge.getSurfaceView();
public View getAnchorView() {
SurfaceView surfaceView = mSurfaceBridge.getSurfaceView();
return surfaceView != null ? surfaceView : this;
}

/**
Expand Down Expand Up @@ -195,47 +200,72 @@ protected SurfaceView createSurfaceView(Context context) {
return new SurfaceView(context);
}

/**
* @return whether the surface view is initialized and ready to render.
*/
public boolean isInitialized() {

@jasonzhangxx jasonzhangxx Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this function was removed, but it appears that it was unused anyway.

return getSurfaceView().getHolder().getSurface() != null;
}

/**
* Enter or leave overlay video mode.
* @param enabled Whether overlay mode is enabled.
*/
public void setOverlayVideoMode(boolean enabled) {
int format = enabled ? PixelFormat.TRANSLUCENT : PixelFormat.OPAQUE;
getSurfaceView().getHolder().setFormat(format);
mSurfaceBridge.setFormat(format);
ContentViewRenderViewJni.get().setOverlayVideoMode(
mNativeContentViewRenderView, ContentViewRenderView.this, enabled);
}

@CalledByNative
private void didSwapFrame() {
if (getSurfaceView().getBackground() != null) {
SurfaceView surfaceView = mSurfaceBridge.getSurfaceView();
if (surfaceView == null) {
// In Window Surface mode, no child SurfaceView background to clear.
return;
}

if (surfaceView.getBackground() != null) {
post(new Runnable() {
@Override
public void run() {
getSurfaceView().setBackgroundResource(0);
surfaceView.setBackgroundResource(0);
}
});
}
}

/**
* Connecting class to hold a SurfaceView.
* Connecting class to hold a surface management strategy.
*/
protected static class SurfaceBridge {
protected abstract static class SurfaceBridge {
protected abstract void initialize(ContentViewRenderView renderView);
protected abstract void connect(SurfaceHolder.Callback surfaceCallback, WindowAndroid windowAndroid);
protected abstract void disconnect();
protected abstract SurfaceView getSurfaceView();
protected abstract void setFormat(int format);
}

/**
* SurfaceBridge implementation that uses a standard SurfaceView.
* This is used for the default rendering path where a child SurfaceView is embedded
* within the ContentViewRenderView.
*
* Lifetime: Bound to the lifetime of the outer ContentViewRenderView.
* Threading: Must be called on the UI thread.
*/
protected static class SurfaceViewBridge extends SurfaceBridge {
private SurfaceView mSurfaceView;
private SurfaceHolder.Callback mSurfaceCallback;

@Override
protected SurfaceView getSurfaceView() {
return mSurfaceView;
}

@Override
protected void setFormat(int format) {
if (mSurfaceView == null) {
return;
}
mSurfaceView.getHolder().setFormat(format);
}

@Override
protected void initialize(ContentViewRenderView renderView) {
mSurfaceView = renderView.createSurfaceView(renderView.getContext());
mSurfaceView.setZOrderMediaOverlay(true);
Expand All @@ -246,17 +276,113 @@ protected void initialize(ContentViewRenderView renderView) {
mSurfaceView.setVisibility(GONE);
}

protected void connect(SurfaceHolder.Callback surfaceCallback) {
@Override
protected void connect(SurfaceHolder.Callback surfaceCallback, WindowAndroid windowAndroid) {
mSurfaceCallback = surfaceCallback;
mSurfaceView.getHolder().addCallback(mSurfaceCallback);
mSurfaceView.setVisibility(VISIBLE);
}

@Override
protected void disconnect() {
mSurfaceView.getHolder().removeCallback(mSurfaceCallback);
}
}

/**
* SurfaceBridge implementation that takes ownership of the Activity's Window surface.
* This allows direct rendering to the window surface instead of a child SurfaceView.
*
* Lifetime: Bound to the lifetime of the outer ContentViewRenderView and the associated Activity.
* Threading: Must be called on the UI thread.
*/
protected static class WindowSurfaceBridge extends SurfaceBridge {
private Window mWindow;
private SurfaceHolder mWindowSurfaceHolder;
private Integer mSurfaceFormat;

private static Window getWindow(WindowAndroid windowAndroid) {
if (windowAndroid == null || windowAndroid.getActivity() == null) {
return null;
}
Activity activity = windowAndroid.getActivity().get();
return activity != null ? activity.getWindow() : null;
}

@Override
protected void initialize(ContentViewRenderView renderView) {}

@Override
protected void connect(
SurfaceHolder.Callback surfaceCallback, WindowAndroid windowAndroid) {

@jasonzhangxx jasonzhangxx Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding a new method ContentViewRenderView.getWindow() in ContentViewRenderView and then retrieving the mWindow during the initialize() would be a cleaner, more consistent approach that aligns better with the SurfaceViewBridge initialization pattern.

mWindow = getWindow(windowAndroid);
if (mWindow == null) {
Log.w(TAG, "ContentViewRenderView: WindowSurfaceBridge connect failed: Activity or Window is null.");
return;
}

mWindow.takeSurface(new SurfaceHolder.Callback2() {
@Override
public void surfaceCreated(SurfaceHolder holder) {
mWindowSurfaceHolder = holder;
if (mSurfaceFormat != null) {
Log.i(TAG, "ContentViewRenderView: Applying pending format");
mWindowSurfaceHolder.setFormat(mSurfaceFormat);

@borongc borongc Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should mSurfaceFormat be cleaned up once it is applied?

}
surfaceCallback.surfaceCreated(holder);
}

@Override
public void surfaceChanged(
SurfaceHolder holder, int format, int width, int height) {
mWindowSurfaceHolder = holder;
surfaceCallback.surfaceChanged(holder, format, width, height);
}

@Override
public void surfaceDestroyed(SurfaceHolder holder) {
mWindowSurfaceHolder = null;
surfaceCallback.surfaceDestroyed(holder);
}

@Override
public void surfaceRedrawNeeded(SurfaceHolder holder) {
if (!(surfaceCallback instanceof SurfaceHolder.Callback2)) {
return;
}
((SurfaceHolder.Callback2) surfaceCallback).surfaceRedrawNeeded(holder);

@jasonzhangxx jasonzhangxx Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input callback is defined as SurfaceHolder.Callback, but takeSurface requires a SurfaceHolder.Callback2. While casting the surfaceCallback to SurfaceHolder.Callback2 is necessary to implement the redraw notification, please ensure that the cast here is intentional and safe.

}
});
}

@Override
protected void disconnect() {
if (mWindow != null) {
mWindow.takeSurface(null);
} else {
Log.w(TAG, "ContentViewRenderView: disconnect() is called w/o connect().");
}
mWindow = null;
mWindowSurfaceHolder = null;
mSurfaceFormat = null;
}

@Override
protected SurfaceView getSurfaceView() {
return null;
}

@Override
protected void setFormat(int format) {
mSurfaceFormat = format;
if (mWindowSurfaceHolder == null) {
Log.i(TAG, "ContentViewRenderView: surface is not ready yet. Will apply format later");
return;
}
mWindowSurfaceHolder.setFormat(format);
}
}

private EventForwarder getEventForwarder() {
if (mWebContents == null || mWebContents.isDestroyed()) {
return null;
Expand Down
Loading