Issue #2624: Reader mode not working for tabs opened in background

mozilla-mobile · May 1, 2019 · a07e8f2 · a07e8f2
1 parent 07949ce
commit a07e8f2
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 25 deletions.
diff --git a/...ature/readerview/src/main/java/mozilla/components/feature/readerview/ReaderViewFeature.kt b/...ature/readerview/src/main/java/mozilla/components/feature/readerview/ReaderViewFeature.kt
@@ -97,10 +97,7 @@ class ReaderViewFeature(
         }
 
         if (portConnected()) {
-            checkReaderable()
-            if (activeSession?.readerMode == true) {
-                showReaderView()
-            }
+            updateReaderViewState()
         }
 
         controlsInteractor.start()
@@ -122,9 +119,12 @@ class ReaderViewFeature(
     }
 
     override fun onSessionSelected(session: Session) {
-        registerContentMessageHandler(activeSession)
-        checkReaderable()
         super.onSessionSelected(session)
+        updateReaderViewState(session)
+    }
+
+    override fun onSessionAdded(session: Session) {
+        registerContentMessageHandler(session)
     }
 
     override fun onSessionRemoved(session: Session) {
@@ -144,8 +144,8 @@ class ReaderViewFeature(
     /**
      * Shows the reader view UI.
      */
-    fun showReaderView() {
-        activeSession?.let {
+    fun showReaderView(session: Session? = activeSession) {
+        session?.let {
             val config = JSONObject()
                 .put(ACTION_VALUE_SHOW_FONT_SIZE, config.fontSize)
                 .put(ACTION_VALUE_SHOW_FONT_TYPE, config.fontType.name.toLowerCase())
@@ -186,8 +186,8 @@ class ReaderViewFeature(
         controlsPresenter.hide()
     }
 
-    internal fun checkReaderable() {
-        activeSession?.let {
+    internal fun checkReaderable(session: Session? = activeSession) {
+        session?.let {
             if (portConnected()) {
                 sendContentMessage(JSONObject().put(ACTION_MESSAGE_KEY, ACTION_CHECK_READERABLE), it)
             }
@@ -202,10 +202,7 @@ class ReaderViewFeature(
         val messageHandler = object : MessageHandler {
             override fun onPortConnected(port: Port) {
                 ports[port.engineSession] = port
-                checkReaderable()
-                if (activeSession?.readerMode == true) {
-                    showReaderView()
-                }
+                updateReaderViewState(session)
             }
 
             override fun onPortDisconnected(port: Port) {
@@ -214,7 +211,7 @@ class ReaderViewFeature(
 
             override fun onPortMessage(message: Any, port: Port) {
                 if (message is JSONObject) {
-                    activeSession?.readerable = message.optBoolean(READERABLE_RESPONSE_MESSAGE_KEY, false)
+                    session.readerable = message.optBoolean(READERABLE_RESPONSE_MESSAGE_KEY, false)
                 }
             }
         }
@@ -229,6 +226,18 @@ class ReaderViewFeature(
         }
     }
 
+    @VisibleForTesting
+    internal fun updateReaderViewState(session: Session? = activeSession) {
+        if (session == null) {
+            return
+        }
+
+        checkReaderable(session)
+        if (session.readerMode) {
+            showReaderView(session)
+        }
+    }
+
     @VisibleForTesting
     internal fun portConnected(session: Session? = activeSession): Boolean {
         return session?.let { ports.containsKey(sessionManager.getEngineSession(session)) } ?: false

diff --git a/...e/readerview/src/test/java/mozilla/components/feature/readerview/ReaderViewFeatureTest.kt b/...e/readerview/src/test/java/mozilla/components/feature/readerview/ReaderViewFeatureTest.kt
@@ -119,12 +119,7 @@ class ReaderViewFeatureTest {
 
         messageHandler.value.onPortConnected(port)
         assertTrue(ReaderViewFeature.ports.containsValue(port))
-        verify(readerViewFeature).checkReaderable()
-        verify(readerViewFeature, never()).showReaderView()
-
-        `when`(session.readerMode).thenReturn(true)
-        messageHandler.value.onPortConnected(port)
-        verify(readerViewFeature).showReaderView()
+        verify(readerViewFeature).updateReaderViewState(eq(session))
 
         val readerableMessage = JSONObject().put("readerable", true)
         messageHandler.value.onPortMessage(readerableMessage, port)
@@ -202,19 +197,46 @@ class ReaderViewFeatureTest {
         `when`(readerViewFeature.portConnected()).thenReturn(true)
         readerViewFeature.start()
 
-        verify(readerViewFeature).checkReaderable()
+        verify(readerViewFeature).updateReaderViewState(any())
     }
 
     @Test
-    fun `check readerable when session is selected`() {
+    fun `update reader view state when session is selected`() {
         val engine = mock(Engine::class.java)
         val sessionManager: SessionManager = mock()
         val view: ReaderViewControlsView = mock()
+        val selectedSession: Session = mock()
 
         val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
-        readerViewFeature.onSessionSelected(mock())
+        readerViewFeature.onSessionSelected(selectedSession)
 
-        verify(readerViewFeature).checkReaderable()
+        verify(readerViewFeature).updateReaderViewState(eq(selectedSession))
+    }
+
+    @Test
+    fun `register content message handler for added session`() {
+        val engine = mock(Engine::class.java)
+        val sessionManager: SessionManager = mock()
+        val view: ReaderViewControlsView = mock()
+        val session: Session = mock()
+        val engineSession: EngineSession = mock()
+        val ext: WebExtension = mock()
+        val messageHandler = argumentCaptor<MessageHandler>()
+
+        ReaderViewFeature.installedWebExt = ext
+
+        `when`(sessionManager.getOrCreateEngineSession(session)).thenReturn(engineSession)
+        val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
+
+        readerViewFeature.onSessionAdded(session)
+        verify(ext).registerContentMessageHandler(eq(engineSession), eq(ReaderViewFeature.READER_VIEW_EXTENSION_ID), messageHandler.capture())
+
+        val port: Port = mock()
+        `when`(port.engineSession).thenReturn(engineSession)
+
+        messageHandler.value.onPortConnected(port)
+        assertTrue(ReaderViewFeature.ports.containsValue(port))
+        verify(readerViewFeature).updateReaderViewState(eq(session))
     }
 
     @Test
@@ -349,6 +371,37 @@ class ReaderViewFeatureTest {
         verify(port, times(1)).postMessage(message.capture())
     }
 
+    @Test
+    fun `update state checks readerable and shows reader mode`() {
+        val engine = mock(Engine::class.java)
+        val sessionManager: SessionManager = mock()
+        val view: ReaderViewControlsView = mock()
+        val session: Session = mock()
+
+        val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
+
+        readerViewFeature.updateReaderViewState(null)
+        verify(readerViewFeature, never()).checkReaderable(any())
+        verify(readerViewFeature, never()).showReaderView(any())
+
+        readerViewFeature.updateReaderViewState(session)
+        verify(readerViewFeature).checkReaderable(eq(session))
+        verify(readerViewFeature, never()).showReaderView(any())
+
+        `when`(session.readerMode).thenReturn(true)
+        readerViewFeature.updateReaderViewState(session)
+        verify(readerViewFeature, times(2)).checkReaderable(eq(session))
+        verify(readerViewFeature).showReaderView(eq(session))
+
+        val selectedSession: Session = mock()
+        `when`(selectedSession.readerMode).thenReturn(true)
+        `when`(sessionManager.selectedSession).thenReturn(selectedSession)
+        readerViewFeature.observeSelected()
+        readerViewFeature.updateReaderViewState(selectedSession)
+        verify(readerViewFeature).checkReaderable(eq(selectedSession))
+        verify(readerViewFeature).showReaderView(eq(selectedSession))
+    }
+
     private fun prepareFeatureForTest(port: Port, session: Session = mock()): ReaderViewFeature {
         val engine = mock(Engine::class.java)
         val sessionManager: SessionManager = mock()