search

mozilla-mobile / focus-android

⚠️ Firefox Focus (Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
2.11k stars 711 forks source link

Firefox Focus crashes when opening "About Firefox Focus" screen #5185

Closed cadeyrn closed 3 years ago

cadeyrn commented 3 years ago

Steps to reproduce

  1. compile Focus from the main branch
  2. open Settings > Mozilla > About Firefox Klar / Firefox Focus

Expected behavior

"About Firefox Klar" screen opens, no crash.

Actual behavior

Firefox Klar / Firefox Focus crashes:

2021-08-19 15:40:53.141 6516-6516/? W/ScreenSize: getCutoutPathDataHeight() - 
    java.lang.reflect.InvocationTargetException
        at java.lang.reflect.Method.invoke(Native Method)
        at com.oneplus.base.ScreenSize.getCutoutPathDataHeight(ScreenSize.java:235)
        at com.oneplus.base.ScreenSize.<init>(ScreenSize.java:146)
        at com.oneplus.base.ScreenSize.<init>(ScreenSize.java:74)
        at com.oneplus.camera.CameraActivity.checkScreenAndWindowSize(CameraActivity.kt:1449)
        at com.oneplus.camera.CameraActivity.access$checkScreenAndWindowSize(CameraActivity.kt:92)
        at com.oneplus.camera.CameraActivity$displayListener$1.onDisplayChanged(CameraActivity.kt:584)
        at android.hardware.display.DisplayManagerGlobal$DisplayListenerDelegate.handleMessage(DisplayManagerGlobal.java:741)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:233)
        at android.app.ActivityThread.main(ActivityThread.java:8010)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978)
     Caused by: java.lang.IllegalArgumentException: Path string cannot be empty.
        at android.util.PathParser.nParseStringForPath(Native Method)
        at android.util.PathParser.createPathFromPathData(PathParser.java:38)
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.oneplus.base.ScreenSize.getCutoutPathDataHeight(ScreenSize.java:235) 
        at com.oneplus.base.ScreenSize.<init>(ScreenSize.java:146) 
        at com.oneplus.base.ScreenSize.<init>(ScreenSize.java:74) 
        at com.oneplus.camera.CameraActivity.checkScreenAndWindowSize(CameraActivity.kt:1449) 
        at com.oneplus.camera.CameraActivity.access$checkScreenAndWindowSize(CameraActivity.kt:92) 
        at com.oneplus.camera.CameraActivity$displayListener$1.onDisplayChanged(CameraActivity.kt:584) 
        at android.hardware.display.DisplayManagerGlobal$DisplayListenerDelegate.handleMessage(DisplayManagerGlobal.java:741) 
        at android.os.Handler.dispatchMessage(Handler.java:106) 
        at android.os.Looper.loop(Looper.java:233) 
        at android.app.ActivityThread.main(ActivityThread.java:8010) 
        at java.lang.reflect.Method.invoke(Native Method) 
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631) 
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978) 

The same steps does not trigger a crash with the Firefox Focus branding from the same revision.

Update: Since #5176 Firefox Focus is affected as well.

Device information

cadeyrn commented 3 years ago

This is a regression from #5153. There is no crash in the build before the logo change. /cc @ionutbedregeanu

mcarare commented 3 years ago

HtmlLoader.loadPngAsDataURI here expects a PNG resource, and after the change, for Klar, it is a vector drawable.

Mugurell commented 3 years ago

@pocmo Would this be a good opportunity to migrate this simple screen from a data url to a Composable? (I see Focus doesn't yet have it's own RequestInterceptor. Maybe it's a good opportunity to add one also 😏)

Mugurell commented 3 years ago

Adding another png resource while we already have a svg seems wasteful. Drawing the SVG in a Bitmap to then be serialized again seems wasteful.

pocmo commented 3 years ago

@pocmo Would this be a good opportunity to migrate this simple screen from a data url to a Composable? (I see Focus doesn't yet have it's own RequestInterceptor. Maybe it's a good opportunity to add one also 😏)

In general, yeah, I think so (Is only 93/main affected?). I would like those data-URLs to go away eventually. :)

pocmo commented 3 years ago

In general, yeah, I think so (Is only 93/main affected?). I would like those data-URLs to go away eventually. :)

