[go: nahoru, domu]

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

Crash when drawing the editor on devices running Android 8 #8828

Closed
jkmassel opened this issue Dec 19, 2018 · 34 comments
Closed

Crash when drawing the editor on devices running Android 8 #8828

jkmassel opened this issue Dec 19, 2018 · 34 comments

Comments

@jkmassel
Copy link
Contributor
jkmassel commented Dec 19, 2018

It appears we're getting a crash specific to Android devices with Android 8.x, prevalently Huawei devices.

The relevant bits of the stack trace:

Fatal Exception: java.lang.ArrayIndexOutOfBoundsException: length=79; index=-19
       at android.text.DynamicLayout.getBlockIndex(DynamicLayout.java:646)
       at android.widget.Editor.drawHardwareAccelerated(Editor.java:1744)
       at android.widget.Editor.onDraw(Editor.java:1713)
       at android.widget.TextView.onDraw(TextView.java:7034)
       at android.view.View.draw(View.java:19314)
       at android.view.View.updateDisplayListIfDirty(View.java:18250)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.draw(View.java:19317)
       at android.widget.ScrollView.draw(ScrollView.java:1777)
       at android.view.View.updateDisplayListIfDirty(View.java:18250)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.draw(View.java:19317)
       at android.view.View.updateDisplayListIfDirty(View.java:18250)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.draw(View.java:19317)
       at android.support.v4.view.ViewPager.draw(ViewPager.java:2420)
       at android.view.View.updateDisplayListIfDirty(View.java:18250)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.updateDisplayListIfDirty(View.java:18241)
       at android.view.View.draw(View.java:19042)
       at android.view.ViewGroup.drawChild(ViewGroup.java:4271)
       at android.view.ViewGroup.dispatchDraw(ViewGroup.java:4054)
       at android.view.View.draw(View.java:19317)
       at com.android.internal.policy.DecorView.draw(DecorView.java:915)
       at android.view.View.updateDisplayListIfDirty(View.java:18250)
       at android.view.ThreadedRenderer.updateViewTreeDisplayList(ThreadedRenderer.java:684)
       at android.view.ThreadedRenderer.updateRootDisplayList(ThreadedRenderer.java:690)
       at android.view.ThreadedRenderer.draw(ThreadedRenderer.java:804)
       at android.view.ViewRootImpl.draw(ViewRootImpl.java:3199)
       at android.view.ViewRootImpl.performDraw(ViewRootImpl.java:2997)
       at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2526)
       at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1515)
       at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:7266)
       at android.view.Choreographer$CallbackRecord.run(Choreographer.java:981)
       at android.view.Choreographer.doCallbacks(Choreographer.java:790)
       at android.view.Choreographer.doFrame(Choreographer.java:721)
       at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:967)
       at android.os.Handler.handleCallback(Handler.java:808)
       at android.os.Handler.dispatchMessage(Handler.java:101)
       at android.os.Looper.loop(Looper.java:166)
       at android.app.ActivityThread.main(ActivityThread.java:7529)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:245)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:921)

It seems likely that this is related to #8827 – both are platform-specific Android 8 issues.

reference: 5a76a4fa8cb3c2fa634286c6-fabric
reference: 5af4233111e9fa0aa513dfa0-fabric
marked as high priority as it's happening ~35 times/day and affecting ~800 users

@jkmassel jkmassel added this to To Do in Groundskeeping via automation Dec 19, 2018
@jkmassel
Copy link
Contributor Author

@jkmassel jkmassel changed the title Crash when drawing the editor on Huawei devices running Android 8 Crash when drawing the editor on devices running Android 8 Dec 19, 2018
@SiobhyB
Copy link
Contributor
SiobhyB commented Feb 22, 2019

There's a report in 1812801-zen that seems to match up with this crash report in Fabric.

The user's device is a Samsung SM-J400M and it's running on Android 8.0.

The app crashes when they attempt to use voice dictation after uploading more than three photos at a time.

@jkmassel: Is there any extra info from the user that'd be useful for troubleshooting this further?

@charliescheer
Copy link
charliescheer commented Mar 8, 2019

Another potential report in 1861204-zen. We had the user update to the most recent version and the problem persists.

Android device name: HUAWEI GRACE
"Almost always when I want to open my blog posts, the app crashes."

@designsimply designsimply moved this from To Do to Prioritized Android in Groundskeeping Mar 15, 2019
@designsimply
Copy link
Contributor
designsimply commented Mar 15, 2019
reference: 5a76a4fa8cb3c2fa634286c6-fabric reference: 5af4233111e9fa0aa513dfa0-fabric marked as high priority as it's happening ~35 times/day and affecting ~800 users

Combining stats for the last 30 days for 5a76a4fa8cb3c2fa634286c6-fabric and 5af4233111e9fa0aa513dfa0-fabric:

