DroidKaigi / conference-app-2018

The Official Conference App for DroidKaigi 2018 Tokyo
Apache License 2.0
1.35k stars 332 forks source link

Fixed a bug that crashes when facebook does not exist #557

Closed charcoJP closed 6 years ago

charcoJP commented 6 years ago

Issue

Overview (Required)

stackTrace

io.github.droidkaigi.confsched2018.debug E/AndroidRuntime: FATAL EXCEPTION: main
                              Process: io.github.droidkaigi.confsched2018.debug, PID: 7908
                              java.lang.NullPointerException: Attempt to read from field 'android.content.pm.ActivityInfo android.content.pm.ResolveInfo.activityInfo' on a null object reference
                                  at io.github.droidkaigi.confsched2018.presentation.NavigationController.navigateToExternalBrowser(NavigationController.kt:171)
                                  at io.github.droidkaigi.confsched2018.presentation.about.AboutThisAppFragment$onAboutThisHeaderIconClickListener$1.invoke(AboutThisAppFragment.kt:25)
                                  at io.github.droidkaigi.confsched2018.presentation.about.AboutThisAppFragment$onAboutThisHeaderIconClickListener$1.invoke(AboutThisAppFragment.kt:19)
                                  at io.github.droidkaigi.confsched2018.presentation.about.item.AboutThisAppHeaderItem$bind$1.onClick(AboutThisAppHeaderItem.kt:26)
                                  at android.view.View.performClick(View.java:5657)
                                  at android.view.View$PerformClick.run(View.java:22314)
                                  at android.os.Handler.handleCallback(Handler.java:751)
                                  at android.os.Handler.dispatchMessage(Handler.java:95)
                                  at android.os.Looper.loop(Looper.java:241)
                                  at android.app.ActivityThread.main(ActivityThread.java:6217)
                                  at java.lang.reflect.Method.invoke(Native Method)
                                  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865)
                                  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755)
io.github.droidkaigi.confsched2018.debug I/CrashlyticsCore: Crashlytics report upload complete: 5A731D59027A-0001-1EE4-7A35EC3832C2

Links

-

Screenshot

Before After
takahirom commented 6 years ago

Yes. I made that bug 😭

takahirom commented 6 years ago

I think there is a smarter correction method, but I could not think of it😭

Me too 😭

takahirom commented 6 years ago

How about this?


    fun navigateToExternalBrowser(url: String) {
        val customTabsPackageName = CustomTabsHelper.getPackageNameToUse(activity)
        if (tryLaunchingSpecificApp(url, customTabsPackageName)) {
            return
        }

        val customTabsIntent = CustomTabsIntent.Builder()
                .setShowTitle(true)
                .setToolbarColor(ContextCompat.getColor(activity, R.color.primary))
                .build()
                .apply {
                    val referrer = Uri.parse("android-app://${activity.packageName}")
                    intent.putExtra(Intent.EXTRA_REFERRER, referrer)
                }
        val webUri = Uri.parse(url)
        if (tryUsingCustomTabs(customTabsPackageName, customTabsIntent, webUri)) {
            return
        }

        // Cannot use custom tabs.
        activity.startActivity(customTabsIntent.intent.setData(webUri))
    }

    private fun tryUsingCustomTabs(customTabsPackageName: String?, customTabsIntent: CustomTabsIntent, webUri: Uri?): Boolean {
        customTabsPackageName?.let {
            customTabsIntent.intent.`package` = customTabsPackageName
            customTabsIntent.launchUrl(activity, webUri)
            return true
        }
        return false
    }

    private fun tryLaunchingSpecificApp(url:String, customTabsPackageName: String?): Boolean {
        val appUri = run {
            val uri = Uri.parse(url)
            if (uri.host.contains("facebook")) {
                return@run Uri.parse(FACEBOOK_SCHEME + url)
            }
            uri
        }
        val appIntent = Intent(Intent.ACTION_VIEW, appUri)
        val intentResolveInfo = activity.packageManager.resolveActivity(
                appIntent,
                PackageManager.MATCH_DEFAULT_ONLY
        )

        intentResolveInfo?.activityInfo?.packageName?.let {
            if (customTabsPackageName != null && it != customTabsPackageName) {
                // Open specific app
                activity.startActivity(appIntent)
                return true
            }
        }
        return false
    }
charcoJP commented 6 years ago

@takahirom Thanks! That's pretty much better!

charcoJP commented 6 years ago

maybe, Using let makes it easier to read.

before

        val appUri = run {
            val uri = Uri.parse(url)
            if (uri.host.contains("facebook")) {
                return@run Uri.parse(FACEBOOK_SCHEME + url)
            }
            uri
        }

after

        val appUri = Uri.parse(url).let {
            if (it.host.contains("facebook")) {
                Uri.parse(FACEBOOK_SCHEME + url)
            } else it
        }
takahirom commented 6 years ago

LGTM 👍 Thank you for fixing bug and refactoring 🐞 🏭

charcoJP commented 6 years ago

@takahirom Thank you for review!✨