Although, be aware of UI tests. Mixing views and compose in a UI test is a bit weird.

sv-ohorvath commented 3 years ago

Focus crashes too in the About page: see UI test report here java.lang.IllegalStateException: Invalid png detected FATAL EXCEPTION: main Process: org.mozilla.focus.debug, PID: 7576 java.lang.IllegalStateException: Invalid png detected at org.mozilla.focus.utils.HtmlLoader.loadPngAsDataURI(HtmlLoader.java:82) at org.mozilla.focus.browser.LocalizedContent.loadAbout(LocalizedContent.kt:49) at org.mozilla.focus.engine.AppContentInterceptor.onLoadRequest(AppContentInterceptor.kt:30) at mozilla.components.browser.engine.gecko.GeckoEngineSession$createNavigationDelegate$1.maybeInterceptRequest(GeckoEngineSession.kt:570) at mozilla.components.browser.engine.gecko.GeckoEngineSession$createNavigationDelegate$1.onLoadRequest(GeckoEngineSession.kt:492) at org.mozilla.geckoview.GeckoSession.lambda$shouldLoadUri$4$GeckoSession(GeckoSession.java:1760) at org.mozilla.geckoview.GeckoSession$$ExternalSyntheticLambda2.run(Unknown Source:8) at org.mozilla.gecko.util.ThreadUtils.runOnUiThread(ThreadUtils.java:53) at org.mozilla.geckoview.GeckoSession.shouldLoadUri(GeckoSession.java:1758) at org.mozilla.geckoview.GeckoSession.load(GeckoSession.java:1700) at mozilla.components.browser.engine.gecko.GeckoEngineSession.loadUrl(GeckoEngineSession.kt:175) at mozilla.components.concept.engine.EngineSession.loadUrl$default(EngineSession.kt:566) at mozilla.components.browser.state.engine.middleware.LinkingMiddleware$performLoadOnMainThread$1.invokeSuspend(LinkingMiddleware.kt:111) at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106) at android.os.Handler.handleCallback(Handler.java:873) at android.os.Handler.dispatchMessage(Handler.java:99) at androidx.test.espresso.base.Interrogator.loopAndInterrogate(Interrogator.java:148) at androidx.test.espresso.base.UiControllerImpl.loopUntil(UiControllerImpl.java:525) at androidx.test.espresso.base.UiControllerImpl.loopUntil(UiControllerImpl.java:484) at androidx.test.espresso.base.UiControllerImpl.injectMotionEvent(UiControllerImpl.java:236) at androidx.test.espresso.action.MotionEvents.sendUp(MotionEvents.java:162) at androidx.test.espresso.action.MotionEvents.sendUp(MotionEvents.java:139) at androidx.test.espresso.action.Tap.sendSingleTap(Tap.java:170) at androidx.test.espresso.action.Tap.access$100(Tap.java:31) at androidx.test.espresso.action.Tap$1.sendTap(Tap.java:47) at androidx.test.espresso.action.GeneralClickAction.perform(GeneralClickAction.java:137) at androidx.test.espresso.ViewInteraction$SingleExecutionViewAction.perform(ViewInteraction.java:366) at androidx.test.espresso.ViewInteraction.doPerform(ViewInteraction.java:255) at androidx.test.espresso.ViewInteraction.access$100(ViewInteraction.java:65) at androidx.test.espresso.ViewInteraction$1.call(ViewInteraction.java:158) at androidx.test.espresso.ViewInteraction$1.call(ViewInteraction.java:155) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at android.os.Handler.handleCallback(Handler.java:873) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:193) at android.app.ActivityThread.main(ActivityThread.java:6669) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

cadeyrn commented 3 years ago

The Focus logo was replaced in #5176. I have updated the issue title accordingly to say Firefox Focus instead of Firefox Klar since Focus is the more common branding.

lobontiumira commented 3 years ago

Verified as fixed on the latest debug build from main, from 8/31 (GV 93.0a1-20210824094724) with Samsung Galaxy Note 8 (Android 9).