Use drawable directly for split tunneling app icons

[Pururun]

Also add a loading circle instead of the missing app icon image

---

This change is 

[linear]

DROID-501 Investigate SplitTunneling Application icon reloading/performance

[Pururun]

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

private const val MAX_BITMAP_SIZE_BYTES = 100 * 1024 * 1024

fun Drawable.isBelowMaxSize(): Boolean =

I am not sure if this is needed anymore as it was a stopgap fix for a check in BitmapPainter and we are no longer using it. I think though that we should have some kind of safeguard for homongous app icons that for some reason exists.

[Rawa]

Reviewable status: 0 of 10 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I am not sure if this is needed anymore as it was a stopgap fix for a check in BitmapPainter and we are no longer using it. I think though that we should have some kind of safeguard for homongous app icons that for some reason exists.

We should verify this still works because I remember us having crashes because some app had a icon of many MB in size.

[Rawa]

Reviewed 1 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 2 of 10 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

We should verify this still works because I remember us having crashes because some app had a icon of many MB in size.

See DROID-846 for more info

[Pururun]

Reviewable status: 2 of 10 files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

See DROID-846 for more info

@Override
    protected void throwIfCannotDraw(Bitmap bitmap) {
        super.throwIfCannotDraw(bitmap);
        int bitmapSize = bitmap.getByteCount();
        if (bitmapSize > MAX_BITMAP_SIZE) {
            throw new RuntimeException(
                    "Canvas: trying to draw too large(" + bitmapSize + "bytes) bitmap.");
        }
    }

Yes here, however it is no longer called as far as I can tell?

[Rawa]

Reviewable status: 2 of 10 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

@Override
    protected void throwIfCannotDraw(Bitmap bitmap) {
        super.throwIfCannotDraw(bitmap);
        int bitmapSize = bitmap.getByteCount();
        if (bitmapSize > MAX_BITMAP_SIZE) {
            throw new RuntimeException(
                    "Canvas: trying to draw too large(" + bitmapSize + "bytes) bitmap.");
        }
    }

Yes here, however it is no longer called as far as I can tell?

Ah, you mean since we no longer us Bitmap to begin with? Yeah, could be true. I just mean we should try and reproduce the old bug as with this code. Just add a icon of 100MB + and see that it renders it fine.

[Rawa]

Reviewable status: 2 of 10 files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r2 (raw file):

private const val MAX_BITMAP_SIZE_BYTES = 100 * 1024 * 1024

fun Drawable.isBelowMaxSize(): Boolean =

We should remove the old corresponding function that existed on bitmap as well.

[Pururun]

Reviewable status: 2 of 10 files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Ah, you mean since we no longer us Bitmap to begin with? Yeah, could be true. I just mean we should try and reproduce the old bug as with this code. Just add a icon of 100MB + and see that it renders it fine.

Yes will do

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r2 (raw file):

Previously, Rawa (David Göransson) wrote…

We should remove the old corresponding function that existed on bitmap as well.

Removed

[Rawa]

Reviewed 8 of 9 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Yes will do

👍 Given this is done, I approve! Well done! 👏

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 6 at r3 (raw file):

import android.graphics.drawable.Drawable

private const val MAX_BITMAP_SIZE_BYTES = 100 * 1024 * 1024

nit: Maybe just add a comment saying it is 100MB

[Pururun]

Reviewable status: 2 of 11 files reviewed, 2 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Yes will do

Tested with a test app with a ridicolous size icon (10 000x10 000) and it did not crash! That icon did crash the app on main so I guess not converting to bitmap has some other benefits as well. I tink we should keep the bitmap drawable check just in case.

[Rawa]

Reviewed 8 of 9 files at r1, 1 of 1 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 8 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Tested with a test app with a ridicolous size icon (10 000x10 000) and it did not crash! That icon did crash the app on main so I guess not converting to bitmap has some other benefits as well. I tink we should keep the bitmap drawable check just in case.

Well done! 👏

[Pururun]

Reviewable status: complete! all files reviewed, all discussions resolved

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/util/Drawable.kt line 6 at r3 (raw file):

Previously, Rawa (David Göransson) wrote…

nit: Maybe just add a comment saying it is 100MB

Done

[Rawa]

Reviewed 1 of 1 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved

[Rawa]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved