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

Disable a11y on detach #14142

Merged
dnfield merged 1 commit into from
Dec 5, 2019
Merged

Disable a11y on detach #14142

dnfield merged 1 commit into from
Dec 5, 2019

Conversation

@dnfield
Copy link
Contributor

@dnfield dnfield commented Dec 5, 2019 •
edited
Loading

Fixes flutter/flutter#46029
Fixes flutter/flutter#31139
Fixes flutter/flutter#33173
Fixes flutter/flutter#45218

When we detach from the engine, we destroy the a11y bridge (which is right, it holds view refs that won't be valid when we reattach). We have to tell the framework that we're destroying all our a11y state on the native side - the only way to do that is to tell the framework that a11y is off. Once we build the bridge again, it will tell the framework that a11y is back on.

The linked bugs are all cases where we get the bridge into a bad state because it thinks it has information it no longer does (because the framework thinks it already sent nodes the bridge knows nothing about, and really might not even be valid anymore because any native backing they have is gone).

Another reason to disable in this case: Even if a11y is enabled, we have no bridge. If the framework sends us a11y messages, they'll get missed by the bridge.

@auto-assign auto-assign bot requested a review from gaaclarke December 5, 2019 08:20
@dnfield
Copy link
Contributor Author

dnfield commented Dec 5, 2019

/cc @tvolkert - if there's any way we can include this as a hotfix, it fixes a bunch of in-the-wild crashes that devs tend to miss because they don't test with a11y turned on.

@dnfield
Copy link
Contributor Author

dnfield commented Dec 5, 2019

/cc @darrenaustin as well, who has been cc'd on a number of bugs related to this.

@Hixie
Copy link
Contributor

Hixie commented Dec 5, 2019

We've been suffering with this forever, if it wasn't a stop ship before, it presumably shouldn't be now. (hot fixes are very risky as we have minimal testing on the branches)

@dnfield
Copy link
Contributor Author

dnfield commented Dec 5, 2019

It's not worth creating a hotfix by itself, but if we end up with another hot fix this would ideally be included.

This problem will get worse as well, since we're now using the new embedding by default - and this problem is specific to the new embedding.

@tvolkert
Copy link
Contributor

tvolkert commented Dec 5, 2019

There will be another hotfix (there was another reported release blocker), and as long as this lands soon, we could still let it bake on beta for some time.

goderbauer
goderbauer approved these changes Dec 5, 2019
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@dnfield
Copy link
Contributor Author

dnfield commented Dec 5, 2019

Going to land on red because red was a flake.

@dnfield dnfield merged commit 73da385 into flutter:master Dec 5, 2019
@dnfield dnfield deleted the disable_a11y_on_detach branch December 5, 2019 19:16
@tvolkert tvolkert mentioned this pull request Dec 5, 2019
Merged
tvolkert added a commit that referenced this pull request Dec 5, 2019
@tvolkert
v1.12.13+hotfix.3 cherry-picks (#14147)
ac93919 
* 73da385 Disable a11y on detach (#14142)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 5, 2019
@engine-flutter-autoroll
73da385 Disable a11y on detach (flutter/engine#14142)
c652a2b
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 5, 2019
@engine-flutter-autoroll
73da385 Disable a11y on detach (flutter/engine#14142)
2c2278f
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 5, 2019
@engine-flutter-autoroll
73da385 Disable a11y on detach (flutter/engine#14142)
a270bf9
@mayurdhurpate
Copy link

mayurdhurpate commented Dec 20, 2019

Has this landed on stable @dnfield? I hope this solves this crash on my Google Play Console faced by many users on our production app:

  #00  pc 000000000006d958  /system/lib64/libc.so (tgkill+8)
  #01  pc 000000000006ad78  /system/lib64/libc.so (pthread_kill+64)
  #02  pc 0000000000024098  /system/lib64/libc.so (raise+24)
  #03  pc 000000000001c93c  /system/lib64/libc.so (abort+52)
  #04  pc 00000000004320b0  /system/lib64/libart.so (_ZN3art7Runtime5AbortEv+360)
  #05  pc 00000000000e5578  /system/lib64/libart.so (_ZN3art10LogMessageD2Ev+1208)
  #06  pc 000000000024c2b8  /system/lib64/libart.so (_ZN3art22IndirectReferenceTable3AddEjPNS_6mirror6ObjectE+308)
  #07  pc 0000000000397ddc  /system/lib64/libart.so (_ZN3artL21Reference_getReferentEP7_JNIEnvP8_jobject+84)
  #08  pc 00000000005f0720  /system/framework/arm64/boot.oat (offset 0x5a1000) (java.lang.ref.Reference.getReferent+124)
  #09  pc 00000000005f086c  /system/framework/arm64/boot.oat (offset 0x5a1000) (java.lang.ref.Reference.get+40)
  #10  pc 0000000002522634  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (android.view.FrameMetricsObserver.notifyDataAvailable+64)
  #11  pc 00000000000d23b4  /system/lib64/libart.so (art_quick_invoke_stub+580)
  #12  pc 00000000000df07c  /system/lib64/libart.so (_ZN3art9ArtMethod6InvokeEPNS_6ThreadEPjjPNS_6JValueEPKc+208)
  #13  pc 000000000042b9f0  /system/lib64/libart.so (_ZN3artL18InvokeWithArgArrayERKNS_33ScopedObjectAccessAlreadyRunnableEPNS_9ArtMethodEPNS_8ArgArrayEPNS_6JValueEPKc+108)
  #14  pc 000000000042cf98  /system/lib64/libart.so (_ZN3art35InvokeVirtualOrInterfaceWithVarArgsERKNS_33ScopedObjectAccessAlreadyRunnableEP8_jobjectP10_jmethodIDSt9__va_list+388)
  #15  pc 0000000000334628  /system/lib64/libart.so (_ZN3art3JNI15CallVoidMethodVEP7_JNIEnvP8_jobjectP10_jmethodIDSt9__va_list+620)
  #16  pc 00000000000a8e54  /system/lib64/libandroid_runtime.so
  #17  pc 00000000000ec194  /system/lib64/libandroid_runtime.so (_ZN7android13NotifyHandler13handleMessageERKNS_7MessageE+304)
  #18  pc 00000000000182ec  /system/lib64/libutils.so (_ZN7android6Looper9pollInnerEi+764)
  #19  pc 0000000000017f30  /system/lib64/libutils.so (_ZN7android6Looper8pollOnceEiPiS1_PPv+60)
  #20  pc 00000000000f35b0  /system/lib64/libandroid_runtime.so (_ZN7android18NativeMessageQueue8pollOnceEP7_JNIEnvP8_jobjecti+48)
  #21  pc 0000000002209620  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (android.os.MessageQueue.nativePollOnce+140)
  #22  pc 000000000220bab0  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (android.os.MessageQueue.next+236)
  #23  pc 00000000022044fc  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (android.os.Looper.loop+440)
  #24  pc 00000000019dd66c  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (android.app.ActivityThread.main+920)
  #25  pc 00000000000d2668  /system/lib64/libart.so (art_quick_invoke_static_stub+600)
  #26  pc 00000000000df0ac  /system/lib64/libart.so (_ZN3art9ArtMethod6InvokeEPNS_6ThreadEPjjPNS_6JValueEPKc+256)
  #27  pc 000000000042b9f0  /system/lib64/libart.so (_ZN3artL18InvokeWithArgArrayERKNS_33ScopedObjectAccessAlreadyRunnableEPNS_9ArtMethodEPNS_8ArgArrayEPNS_6JValueEPKc+108)
  #28  pc 000000000042d55c  /system/lib64/libart.so (_ZN3art12InvokeMethodERKNS_33ScopedObjectAccessAlreadyRunnableEP8_jobjectS4_S4_m+1188)
  #29  pc 00000000003a4c0c  /system/lib64/libart.so (_ZN3artL13Method_invokeEP7_JNIEnvP8_jobjectS3_S3_+56)
  #30  pc 0000000000671488  /system/framework/arm64/boot.oat (offset 0x5a1000) (java.lang.reflect.Method.invoke+180)
  #31  pc 000000000293e150  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run+124)
  #32  pc 000000000293fb34  /system/framework/arm64/boot-framework.oat (offset 0x191d000) (com.android.internal.os.ZygoteInit.main+2400)
  #33  pc 00000000000d2668  /system/lib64/libart.so (art_quick_invoke_static_stub+600)
  #34  pc 00000000000df0ac  /system/lib64/libart.so (_ZN3art9ArtMethod6InvokeEPNS_6ThreadEPjjPNS_6JValueEPKc+256)
  #35  pc 000000000042b9f0  /system/lib64/libart.so (_ZN3artL18InvokeWithArgArrayERKNS_33ScopedObjectAccessAlreadyRunnableEPNS_9ArtMethodEPNS_8ArgArrayEPNS_6JValueEPKc+108)
  #36  pc 000000000042b648  /system/lib64/libart.so (_ZN3art17InvokeWithVarArgsERKNS_33ScopedObjectAccessAlreadyRunnableEP8_jobjectP10_jmethodIDSt9__va_list+380)
  #37  pc 00000000003466d0  /system/lib64/libart.so (_ZN3art3JNI21CallStaticVoidMethodVEP7_JNIEnvP7_jclassP10_jmethodIDSt9__va_list+600)
  #38  pc 00000000000a2090  /system/lib64/libandroid_runtime.so
  #39  pc 00000000000a46e8  /system/lib64/libandroid_runtime.so (_ZN7android14AndroidRuntime5startEPKcRKNS_6VectorINS_7String8EEEb+708)
  #40  pc 000000000000225c  /system/bin/app_process64
  #41  pc 000000000001a5d4  /system/lib64/libc.so (__libc_init+88)
  #42  pc 0000000000001cc8  /system/bin/app_process64

I was unable to locate this on Crashylytics and it kept showing on Google Play Console. I have many similar-looking crash logs with minor changes. Is there a way to check if they were related to the this issue (#14142) by looking at these logs?

@dnfield
Copy link
Contributor Author

dnfield commented Dec 20, 2019

This should be present in the latest hotfix release

filmil pushed a commit to filmil/engine that referenced this pull request Mar 13, 2020
@dnfield @filmil
Disable a11y on detach (flutter#14142)
dc18459
@dnfield dnfield mentioned this pull request Mar 31, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@xster xster Awaiting requested review from xster

@matthew-carroll matthew-carroll Awaiting requested review from matthew-carroll

2 more reviewers

@goderbauer goderbauer goderbauer approved these changes

@gaaclarke gaaclarke gaaclarke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

7 participants

@dnfield @Hixie @tvolkert @mayurdhurpate @goderbauer @gaaclarke @googlebot