Crashed ~90 times a day or 2844 times for 809 users in the past 30 days Feb 14 - Mar 14, 2019.

Crashes are trending up for 11.9 in 5af4233111e9fa0aa513dfa0-fabric.

@malinajirka malinajirka self-assigned this Mar 18, 2019
@malinajirka malinajirka moved this from Prioritized Android to In Progress in Groundskeeping Mar 18, 2019
@malinajirka
Copy link
Contributor

I can confirm it's related to
wordpress-mobile/AztecEditor-Android#729 and wordpress-mobile/AztecEditor-Android#516

Unfortunately, it's a bug in the OS which was introduced in Android 8.0 and was fixed in Android 9.0 (https://android-review.googlesource.com/c/platform/frameworks/base/+/634929).

wordpress-mobile/AztecEditor-Android#516 contains steps how to reproduce the issue.

All these crashes seems related - it also seems the crash is not specific to Huawei phones. It's specific to Android 8.0 and 8.1.
5a86f7f48cb3c2fa632d5149-fabric
5af4233111e9fa0aa513dfa0-fabric
5a76a4fa8cb3c2fa634286c6-fabric
5a04a91461b02d480d160132-fabric
5ac6aba236c7b23527a48419-fabric
5b9ef56df8b88c2963244ca6-fabric
5ae8d77a638393737aeae584-fabric
5ae6655f638393737ab63fe7-fabric

If we sum up all these crashes, it's definitely our #1. I can think of 3 options we have, but none of them is great.

  • Disable hardware acceleration for the EditText. However, imho we can't do that for Aztec as it'd have huge performance impact for all our users.
  • Copy-paste Editor.java and TextView.java from google sources for Android 8.1 -> fix the bug in the Editor.java. Create a CustomTextView in Aztec repo which would use the copy-pasted files on Android 8.0 and 8.1. This solution is hacky and it might bite us.
  • Don't do anything at all as the bug is fixed in Android 9 -> Do we know whether the bug occurs with Gutenberg?

Wdyt @daniloercoli ?

@rfaile313
Copy link
Member
rfaile313 commented Mar 18, 2019

from 1861204-zen

"It's not when trying to open a specific post, but instead when trying to view the list of posts"

@daniloercoli
Copy link
Contributor

It seems the issue is not only happening when opening the editor @malinajirka. Also when accessing the posts lists, accordingly to the report ^ above.

@jkmassel
Copy link
Contributor Author

Are we able to detect the current Android version in code, and use a different path for that?

I think either of those two solutions could be applied judiciously, perhaps as a setting? I'd imagine we could detect if a user crashed in the editor on Android 8, and prompt them on next launch to enable "compatibility mode" or whatever we want to call it that fixes the issue in v8. Then, if they later upgrade to v9, we silently disable it?

Typically for a simple bug we might not go to all this trouble, but it seems quite widespread.

WDYT @malinajirka / @daniloercoli ?

@malinajirka
Copy link
Contributor

"It's not when trying to open a specific post, but instead when trying to view the list of posts"
It seems the issue is not only happening when opening the editor @malinajirka. Also when accessing the posts lists, accordingly to the report ^ above.

I've checked the ticket and I haven't found any indication it's the same crash. I might be missing something. cc @daniloercoli

Are we able to detect the current Android version in code, and use a different path for that?

Yes.

I think either of those two solutions could be applied judiciously, perhaps as a setting? I'd imagine we could detect if a user crashed in the editor on Android 8, and prompt them on next launch to enable "compatibility mode" or whatever we want to call it that fixes the issue in v8. Then, if they later upgrade to v9, we silently disable it?

I'll try to measure the performance impact if we turn off the hardware acceleration - it's definitely the easiest and the only not-so-hacky solution.

The option 2 should be considered as a last resort solution which might not be even worth it. Especially since we don't have feature flags and we won't be able to disable the "fix" if something goes wrong - which can easily happen on custom roms. We'll definitely need to consult it with other Android devs to consider the risks.

I'm not sure we can somehow detect the app crashed.

  • If we could wrap it in try/catch block, we could also try to recover from the exception. However, since the crash doesn't happen in our code, I'm not sure we can do that.
  • We could register our own UncaughtExceptionHandler, but I'm a bit worried we'd override Fabric's handler. I'd need to look into this - we might be able to call the Fabric's handler within our handler.

However, we know it'll crash under certain circumstances on Android 8. So if we come up with a solution, we can just enable it by default on Android 8.

@daniloercoli
Copy link
Contributor

We could register our own UncaughtExceptionHandler, but I'm a bit worried we'd override Fabric's handler. I'd need to look into this - we might be able to call the Fabric's handler within our handler.

