Skip to content
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.

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

Maybe reflection is unneded here #3073

Closed
twaik opened this issue Mar 1, 2022 · 10 comments
Closed

Maybe reflection is unneded here #3073

twaik opened this issue Mar 1, 2022 · 10 comments

Comments

@twaik
Copy link

twaik commented Mar 1, 2022

Gradle allows us to add our jars to bootClasspath. At least I do not see if there is read-only or public modificator.

project.afterEvaluate {
	project.android.bootClasspath += '/path/to/classpath.jar'
}

You can try to build a library containing Android classes (parts of them) and use it in main project.

@twaik
Copy link
Author

twaik commented May 9, 2022

@twaik
Copy link
Author

twaik commented Jun 9, 2022

Shizuku uses this. Shortly: you can write some stubs for Android classes in some library and use it as compileOnly dependency inside a project. This solution will not use any kind of reflection.

@twaik
Copy link
Author

twaik commented Jun 22, 2023

Stale issue.

@twaik twaik closed this as not planned Won't fix, can't repro, duplicate, stale Jun 22, 2023
@rom1v
Copy link
Collaborator

rom1v commented Jun 22, 2023

Scrcpy already used the android framework jar for compiling. Reflection is used for hidden APIs (which have not the same signatures on all Android API versions or devices).

@twaik
Copy link
Author

twaik commented Jun 22, 2023

The sense of the issue is to not use reflection at all. It is possible to use stubs (like this) with gradle's compileOnly. In this case you are getting much more readable code. In the case if it is not possible (i.e. class is already defined in android.jar) you can use HiddenApiRefine. But this solution does not let you change field protection so reflection will be neede only there.

@rom1v
Copy link
Collaborator

rom1v commented Jun 22, 2023

But sometimes, the function just does not exist on the device (or with a different signature). If you compile against a stub, it will fail to call it and crash the scrcpy server.

Cf for example:

if (getPrimaryClipMethod == null) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q) {
getPrimaryClipMethod = manager.getClass().getMethod("getPrimaryClip", String.class);
} else {
try {
getPrimaryClipMethod = manager.getClass().getMethod("getPrimaryClip", String.class, int.class);
getMethodVersion = 0;
} catch (NoSuchMethodException e1) {
try {
getPrimaryClipMethod = manager.getClass().getMethod("getPrimaryClip", String.class, String.class, int.class);
getMethodVersion = 1;
} catch (NoSuchMethodException e2) {
try {
getPrimaryClipMethod = manager.getClass().getMethod("getPrimaryClip", String.class, String.class, int.class, int.class);
getMethodVersion = 2;
} catch (NoSuchMethodException e3) {
getPrimaryClipMethod = manager.getClass().getMethod("getPrimaryClip", String.class, int.class, String.class);
getMethodVersion = 3;
}
}
}
}
}

So you can't "link" the methods at compile time.

@twaik
Copy link
Author

twaik commented Jun 22, 2023

That happens only if you are using new API on old devices, like View.requestPointerCapture() that was added in Android 8.0 (api-level 26) and did not exist before this release. So it is possible to check android versions to avoid API incompatibility.

@rom1v
Copy link
Collaborator

rom1v commented Jun 22, 2023

Sometimes, but not always:

  • an internal method may change for the same API version (especially during alpha/beta/preview, but also after)
  • some vendors change methods, so the method is not the same on different devices even if the API version is the same (a recent example: Can no copy text from Magic5Pro to Windows #3885)

@twaik
Copy link
Author

twaik commented Jun 22, 2023

In the case if you are calling non-existing method you will get the same NoSuchMethodException. But code will be much more clear even with this try-catch sequences.

@rom1v
Copy link
Collaborator

rom1v commented Jun 22, 2023

In the case if you are calling non-existing method you will get the same NoSuchMethodException.

In that case, it's a NoSuchMethodError (not a NoSuchMethodException as with reflection).

But code will be much more clear even with this try-catch sequences.

On the positive side, the implementation of wrappers would be more readable.

On the negative side:

  • there are stubs to write and maintain (sometimes even the parameters of the methods to call have an unknown type, so they must also be stubbed…).
  • we don't see explicitly what is "safe" to call (when it's in the API) and what is not (it compiled just because we provided some stubs);
  • catching Errors (not Exceptions) is a bit ugly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants