[Android] Drops semantics query when app is not attached #52040

chunhtai · 2024-04-10T23:01:47Z

Is seems automatic test may send a11y query before the engine is attached. Add a guard to guard against it.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2024-04-10T23:01:50Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

chunhtai · 2024-04-10T23:05:32Z

shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

@@ -873,8 +873,9 @@ private native void nativeDispatchSemanticsAction(
  @UiThread
  public void setSemanticsEnabled(boolean enabled) {
    ensureRunningOnMainThread();
-    ensureAttachedToNative();
-    nativeSetSemanticsEnabled(nativeShellHolderId, enabled);
+    if (isAttached()) {


Hi @reidbaker is there a good way to test this?

Things i have tried:

make nativeSetSemanticsEnabled public and mock it using mockito.

I try creating a public method that just call nativeSetSemanticsEnabled and use spy to mock the instance.

both of them throws UnSatisfiedLink error on nativeSetSemanticsEnabled. The only way I can think of is to extends the entire FlutterJNI and override method to do nothing, but I am wondering if that is preferred or is there any other way.

Since this pr does not have a test I cant see what testing environment you are using. My guess is that if your test contains whatever dependency has nativeSetSemanticsEnabled then the issue is that your c code needs to run on an android device (or maybe with robolectric).

I also found this chromium documentation https://www.chromium.org/developers/testing/android-tests/testing-android-code-that-crosses-the-c-java-boundary/ that suggest that a spy is the correct solution.

I still can't find a good way to use spy without trigger the error. I end up override entire class a mock out the part i don't need

reidbaker · 2024-04-22T17:39:21Z

@chunhtai I am going through my pr requests and found this. I believe you are not waiting on me if that is the case let me know.

chunhtai · 2024-04-22T18:42:19Z

no, I have some other ideas to test this code. I will let you know once this pr is ready

reidbaker · 2024-06-24T17:07:24Z

follow up to see if this is ready for review.

reidbaker · 2024-08-13T14:54:54Z

Follow up again on this pr, can we close it if it is still not ready for review?

chunhtai · 2024-08-13T21:19:54Z

I have time know, will ready this pr within this week

…lutter/engine#52040)

…sions) (#154137) Manual roll requested by [email protected] flutter/engine@365b0c7...f645ca5 2024-08-26 [email protected] Revert dart to b81b344a194f (flutter/engine#54782) 2024-08-26 [email protected] [web] set the "dialog" ARIA role unconditionally (flutter/engine#54761) 2024-08-26 [email protected] Revert "Test running the macOS engine has no stray logging" (flutter/engine#54778) 2024-08-26 [email protected] clangd_check: write .clangd as part of test (flutter/engine#54766) 2024-08-26 [email protected] Roll Dart SDK from c7eb3230248a to 94c21ec02e28 (2 revisions) (flutter/engine#54776) 2024-08-26 [email protected] [Android] Drops semantics query when app is not attached (flutter/engine#52040) 2024-08-26 [email protected] [ui] fix missing color conversion in drawAtlas. (flutter/engine#54750) 2024-08-26 [email protected] Roll Skia from ce0656c1d79c to a05dbd2a60f0 (3 revisions) (flutter/engine#54774) 2024-08-26 [email protected] Roll Fuchsia Linux SDK from CVnRW3RAZuTq0lVaw... to QqRZlA9wVbeg4WoPz... (flutter/engine#54770) 2024-08-26 [email protected] [Impeller] delete impeller image class. (flutter/engine#54767) 2024-08-25 [email protected] [Impeller] more test migration. (flutter/engine#54763) 2024-08-25 [email protected] [macOS] Add TODOs to remove AppKit bug workaround (flutter/engine#54764) 2024-08-25 [email protected] Roll Fuchsia Linux SDK from 5uR6WvDV5FX-aZgKQ... to CVnRW3RAZuTq0lVaw... (flutter/engine#54765) 2024-08-24 [email protected] Roll Skia from d59df6f6f4f3 to ce0656c1d79c (2 revisions) (flutter/engine#54760) 2024-08-24 [email protected] Roll Skia from 741972846fa2 to d59df6f6f4f3 (1 revision) (flutter/engine#54758) 2024-08-24 [email protected] Roll Fuchsia Linux SDK from 0d2McFgsNwG-5r1Rb... to 5uR6WvDV5FX-aZgKQ... (flutter/engine#54757) 2024-08-24 [email protected] Roll Dart SDK from 586ba94c96a4 to c7eb3230248a (2 revisions) (flutter/engine#54755) 2024-08-24 [email protected] Roll Skia from 0086a17e0d4c to 741972846fa2 (3 revisions) (flutter/engine#54753) 2024-08-24 [email protected] Roll Dart SDK from b81b344a194f to 586ba94c96a4 (1 revision) (flutter/engine#54752) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 0d2McFgsNwG- to QqRZlA9wVbeg If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…sions) (flutter#154137) Manual roll requested by [email protected] flutter/engine@365b0c7...f645ca5 2024-08-26 [email protected] Revert dart to b81b344a194f (flutter/engine#54782) 2024-08-26 [email protected] [web] set the "dialog" ARIA role unconditionally (flutter/engine#54761) 2024-08-26 [email protected] Revert "Test running the macOS engine has no stray logging" (flutter/engine#54778) 2024-08-26 [email protected] clangd_check: write .clangd as part of test (flutter/engine#54766) 2024-08-26 [email protected] Roll Dart SDK from c7eb3230248a to 94c21ec02e28 (2 revisions) (flutter/engine#54776) 2024-08-26 [email protected] [Android] Drops semantics query when app is not attached (flutter/engine#52040) 2024-08-26 [email protected] [ui] fix missing color conversion in drawAtlas. (flutter/engine#54750) 2024-08-26 [email protected] Roll Skia from ce0656c1d79c to a05dbd2a60f0 (3 revisions) (flutter/engine#54774) 2024-08-26 [email protected] Roll Fuchsia Linux SDK from CVnRW3RAZuTq0lVaw... to QqRZlA9wVbeg4WoPz... (flutter/engine#54770) 2024-08-26 [email protected] [Impeller] delete impeller image class. (flutter/engine#54767) 2024-08-25 [email protected] [Impeller] more test migration. (flutter/engine#54763) 2024-08-25 [email protected] [macOS] Add TODOs to remove AppKit bug workaround (flutter/engine#54764) 2024-08-25 [email protected] Roll Fuchsia Linux SDK from 5uR6WvDV5FX-aZgKQ... to CVnRW3RAZuTq0lVaw... (flutter/engine#54765) 2024-08-24 [email protected] Roll Skia from d59df6f6f4f3 to ce0656c1d79c (2 revisions) (flutter/engine#54760) 2024-08-24 [email protected] Roll Skia from 741972846fa2 to d59df6f6f4f3 (1 revision) (flutter/engine#54758) 2024-08-24 [email protected] Roll Fuchsia Linux SDK from 0d2McFgsNwG-5r1Rb... to 5uR6WvDV5FX-aZgKQ... (flutter/engine#54757) 2024-08-24 [email protected] Roll Dart SDK from 586ba94c96a4 to c7eb3230248a (2 revisions) (flutter/engine#54755) 2024-08-24 [email protected] Roll Skia from 0086a17e0d4c to 741972846fa2 (3 revisions) (flutter/engine#54753) 2024-08-24 [email protected] Roll Dart SDK from b81b344a194f to 586ba94c96a4 (1 revision) (flutter/engine#54752) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from 0d2McFgsNwG- to QqRZlA9wVbeg If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

chunhtai requested a review from reidbaker April 10, 2024 23:01

github-actions bot added the platform-android label Apr 10, 2024

chunhtai commented Apr 10, 2024

View reviewed changes

reidbaker marked this pull request as draft April 11, 2024 13:59

chunhtai closed this Aug 13, 2024

chunhtai added 2 commits August 13, 2024 12:45

Drops semantics query when app is not attached

9a0f9ed

update

44a9ee9

chunhtai reopened this Aug 13, 2024

chunhtai added 2 commits August 13, 2024 16:00

update

404a55e

format

c9e5da9

chunhtai force-pushed the guard-is-attached branch from 857c00b to c9e5da9 Compare August 13, 2024 23:04

chunhtai marked this pull request as ready for review August 13, 2024 23:05

gmackall requested a review from a team August 22, 2024 18:18

reidbaker approved these changes Aug 26, 2024

View reviewed changes

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2024

auto-submit bot merged commit 965fb87 into flutter:main Aug 26, 2024
29 checks passed

engine-flutter-autoroll mentioned this pull request Aug 26, 2024

Roll Flutter Engine from 365b0c70faf5 to 965fb87daab3 (14 revisions) flutter/flutter#154127

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 26, 2024

965fb87da [Android] Drops semantics query when app is not attached (f…

799af2e

…lutter/engine#52040)

engine-flutter-autoroll mentioned this pull request Aug 26, 2024

Roll Flutter Engine from 365b0c70faf5 to e708809e4c20 (16 revisions) flutter/flutter#154129

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 26, 2024

965fb87da [Android] Drops semantics query when app is not attached (f…

bd76403

…lutter/engine#52040)

engine-flutter-autoroll mentioned this pull request Aug 26, 2024

Roll Flutter Engine from 365b0c70faf5 to 9fb55ef52150 (17 revisions) flutter/flutter#154131

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 26, 2024

965fb87da [Android] Drops semantics query when app is not attached (f…

f7dfe24

…lutter/engine#52040)

engine-flutter-autoroll mentioned this pull request Aug 26, 2024

Manual roll Flutter Engine from 365b0c70faf5 to f645ca5d8433 (19 revisions) flutter/flutter#154137

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 26, 2024

965fb87da [Android] Drops semantics query when app is not attached (f…

0da32ff

…lutter/engine#52040)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Android] Drops semantics query when app is not attached #52040

[Android] Drops semantics query when app is not attached #52040

Uh oh!

chunhtai commented Apr 10, 2024

Uh oh!

flutter-dashboard bot commented Apr 10, 2024

Uh oh!

chunhtai Apr 10, 2024

Uh oh!

reidbaker Apr 11, 2024

Uh oh!

chunhtai Aug 13, 2024

Uh oh!

reidbaker commented Apr 22, 2024

Uh oh!

chunhtai commented Apr 22, 2024

Uh oh!

reidbaker commented Jun 24, 2024

Uh oh!

reidbaker commented Aug 13, 2024

Uh oh!

chunhtai commented Aug 13, 2024

Uh oh!

Uh oh!

Uh oh!

[Android] Drops semantics query when app is not attached #52040

[Android] Drops semantics query when app is not attached #52040

Uh oh!

Conversation

chunhtai commented Apr 10, 2024

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Apr 10, 2024

Uh oh!

chunhtai Apr 10, 2024

Choose a reason for hiding this comment

Uh oh!

reidbaker Apr 11, 2024

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

reidbaker commented Apr 22, 2024

Uh oh!

chunhtai commented Apr 22, 2024

Uh oh!

reidbaker commented Jun 24, 2024

Uh oh!

reidbaker commented Aug 13, 2024

Uh oh!

chunhtai commented Aug 13, 2024

Uh oh!

Uh oh!

Uh oh!