Yes I think this is a good idea. Take a look at AztecExceptionHandler since we're already doing "things" on crashes on Aztec 😬

I'll try to measure the performance impact if we turn off the hardware acceleration - it's definitely the easiest and the only not-so-hacky solution.

I guess we can turn off the hardware acceleration when we detect the crash happened on Android 8. Just store a property, and disable the HW acceleration at next startup when the prop is true and Android == 8. I'm not sure about prompt the user about it, since it will be hard to explain where to go to enable the Compatibility mode, and why it's required. Maybe we can show a prompt "Compatibility mode enabled", explain why in the message, and give some info about how to turn it off in App settings. Where we should add a toggle for it.

@jkmassel
Copy link
Contributor Author

Extension on the above idea – given that we know how the bug is triggered, is it possible to examine the contents of a post to determine whether or not the crash will happen when the post is placed in the editor? That'd allow us to avoid that initial crash by preemptively putting Aztec in compatibility mode.

WDYT?

@malinajirka
Copy link
Contributor
malinajirka commented Mar 20, 2019

@jkmassel I've looked into it and I wasn't able to come up with a solution. It's definitely a good idea and I'm not saying it's not possible to somehow do it. The code which contains the bug is just really complicated + it's buried in the Android OS so we don't have access to the data.

I spent a bit more time exploring the options I mentioned earlier

