Skip to content

Extend VirtualDisplay shadow support down to SDK 28 #9339

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

Jump to bottom
Merged
merged 1 commit into from
Jul 30, 2024
Merged

Extend VirtualDisplay shadow support down to SDK 28 #9339

merged 1 commit into from
Jul 30, 2024

Conversation

@jselbo
Copy link
Contributor

@jselbo jselbo commented Jul 22, 2024

194ac08 introduced shadow support for virtual displays but the IDisplayManager#createVirtualDisplay proxy signature only works for SDK 30+. This adds a signature for pre-SDK 30.

Overview

I found when integrating Robo 4.13 that several tests using virtual displays on SDK 28 began failing with:

java.lang.NullPointerException
	at java.base/java.util.Objects.requireNonNull(Objects.java:209)
	at java.base/java.util.TreeMap.getEntry(TreeMap.java:345)
	at java.base/java.util.TreeMap.get(TreeMap.java:279)
	at org.robolectric.shadows.ShadowDisplayManagerGlobal$DisplayManagerProxyDelegate.resizeVirtualDisplay(ShadowDisplayManagerGlobal.java:229)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.robolectric.util.ReflectionHelpers.lambda$createDelegatingProxy$2(ReflectionHelpers.java:104)
	at jdk.proxy3/jdk.proxy3.$Proxy36.resizeVirtualDisplay(Unknown Source)
	at android.hardware.display.DisplayManagerGlobal.resizeVirtualDisplay(DisplayManagerGlobal.java:449)
	at android.hardware.display.VirtualDisplay.resize(VirtualDisplay.java:90)
... (omitted)

The root of the issue is that the createVirtualDisplay proxy was not intercepting due to signature mis-match, but the resizeVirtualDisplay proxy was (and virtualDisplayIds was never populated).

Proposed Changes

Add a pre-SDK-30 compatible signature for IDisplayManager#createVirtualDisplay in the proxy. Update the tests to verify from SDK 28 onwards.

@hoisie
Copy link
Contributor

hoisie commented Jul 26, 2024

Thanks, I will run some checks to verify that this doesn't cause any compatibility issues.

@hoisie
Copy link
Contributor

hoisie commented Jul 26, 2024 •
edited
Loading

I am seeing this test failure when running the test in a bazel environment in Android P (28) and Q (29):

java.lang.RuntimeException: Stub!
	at android.hardware.display.VirtualDisplayConfig$Builder.__constructor__(VirtualDisplayConfig.java)
	at android.hardware.display.VirtualDisplayConfig$Builder.<init>(VirtualDisplayConfig.java:60)
	at org.robolectric.shadows.ShadowDisplayManagerGlobalTest.createVirtualDisplay(ShadowDisplayManagerGlobalTest.java:126)
	at org.robolectric.shadows.ShadowDisplayManagerGlobalTest.testVirtualDisplay_setSurfaceTogglesState(ShadowDisplayManagerGlobalTest.java:109)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:61)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:308)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:488)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:294)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:104)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1570)

hoisie
hoisie reviewed Jul 26, 2024
View reviewed changes
robolectric/src/test/java/org/robolectric/shadows/ShadowDisplayManagerGlobalTest.java Outdated
private VirtualDisplay createVirtualDisplay(Surface surface) {
VirtualDisplayConfig config =
new VirtualDisplayConfig.Builder("name", DISPLAY_WIDTH, DISPLAY_HEIGHT, DISPLAY_DPI)
.setSurface(surface)
Copy link
Contributor

Choose a reason for hiding this comment

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

The line of code that creates a VirtualDisplayConfig can only be run on SDK >= R.

@hoisie
Copy link
Contributor

hoisie commented Jul 26, 2024

It seems like currently the Robolectric android-all jar from U exists on the runtime classpath when running Robolectric's tests on GitHub, so the VirtualDisplayConfig is coming from Android U android-all jar, even though it doesn't exist in SDK 28 and 29. This is why tests are currently passing in this PR, I filed #9357.

@jselbo
Copy link
Contributor Author

jselbo commented Jul 26, 2024

It seems like currently the Robolectric android-all jar from U exists on the runtime classpath when running Robolectric's tests on GitHub, so the VirtualDisplayConfig is coming from Android U android-all jar, even though it doesn't exist in SDK 28 and 29. This is why tests are currently passing in this PR, I filed #9357.

Thanks, nice catch! Updating ...

utzcoz
utzcoz reviewed Jul 27, 2024
View reviewed changes
robolectric/src/test/java/org/robolectric/shadows/ShadowDisplayManagerGlobalTest.java Outdated
}
}

@TargetApi(R)
Copy link
Member

Choose a reason for hiding this comment

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

IIRC, AndroidX recommends to use RequiresApi than TargeApi.

Extend VirtualDisplay shadow support down to SDK 28
a8b859f 
194ac08 introduced shadow support for virtual displays
but the `IDisplayManager#createVirtualDisplay` proxy signature only works for SDK 30+. This adds
a signature for pre-SDK 30.
@jselbo
Copy link
Contributor Author

jselbo commented Jul 29, 2024

To be clear, I reran the ShadowDisplayManagerGlobalTest tests with e4a7f47 locally applied to verify they still work without the latest SDK being on the classpath

@hoisie
Copy link
Contributor

hoisie commented Jul 30, 2024

Thanks!

@hoisie hoisie merged commit 89cd245 into robolectric:master Jul 30, 2024
@jselbo jselbo deleted the virtualdisplay branch July 30, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hoisie hoisie hoisie left review comments

@utzcoz utzcoz utzcoz left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jselbo @hoisie @utzcoz