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

Fix array == null error in AccessibilityBridge #35540

Merged
auto-submit merged 1 commit into from
Oct 20, 2022
Merged

Fix array == null error in AccessibilityBridge #35540

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
6 changes: 6 additions & 0 deletions shell/platform/android/io/flutter/view/AccessibilityBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2695,6 +2695,12 @@ private void updateRecursively(
if (globalTransform == null) {
globalTransform = new float[16];
}
if (transform == null) {
Copy link
Contributor

@chunhtai chunhtai Sep 30, 2022

Choose a reason for hiding this comment

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

I imagine you still need another condition to initialize it so it won't crash in release code.

I did try to compile a custom engine in release mode, but it looks like the assertion will still crash the app. @GaryQian Do you know if the assertionError will be ignored in release mode?

Copy link
Contributor Author

@GuaiYiHu GuaiYiHu Sep 30, 2022

Choose a reason for hiding this comment

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

You meant the exception will be thrown only on debug mode, right? Shall BuildConfig.DEBUG work?

Copy link
Contributor

@chunhtai chunhtai Sep 30, 2022

Choose a reason for hiding this comment

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

yeah I think BuildConfig.DEBUG is probably the best way to do this

Copy link
Contributor Author

@GuaiYiHu GuaiYiHu Sep 30, 2022

Choose a reason for hiding this comment

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

will reedit it.

if (BuildConfig.DEBUG) {
throw new AssertionError("transform has not been initialized");
}
transform = new float[16];
Copy link
Contributor

@chinmaygarde chinmaygarde Aug 25, 2022

Choose a reason for hiding this comment

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

Shouldn't this and the thing above be initialized to an identity transformation?

Copy link
Contributor

@chinmaygarde chinmaygarde Aug 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

cc @Hixie who may have context as the author of the original null check that initialized all matrix elements to zero.

Copy link
Contributor Author

@GuaiYiHu GuaiYiHu Aug 30, 2022

Choose a reason for hiding this comment

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

better than crash

Copy link
Contributor Author

@GuaiYiHu GuaiYiHu Aug 31, 2022

Choose a reason for hiding this comment

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

cc @Hixie who may have context as the author of the original null check that initialized all matrix elements to zero.

Hope merge, plz

Copy link
Contributor

@chunhtai chunhtai Sep 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This should be initialized during the SemanticsNode.updateWith. The only possible case I can think of is the framework sent a semantics node with child that is not in the update list. This feels like a bug in the framework side. Do you have a reproducible code?

}
Matrix.multiplyMM(globalTransform, 0, ancestorTransform, 0, transform, 0);

final float[] sample = new float[4];
Expand Down