Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Android] Convert int in Dart to long instead of int in java. #39476

Closed
wants to merge 2 commits into from
Closed

[Android] Convert int in Dart to long instead of int in java. #39476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a081806
[Android] Convert `int` in Dart to `long` instead of `int` in java.
0xZOne Feb 8, 2023
ccfedc8
Makes | platformViewId | large enough to require a long.
0xZOne Feb 20, 2023
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
2 changes: 1 addition & 1 deletion shell/platform/android/android_context_gl_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI {
MOCK_METHOD1(SurfaceTextureDetachFromGLContext,
void(JavaLocalRef surface_texture));
MOCK_METHOD8(FlutterViewOnDisplayPlatformView,
void(int view_id,
void(int64_t view_id,
int x,
int y,
int width,
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/android/android_shell_holder_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class MockPlatformViewAndroidJNI : public PlatformViewAndroidJNI {
MOCK_METHOD1(SurfaceTextureDetachFromGLContext,
void(JavaLocalRef surface_texture));
MOCK_METHOD8(FlutterViewOnDisplayPlatformView,
void(int view_id,
void(int64_t view_id,
int x,
int y,
int width,
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,7 @@ private native void nativeDeferredComponentInstallFailure(
// @SuppressWarnings("unused")
@UiThread
public void onDisplayPlatformView(
int viewId,
long viewId,
int x,
int y,
int width,
Expand Down
38 changes: 19 additions & 19 deletions shell/platform/android/io/flutter/embedding/engine/systemchannels/PlatformViewsChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class PlatformViewsChannel {
private final MethodChannel channel;
private PlatformViewsHandler handler;

public void invokeViewFocused(int viewId) {
public void invokeViewFocused(long viewId) {
if (channel == null) {
return;
}
Expand Down Expand Up @@ -93,7 +93,7 @@ private void create(@NonNull MethodCall call, @NonNull MethodChannel.Result resu
if (usesPlatformViewLayer) {
final PlatformViewCreationRequest request =
new PlatformViewCreationRequest(
(int) createArgs.get("id"),
((Number) createArgs.get("id")).longValue(),
(String) createArgs.get("viewType"),
0,
0,
Expand All @@ -116,7 +116,7 @@ private void create(@NonNull MethodCall call, @NonNull MethodChannel.Result resu
.TEXTURE_WITH_VIRTUAL_FALLBACK;
final PlatformViewCreationRequest request =
new PlatformViewCreationRequest(
(int) createArgs.get("id"),
((Number) createArgs.get("id")).longValue(),
(String) createArgs.get("viewType"),
createArgs.containsKey("top") ? (double) createArgs.get("top") : 0.0,
createArgs.containsKey("left") ? (double) createArgs.get("left") : 0.0,
Expand Down Expand Up @@ -144,7 +144,7 @@ private void create(@NonNull MethodCall call, @NonNull MethodChannel.Result resu

private void dispose(@NonNull MethodCall call, @NonNull MethodChannel.Result result) {
Map<String, Object> disposeArgs = call.arguments();
int viewId = (int) disposeArgs.get("id");
long viewId = ((Number) disposeArgs.get("id")).longValue();

try {
handler.dispose(viewId);
Expand All @@ -158,7 +158,7 @@ private void resize(@NonNull MethodCall call, @NonNull MethodChannel.Result resu
Map<String, Object> resizeArgs = call.arguments();
PlatformViewResizeRequest resizeRequest =
new PlatformViewResizeRequest(
(int) resizeArgs.get("id"),
((Number) resizeArgs.get("id")).longValue(),
(double) resizeArgs.get("width"),
(double) resizeArgs.get("height"));
try {
Expand All @@ -183,7 +183,7 @@ private void offset(@NonNull MethodCall call, @NonNull MethodChannel.Result resu
Map<String, Object> offsetArgs = call.arguments();
try {
handler.offset(
(int) offsetArgs.get("id"),
((Number) offsetArgs.get("id")).longValue(),
(double) offsetArgs.get("top"),
(double) offsetArgs.get("left"));
result.success(null);
Expand Down Expand Up @@ -223,7 +223,7 @@ private void touch(@NonNull MethodCall call, @NonNull MethodChannel.Result resul

private void setDirection(@NonNull MethodCall call, @NonNull MethodChannel.Result result) {
Map<String, Object> setDirectionArgs = call.arguments();
int newDirectionViewId = (int) setDirectionArgs.get("id");
long newDirectionViewId = ((Number) setDirectionArgs.get("id")).longValue();
int direction = (int) setDirectionArgs.get("direction");

try {
Expand All @@ -235,7 +235,7 @@ private void setDirection(@NonNull MethodCall call, @NonNull MethodChannel.Resul
}

private void clearFocus(@NonNull MethodCall call, @NonNull MethodChannel.Result result) {
int viewId = call.arguments();
long viewId = call.arguments();
try {
handler.clearFocus(viewId);
result.success(null);
Expand Down Expand Up @@ -319,7 +319,7 @@ public interface PlatformViewsHandler {
long createForTextureLayer(@NonNull PlatformViewCreationRequest request);

/** The Flutter application would like to dispose of an existing Android {@code View}. */
void dispose(int viewId);
void dispose(long viewId);

/**
* The Flutter application would like to resize an existing Android {@code View}.
Expand All @@ -334,7 +334,7 @@ void resize(
/**
* The Flutter application would like to change the offset of an existing Android {@code View}.
*/
void offset(int viewId, double top, double left);
void offset(long viewId, double top, double left);

/**
* The user touched a platform view within Flutter.
Expand All @@ -348,10 +348,10 @@ void resize(
* {@code View}, i.e., platform view.
*/
// TODO(mattcarroll): Introduce an annotation for @TextureId
void setDirection(int viewId, int direction);
void setDirection(long viewId, int direction);

/** Clears the focus from the platform view with a give id if it is currently focused. */
void clearFocus(int viewId);
void clearFocus(long viewId);

/**
* Whether the render surface of {@code FlutterView} should be converted to a {@code
Expand All @@ -376,7 +376,7 @@ public enum RequestedDisplayMode {
}

/** The ID of the platform view as seen by the Flutter side. */
public final int viewId;
public final long viewId;

/** The type of Android {@code View} to create for this platform view. */
@NonNull public final String viewType;
Expand Down Expand Up @@ -408,7 +408,7 @@ public enum RequestedDisplayMode {

/** Creates a request to construct a platform view. */
public PlatformViewCreationRequest(
int viewId,
long viewId,
@NonNull String viewType,
double logicalTop,
double logicalLeft,
Expand All @@ -430,7 +430,7 @@ public PlatformViewCreationRequest(

/** Creates a request to construct a platform view with the given display mode. */
public PlatformViewCreationRequest(
int viewId,
long viewId,
@NonNull String viewType,
double logicalTop,
double logicalLeft,
Expand All @@ -454,15 +454,15 @@ public PlatformViewCreationRequest(
/** Request sent from Flutter to resize a platform view. */
public static class PlatformViewResizeRequest {
/** The ID of the platform view as seen by the Flutter side. */
public final int viewId;
public final long viewId;

/** The new density independent width to display the platform view. */
public final double newLogicalWidth;

/** The new density independent height to display the platform view. */
public final double newLogicalHeight;

public PlatformViewResizeRequest(int viewId, double newLogicalWidth, double newLogicalHeight) {
public PlatformViewResizeRequest(long viewId, double newLogicalWidth, double newLogicalHeight) {
this.viewId = viewId;
this.newLogicalWidth = newLogicalWidth;
this.newLogicalHeight = newLogicalHeight;
Expand Down Expand Up @@ -491,7 +491,7 @@ public interface PlatformViewBufferResized {
/** The state of a touch event in Flutter within a platform view. */
public static class PlatformViewTouch {
/** The ID of the platform view as seen by the Flutter side. */
public final int viewId;
public final long viewId;

/** The amount of time that the touch has been pressed. */
@NonNull public final Number downTime;
Expand Down Expand Up @@ -525,7 +525,7 @@ public static class PlatformViewTouch {
public final long motionEventId;

public PlatformViewTouch(
int viewId,
long viewId,
@NonNull Number downTime,
@NonNull Number eventTime,
int action,
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ public InputConnection getLastInputConnection() {
* <p>This is called when a platform view is disposed to make sure we're not hanging to a stale
* input connection.
*/
public void clearPlatformViewClient(int platformViewId) {
public void clearPlatformViewClient(long platformViewId) {
if ((inputTarget.type == InputTarget.Type.VIRTUAL_DISPLAY_PLATFORM_VIEW
|| inputTarget.type == InputTarget.Type.PHYSICAL_DISPLAY_PLATFORM_VIEW)
&& inputTarget.id == platformViewId) {
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/android/io/flutter/plugin/platform/PlatformViewFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public PlatformViewFactory(@Nullable MessageCodec<Object> createArgsCodec) {
* null, or no arguments were sent from the Flutter app.
*/
@NonNull
public abstract PlatformView create(Context context, int viewId, @Nullable Object args);
public abstract PlatformView create(Context context, long viewId, @Nullable Object args);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a breaking change. Any advice?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hard-breaking every plugin that uses platform views is definitely a non-starter.

Is there some real problem that we need this change to solve? From what I see in the PR description and the issue, it's entirely a theoretical concern, rather than something that has actually caused problems. Presumably there are no Flutter apps that need more than several billion platform views over the life of the app, and I'm having a hard time imagining a scenario where some kind of non-sequential random mapping over more than the range of int would be required.

If this is all just theoretical, the easy solution would seem to be to throw if the value is too large, and document+assert on the Dart side that the value must be within the range of int.

Copy link
Contributor

Choose a reason for hiding this comment

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

(E.g., it's also the case that on the Dart side setSize on a platform view takes a Size, whose width and height are doubles, but I'm willing to bet that passing double.maxFinite for both would not end well. It's common for APIs to not accept every value that the types could allow for.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, This is not a problem in practice, and given the breaking change, I am also more inclined to the solution you proposed.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, exposing the |viewId| to the user doesn't seem to make much sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't realized this was breaking. Stuart's suggestion seems like it is a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't realized this was breaking. Stuart's suggestion seems like it is a good idea.

Sorry, that's my mistake. The breaking change is not obvious, and I forgot to mark it at the beginning.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have submitted a new PR. Please review it when you have time. Thank you.

This comment was marked as spam.

Sign in to view


/** Returns the codec to be used for decoding the args parameter of {@link #create}. */
@Nullable
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/android/io/flutter/plugin/platform/PlatformViewsAccessibilityDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ public interface PlatformViewsAccessibilityDelegate {
* there is no corresponding view.
*/
@Nullable
View getPlatformViewById(int viewId);
View getPlatformViewById(long viewId);

/** Returns true if the platform view uses virtual displays. */
boolean usesVirtualDisplay(int id);
boolean usesVirtualDisplay(long viewId);

/**
* Attaches an accessibility bridge for this platform views accessibility delegate.
Expand Down
Loading