tor-browser

---

layout: page title: Decouple HomeActivity and ExternalAppBrowserActivity permalink: /rfc/0011-decouple-home-activity-and-external-app-browser-activity

---

• Start date: 2023-12-06
• RFC PR: #4732

Summary

The Custom Tabs feature is partially implemented by ExternalAppBrowserActivity and AuthCustomTabActivity. The use of inheritance in these Activitys contributes to a closely coupled system.

The aim of this RFC is to strive towards a single Activity architecture as is recommended for Modern Android Development (MAD).

The initial tasks will aim to:

1. RFC PR: #4732
2. RFC PR: #4732
3. RFC PR: #4732

⚠️ This work is exclusively a refactor of the current implementation, there should be no changes to existing behavior or features.

Motivation

Fenix has numerous open bugs and feature requests relating to the Custom Tabs feature. To facilitate efficient implementation & debugging of these (and future) bugs we should create code that is easy to maintain. Some notable issues:

1. RFC PR: #4732

* difficult to detect bugs * difficultly debugging * unexpected behavior * making the area resistant to change * confusion of responsibility

1. RFC PR: #4732

1. RFC PR: #4732

1. RFC PR: #4732

Activity architecture overview

Note: This is an abridged abstraction of the mentioned classes focusing on only the relevant public and protected APIs.

classDiagram

    class LocaleAwareAppCompatActivity {
        override fun attachBaseContext() calls super
        override fun onCreate() calls super
    }
    AppCompatActivity <|-- LocaleAwareAppCompatActivity

    class NavHostActivityInterface {
        fun getSupportActionBarAndInflateIfNecessary()
    }

    class HomeActivity {
        override fun getSupportActionBarAndInflateIfNecessary()
        override fun onCreate() calls super
        override fun onResume() calls super
        override fun onStart() calls super
        override fun onStop() calls super
        override fun onPause() calls super
        override fun onProvideAssistContent() calls super
        override fun onDestroy() calls super
        override fun onConfigurationChanged() calls super
        override fun recreate() calls super
        override fun onNewIntent() calls super
        override fun onCreateView() calls super
        override fun onActionModeStarted() calls super
        override fun onActionModeFinished() calls super
        override fun onBackPressed()
        override fun onActivityResult() calls super
        override fun onKeyDown() calls super
        override fun onKeyUp() calls super
        override fun onKeyLongPress() calls super
        override fun onUserLeaveHint() calls super
        open fun getNavDirections()
        open fun getBreadcrumbMessage()
        open fun getIntentSource()
        open fun getIntentSessionId()
    }
    NavHostActivityInterface <|-- HomeActivity
    LocaleAwareAppCompatActivity <|-- HomeActivity

    class ExternalAppBrowserActivity {
        override fun onResume() calls super
        override fun onDestroy() calls super
        override fun onProvideAssistContent() calls super

        override fun getNavDirections()
        override fun getBreadcrumbMessage()
        override fun getIntentSource()
        override fun getIntentSessionId()
    }
    HomeActivity <|-- ExternalAppBrowserActivity

    class AuthCustomActivity {
         override fun onResume() calls super
    }
    ExternalAppBrowserActivity <|-- AuthCustomActivity

Observations

1. RFC PR: #4732

1. RFC PR: #4732

• onResume()
• onDestroy()
• onProvideAssistContent()

which all depend on the super (HomeActivity) definitions prior to adding the ExternalAppBrowserActivity behaviour. onResume() is further propagated to AuthCustomTabActivity which depends on the ExternalAppBrowserActivity implementation.

1. onProvideAssistContent()

• getNavDirections()
• getBreadcrumbMessage()
• getIntentSource()
• getIntentSessionId()

It's notable that the HomeActivity getIntentSessionId() implementation always returns null and is required to take a redundant intent parameter. The HomeActivity getNavDirections() implementation is required to take a redundant customTabSessionId parameter.

1. getIntentSessionId()

• maybeShowSplashScreen()
• shouldShowOnboarding()
• perform checks for Homepage Contile feature activity
• perform checks for Pocket feature activity

This list is not exhaustive but gives an insight into the redundant work being forced upon the ExternalAppBrowserActivity. This applies to many of the other HomeActivity lifecycle related functions.

Proposal

Reduce the coupling of the Activitys. Establish HomeActivity as the main Activity and define clear responsibilities. Below is an list of suggested refactorings to initiate the decoupling.

1. perform checks for Pocket feature activity

1. perform checks for Pocket feature activity

1. perform checks for Pocket feature activity

1. perform checks for Pocket feature activity

1. perform checks for Pocket feature activity

1. perform checks for Pocket feature activity

1. perform checks for Pocket feature activity

The next steps are less clearly defined and require more investigation on the completion of the above tasks.

• Explore alternatives to the current Inheritance structure. E.g. Kotlin Delegation methods, an alternative 'base' Activity model or consolidating ExternalAppBrowserActivity & HomeActivity into a single Activity.
• Prevent unnecessary checks being carried out by ExternalAppBrowserActivity in HomeActivity lifecycle related functions.

Drawbacks

• Current reported Custom Tabs Bugzilla bugs may be affected by the changes mentioned here.
• No immediate tangible user facing improvements.

Rationale and alternatives

Rationale

• It is well established best practice to 'Prefer composition over inheritence'. An implementation of this principal is also exemplified in AC UI classes, see Reference Browser UI classes e.g. BrowserActivity & BaseBrowserFragment.
• Separation of concerns.

The most important principle to follow is separation of concerns. It's a common mistake to write all your code in an Activity or a Fragment. These UI-based classes should only contain logic that handles UI and operating system interactions. By keeping these classes as lean as possible, you can avoid many problems related to the component lifecycle, and improve the testability of these classes.

Reducing the coupling of the Activitys and clear delineation of responsibilities should vastly improve their maintainability & testability.

Alternatives

• Leave the inheritance as-is between HomeActivity, ExternalAppBrowserActivity & AuthCustomTabActivity and only focus on refactoring work.