For #4066: Create InflationAwareFeature for lazy inflation

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -183,7 +183,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler, SessionManager.Obs
                 feature = FindInPageIntegration(
                     sessionManager = requireComponents.core.sessionManager,
                     sessionId = customTabSessionId,
-                    view = view.findInPageView,
+                    stub = view.stubFindInPage,
                     engineView = view.engineView,
                     toolbar = toolbar
                 ),

app/src/main/java/org/mozilla/fenix/components/FindInPageIntegration.kt

@@ -5,11 +5,9 @@
 package org.mozilla.fenix.components
 
 import android.content.Context
-import android.os.Looper
 import android.util.AttributeSet
 import android.view.View
 import android.view.ViewStub
-import androidx.annotation.UiThread
 import androidx.coordinatorlayout.widget.CoordinatorLayout
 import mozilla.components.browser.session.SessionManager
 import mozilla.components.browser.session.runWithSessionIdOrSelected
@@ -18,54 +16,31 @@ import mozilla.components.concept.engine.EngineView
 import mozilla.components.feature.findinpage.FindInPageFeature
 import mozilla.components.feature.findinpage.view.FindInPageBar
 import mozilla.components.feature.findinpage.view.FindInPageView
-import mozilla.components.support.base.feature.BackHandler
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 import org.mozilla.fenix.test.Mockable
 