Disable HW acceleration

  • it's possible to disable the hw acceleration specifically on Android 8
  • it's possible to disable the hw acceleration specifically for the Editor screen
  • it's even possible to disable it after the user experiences the issue for the first time
  • the performance impact is significant :(. Especially while scrolling over images - it's lagging even on my Pixel 3 (high end device). I don't have any low end device with Android 8, but I think it might be completely unusable.

IMHO disabling hw acceleration is not worth it. The bug occurs just on certain posts under specific circumstances and even if we disable it just for users who are experiencing the issue, the performance hit will affect all their posts.

Copy-paste method
Unfortunately, this method won't work either :(. The classes we'd need to copy are dependant on many internal classes which are not accessible to us from the code (we'd need to copy them as well).

Even though I don't like it, I give up for now. Maybe I'll come up with some more ideas how to work around it. I still hope this bug is not present in Gutenberg and it'll become less critical in the near future.

@designsimply
Copy link
Contributor

30-day impact: ~52 times a day
Devices: Samsung Android 8, HUAWEI Android 8
Last seen in: 12.1.1
(5a76a4fa8cb3c2fa634286c6-fabric-android and 5af4233111e9fa0aa513dfa0-fabric-android)

@mzorz
Copy link
Contributor
mzorz commented Jun 7, 2019

something like UI/Application Exerciser Monkey could help in some way to automate the search for steps.

I think at this point trying this is totally worth it! I recall using this same technique back when programming for PalmOS, we can certainly do that here and see what we can find.

@designsimply
Copy link
Contributor
designsimply commented Jul 11, 2019

90-day impact: ~84 per day
Users affected in the last 90 days: 2.3k
Last seen in: 12.6
Limited to: Android 8

https://sentry.io/share/issue/9f844f6ec25f410481e73c996cc6410f/

@mkevins
Copy link
Contributor
mkevins commented Jul 18, 2019

Unassigning myself from this for now, as I'll be focused on other work in the near future. For anyone who may pick this up, my rough idea of next steps is to see if UI/Application Exerciser Monkey could reproduce the crash "route(s)". I've left this unexplored, and had only considered this approach as a last resort, conditioned on a relatively low time-cost in configuring that tool.

@mkevins mkevins removed their assignment Jul 18, 2019
@designsimply designsimply moved this from In Progress to Prioritized Android in Groundskeeping Aug 1, 2019
@designsimply
Copy link
Contributor
designsimply commented Aug 15, 2019

Found some similar issues in Sentry and merged them and here is a new count after the merge:

90-day impact: ~133 per day
Users affected in the last 90 days: 3.3k
Last seen in: 12.9
Limited to: Android 8

https://sentry.io/share/issue/9f844f6ec25f410481e73c996cc6410f/

@designsimply
Copy link
Contributor

@mzorz @mkevins would it be possible to aggressively detect folks using Aztec on the affected versions and push them to Gutenberg? Because, for this case, it feels like switching to the new editor would be a better experience than not being able to post because of a platform bug.

@mzorz
Copy link
Contributor
mzorz commented Aug 16, 2019

I think that's a definitely a possibility @designsimply - would defer the call to @hypest @daniloercoli at this point, I think if my memory doesn't fail much, @daniloercoli came up with that same approach to workaround this or other issues before, but it was kind of difficult to make the switch back then, maybe it's easier now that Gutenberg is more developed on mobile

@designsimply
Copy link
Contributor

Thanks! I will review this again with @daniloercoli at the Oct 14-18 Groundskeeping rotation.

@mkevins
Copy link
Contributor
mkevins commented Aug 19, 2019

for this case, it feels like switching to the new editor would be a better experience than not being able to post

I totally agree, and this sounds like a very sensible way to mitigate the issue.

@daniloercoli daniloercoli self-assigned this Oct 14, 2019
@daniloercoli daniloercoli moved this from Prioritized Android to In Progress in Groundskeeping Oct 14, 2019
@daniloercoli
Copy link
Contributor
daniloercoli commented Oct 15, 2019

Hey @designsimply - looking at https://sentry.io/share/issue/9f844f6ec25f410481e73c996cc6410f/ it seems that the problem is not limited to Huawei devices, but on devices running Android 8.X. Would be good to update the description of the issue with these new details.

Unfortunately i don't see a reliable way to fix this issue once and for all, since the problem is deep inside the Android platform in version 8.X. It got fixed in v9.
We already tried in a PR mentioned above in comments to go case by case, but it's hard to reproduce this issue and we may not catch all of those anyway.

would it be possible to aggressively detect folks using Aztec on the affected versions and push them to Gutenberg? Because, for this case, it feels like switching to the new editor would be a better experience than not being able to post because of a platform bug.

We're already pushing users to the new Gutenberg mobile editor (since late August), by enabling it when a post with blocks is opened in the app. So GB mobile should be used more than before and this crash should go down shortly.
Is there a way in Sentry to check if the occurrences of this crash are going down since then? I can only check 30 days back, and it seems we're doing better since September.

Possible solutions:

  • Detect the crash and disable HW acceleration only on that post. I think this is the best solution at the moment.
  • Detect the crash and force the user to GB mobile. This is a quite strong solution, since the post should be converted to blocks. GB mobile does't have the logic in place for that, and it's missing from the "Classic" block. (Btw, the Classic block will be based on Aztec, so we may have the some problem there).
    We may need to check other conditions before applying the conversion to a Blocks based post.

I propose to just keep an eye on this crash without doing anything special at the moment:

  • GB mobile V2 is not that far away
  • GB is now the default editor of WP, so less and less old style posts will be opened in the mobile app
  • Android 8 adoption should go down since we're already with Android 10 out there.

If you think that is better to propose a fix we can go down by disabling HW acceleration on a per-post basis. I'm open to other suggestions though.

@designsimply
Copy link
Contributor

Would be good to update the description of the issue with these new details.

I updated the first sentence to reflect the issue is not limited to only Huawei devices but is limited to devices running Android 8.x (and we might have started seeing the other devices when I merged Sentry issues on 2019-08-15).

Is there a way in Sentry to check if the occurrences of this crash are going down since then? I can only check 30 days back, and it seems we're doing better since September.

Sentry should show 90 days back overall—the graph at right only shows 30, the numbers at the top and overall crashes recorded are 90.

Looking at the numbers again, it looks like there are more crashes recorded for less people compared to the last time I checked.

90-day impact: ~200 per day
Users affected in the last 90 days: 2400
Limited to: Android 8.x

https://sentry.io/share/issue/9f844f6ec25f410481e73c996cc6410f/

I agree the first solution you proposed is the best route if we don't see the numbers go down

Detect the crash and disable HW acceleration only on that post.

I also checked the Statistics section of the Google Play Console for WPAndroid and found that on 2019-10-13 Android 8.0 are down 8.7% compared to the same day one month prior 2019-09-13 which is a check to show Android 8.0 usage for our app is indeed trending down (as expected and as you noted), and we should review the overall percentage of Android 8 users on our app (is that number ok to post publicly?) and use that to help determine whether to implement the solution to disable HW acceleration only on posts affected by this crash.

@designsimply
Copy link
Contributor

90-day impact: ~211 per day
Users affected in the last 90 days: 2200
Limited to: Android 8.0.0 and 8.1.0

https://sentry.io/share/issue/9f844f6ec25f410481e73c996cc6410f/

  • Android 8.0 -6.1% Nov 5, 2019 vs Oct 6, 2019
  • Android 8.1 -4.3% Nov 5, 2019 vs Oct 6, 2019

@designsimply
Copy link
Contributor
designsimply commented Nov 6, 2019

Consider closing with #10620 as the accepted solution.

@designsimply designsimply moved this from In Progress to Prioritized Android in Groundskeeping Nov 6, 2019
@malinajirka
Copy link
Contributor

Fixed in #10620. See the PR for details -> Unfortunately this PR does act after the fact, and probably we won't see crash report numbers disappear completely..

Copy link
sentry-io bot commented Feb 29, 2024

Sentry Issue: WORDPRESS-ANDROID-2D14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Groundskeeping
  
Done Nov 18-29, 2019
Development

Successfully merging a pull request may close this issue.

9 participants