1372040 - Make Fennec support AdaptiveIconDrawable in Android O

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Data Providers, defect)

Description

[Gabriel Chadwick]

Since updating to Android O Firefox crashes when taping the three dots share button after a long tap on a link.

https://crash-stats.mozilla.com/report/index/f95f11cf-221e-421f-99ac-e3fea0170611

The behavior is somewhat intermittent.

Using Firefox 53.0.2

Comment 1

[Gabriel Chadwick]

Tried with Nightly
https://crash-stats.mozilla.com/report/index/c25aeb9a-3c5f-4ab6-9a69-cef000170611

Comment 2

[Teng-pao Yu [:tyu]]

The crash is at:

            Bitmap bitmap = ((BitmapDrawable) item.getIcon()).getBitmap();

There are two paths setting the item.icon (which is a drawable) and neither of the two paths guarantees the Drawable being a BitmapDrawable (both uses the Drawable abstract class only.)

One source of the Drawable is https://android.googlesource.com/platform/frameworks/support/+/master/v7/appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java#189 [PromptListItem.constructor] and another is https://developer.android.com/reference/android/content/pm/ResolveInfo.html#loadIcon(android.content.pm.PackageManager) [PromptListItem.setIcon()]

AdaptiveIconDrawable is a new Android O implementation of Drawable

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Looking at the crash report, the number of crashes seems to be low after bug 1378617. But we can find a way to make some enhancement here.

Comment 4

[Nevin Chen(Not active on Bugzilla)]

Attachment: AdaptiveIconDrawable.png

Hi Ioana
Please help test it again using Nightly. I've tested this using Android O and there's no crash.
bug 1378617 should already fix this issue. 



In Android O, Gmail, Messages, Google Keep are using AdaptiveIconDrawable, and we currently don't support api 26+ (see comment 2). So we just show blank see the attachment and [1]. 

[1] https://searchfox.org/mozilla-central/rev/f2a1911ad310bf8651f342d719e4f4ca0a7b9bfb/mobile/android/base/java/org/mozilla/gecko/prompts/PromptListAdapter.java#129

I'll change the title for this bug and make sure we should support AdaptiveIconDrawable after we move to API 26

Comment 5

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1372040 - Temporary showing large icons for AdaptiveIconDrawable.

I don't want to call BitmapUtils.getBitmapFromDrawable() here for AdaptiveIconDrawable cause there might be performance impact if I create bitmap in main thread.
I'll use bug 1397174 to follow up this issue.

Review commit: https://reviewboard.mozilla.org/r/176594/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176594/

Comment 6

[Max Liu [:maliu]]

Comment on attachment 8904916 [details]
Bug 1372040 - Temporary showing large icons for AdaptiveIconDrawable.

https://reviewboard.mozilla.org/r/176594/#review181710

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd75dcec7da1
Temporary showing large icons for AdaptiveIconDrawable. r=maliu

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dd75dcec7da1

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 10

[Nevin Chen(Not active on Bugzilla)]

Hi Ioana 
Could you please help verify if it's fixed in Nightly? Thank you!

Comment 11

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8904916 [details]
Bug 1372040 - Temporary showing large icons for AdaptiveIconDrawable.

Approval Request Comment
[Feature/Bug causing the regression]: Android's new OS version : O cause this problem
[User impact if declined]: Although there's no crash now, the user will see blank icon. 
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]:  no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: cause it just skips scaling image. Just use it as is.
[String changes made/needed]:

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8904916 [details]
Bug 1372040 - Temporary showing large icons for AdaptiveIconDrawable.

Not a lot of time left in beta, let's uplift now for beta 11 and verify the fix there.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/df29a08ea431

Comment 14

[u549602]

Could not reproduce this on the latest Nightly build 57.0a1 nor on the latest beta build 56.0b11.
This issue was tested on a Nexus 6P (Android 8.0.0)