-/**
- * This class provides lazy loading of the Find in Page View.
- * It should be launched on the app's UI thread.
- */
 @Mockable
 class FindInPageIntegration(
     private val sessionManager: SessionManager,
     private val sessionId: String? = null,
-    private val stub: ViewStub,
-    engineView: EngineView,
+    stub: ViewStub,
+    private val engineView: EngineView,
     private val toolbar: BrowserToolbar
-) : LifecycleAwareFeature, BackHandler {
-
-    private var view: FindInPageView? = null
-    private val feature: FindInPageFeature by lazy(LazyThreadSafetyMode.NONE) {
-        view = stub.inflate() as FindInPageView
-        FindInPageFeature(sessionManager, view!!, engineView, ::onClose).also { it.start() }
+) : InflationAwareFeature(stub) {
+    override fun onViewInflated(view: View): LifecycleAwareFeature {
+        return FindInPageFeature(sessionManager, view as FindInPageView, engineView) {
+            toolbar.visibility = View.VISIBLE
+            view.visibility = View.GONE
+        }
     }
 
-    override fun start() {
-    }
-
-    override fun stop() {
-        if (view != null) feature.stop()
-    }
-
-    override fun onBackPressed(): Boolean {
-        return if (view != null) feature.onBackPressed() else false
-    }
-
-    private fun onClose() {
-        toolbar.visibility = View.VISIBLE
-        view?.asView()?.visibility = View.GONE
-    }
-
-    @UiThread
-    fun launch() {
-        require(Looper.myLooper() == Looper.getMainLooper()) { "This method should be run on the main UI thread." }
-        sessionManager.runWithSessionIdOrSelected(sessionId) {
-            if (!it.isCustomTabSession()) {
+    override fun onLaunch(view: View, feature: LifecycleAwareFeature) {
+        sessionManager.runWithSessionIdOrSelected(sessionId) { session ->
+            if (!session.isCustomTabSession()) {
                 toolbar.visibility = View.GONE
             }
-            feature.bind(it)
-            view?.asView()?.visibility = View.VISIBLE
+            view.visibility = View.VISIBLE
+            (feature as FindInPageFeature).bind(session)
         }
     }
 }

app/src/main/java/org/mozilla/fenix/components/InflationAwareFeature.kt

@@ -0,0 +1,84 @@
+package org.mozilla.fenix.components
+
+import android.view.View
+import android.view.ViewStub
+import androidx.annotation.UiThread
+import mozilla.components.support.base.feature.BackHandler
+import mozilla.components.support.base.feature.LifecycleAwareFeature
+import java.lang.ref.WeakReference
+
+/**
+ * A base feature class that enables lazy inflation of a view needed by a feature.
+ *
+ * When a feature needs to be launched (e.g. by user interaction) calling [launch]
+ * will inflate the view only then, start the feature, and then executes [onLaunch]
+ * for any feature-specific startup needs.
+ */
+abstract class InflationAwareFeature(
+    private val stub: ViewStub
+) : LifecycleAwareFeature, BackHandler {
+
+    internal lateinit var view: WeakReference<View>
+    internal var feature: LifecycleAwareFeature? = null
+    private val stubListener = ViewStub.OnInflateListener { _, inflated ->
+        view = WeakReference(inflated)
+        feature = onViewInflated(inflated).also {
+            it.start()
+            onLaunch(inflated, it)
+        }
+    }
+
+    /**
+     * Invoked when a view-dependent feature needs to be started along with the feature itself.
+     */
+    @UiThread
+    fun launch() {
+        // If we have a feature and view, we can launch immediately.
+        if (feature != null && view.get() != null) {
+            onLaunch(view.get()!!, feature!!)
+        } else {
+            stub.apply {
+                setOnInflateListener(stubListener)
+                inflate()
+            }
+        }
+    }
+
+    /**
+     * Implementation notes: This implemented method does nothing since we only start the feature
+     * when the view is inflated.
+     */
+    override fun start() {
+        // We don't do anything because we only want to start the feature when it's being used.
+    }
+
+    override fun stop() {
+        feature?.stop()
+    }
+
+    /**
+     * Called when the feature gets the option to handle the user pressing the back key.
+     *
+     * @return true if the feature also implements [BackHandler] and the feature has been initiated.
+     */
+    override fun onBackPressed(): Boolean {
+        return (feature as? BackHandler)?.onBackPressed() ?: false
+    }
+
+    /**
+     * Invoked when the view has been inflated for the feature to be created with it.
+     *
+     * @param view The newly created view.
+     * @return The feature initiated with the view.
+     */
+    abstract fun onViewInflated(view: View): LifecycleAwareFeature
+
+    /**
+     * Invoked after the feature is instantiated. If the feature already exists,
+     * this is invoked immediately.
+     *
+     * @param view The view that is attached to the feature.
+     * @param feature The feature that was instantiated.
+     */
+    abstract fun onLaunch(view: View, feature: LifecycleAwareFeature)
+}

app/src/main/res/layout/fragment_browser.xml

@@ -31,10 +31,12 @@
         app:layout_behavior="org.mozilla.fenix.quickactionsheet.QuickActionSheetBehavior" />
 
     <ViewStub
-        android:id="@+id/stub_find_in_page"
+        android:id="@+id/stubFindInPage"
         android:inflatedId="@+id/findInPageView"
         android:layout="@layout/stub_find_in_page"
-        android:layout_gravity="bottom" />
+        android:layout_gravity="bottom"
+        android:layout_width="match_parent"
+        android:layout_height="56dp" />
 
     <mozilla.components.feature.readerview.view.ReaderViewControlsBar
         android:id="@+id/readerViewControlsBar"

app/src/test/java/org/mozilla/fenix/components/InflationAwareFeatureTest.kt

@@ -0,0 +1,107 @@
+package org.mozilla.fenix.components
+
+import android.view.View
+import android.view.ViewStub
+import mozilla.components.support.base.feature.BackHandler
+import mozilla.components.support.base.feature.LifecycleAwareFeature
+import mozilla.components.support.test.any
+import mozilla.components.support.test.mock
+import org.junit.Test
+import org.mockito.Mockito.never
+import org.mockito.Mockito.spy
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyNoMoreInteractions
+import java.lang.ref.WeakReference
+
+class InflationAwareFeatureTest {
+    @Test
+    fun `stub inflates if no feature or view exists`() {
+        val stub: ViewStub = mock()
+        val feature: InflationAwareFeature = spy(TestableInflationAwareFeature(stub))
+
+        feature.launch()
+
+        verify(stub).setOnInflateListener(any())
+        verify(stub).inflate()
+    }
+
+    @Test
+    fun `stub immediately launches if the feature is available`() {
+        val stub: ViewStub = mock()
+        val feature: InflationAwareFeature = spy(TestableInflationAwareFeature(stub))
+
+        feature.feature = mock()
+        feature.view = WeakReference(mock())
+
+        feature.launch()
+
+        verify(stub, never()).setOnInflateListener(any())
+        verify(stub, never()).inflate()
+        verify(feature).onLaunch(any(), any())
+    }
+
+    @Test
+    fun `feature calls stop if created`() {
+        val stub: ViewStub = mock()
+        val inflationFeature: InflationAwareFeature = spy(TestableInflationAwareFeature(stub))
+        val innerFeature: LifecycleAwareFeature = mock()
+
+        inflationFeature.stop()
+
+        verify(innerFeature, never()).stop()
+
+        inflationFeature.feature = innerFeature
+
+        inflationFeature.stop()
+
+        verify(innerFeature).stop()
+    }
+
+    @Test
+    fun `start does nothing`() {
+        val stub: ViewStub = mock()
+        val inflationFeature: InflationAwareFeature = spy(TestableInflationAwareFeature(stub))
+        val innerFeature: LifecycleAwareFeature = mock()
+
+        inflationFeature.feature = innerFeature
+        inflationFeature.view = WeakReference(mock())
+
+        inflationFeature.start()
+
+        verifyNoMoreInteractions(innerFeature)
+        verifyNoMoreInteractions(stub)
+    }
+
+    @Test
+    fun `if feature has implemented BackHandler invoke it`() {
+        val stub: ViewStub = mock()
+        val inflationFeature: InflationAwareFeature = spy(TestableInflationAwareFeature(stub))
+        val innerFeature: LifecycleAwareFeature = mock()
+        val backHandlerFeature = object : LifecycleAwareFeature, BackHandler {
+            override fun onBackPressed() = true
+
+            override fun start() {}
+
+            override fun stop() {}
+        }
+
+        assert(!inflationFeature.onBackPressed())
+
+        inflationFeature.feature = innerFeature
+
+        assert(!inflationFeature.onBackPressed())
+
+        inflationFeature.feature = backHandlerFeature
+
+        assert(inflationFeature.onBackPressed())
+    }
+}
+
+class TestableInflationAwareFeature(stub: ViewStub) : InflationAwareFeature(stub) {
+    override fun onViewInflated(view: View): LifecycleAwareFeature {
+        return mock()
+    }
+
+    override fun onLaunch(view: View, feature: LifecycleAwareFeature) {
+    }
+}