Closes mozilla-mobile#1385: Determine and expose reader-ability state

components/browser/session/src/main/java/mozilla/components/browser/session/Session.kt

@@ -72,6 +72,7 @@ class Session(
         fun onMediaAdded(session: Session, media: List<Media>, added: Media) = Unit
         fun onCrashStateChanged(session: Session, crashed: Boolean) = Unit
         fun onIconChanged(session: Session, icon: Bitmap?) = Unit
+        fun onReaderableStateUpdated(session: Session, readerable: Boolean) = Unit
     }
 
     /**
@@ -371,6 +372,13 @@ class Session(
         notifyObservers(old, new) { onCrashStateChanged(this@Session, new) }
     }
 
+    /**
+     * Readerable state, whether or not the current page can be shown in a reader view.
+     */
+    var readerable: Boolean by Delegates.observable(false) { _, _, new ->
+        notifyObservers { onReaderableStateUpdated(this@Session, new) }
+    }
+
     /**
      * Returns whether or not this session is used for a Custom Tab.
      */

components/browser/session/src/test/java/mozilla/components/browser/session/SessionTest.kt

@@ -609,6 +609,7 @@ class SessionTest {
         defaultObserver.onMediaAdded(session, emptyList(), mock())
         defaultObserver.onMediaRemoved(session, emptyList(), mock())
         defaultObserver.onIconChanged(session, mock())
+        defaultObserver.onReaderableStateUpdated(session, true)
     }
 
     @Test
@@ -932,4 +933,28 @@ class SessionTest {
 
         assertEquals(bitmapMock, notifiedIcon)
     }
+
+    @Test
+    fun `observer is notified when readerable state updated`() {
+        val observer = mock(Session.Observer::class.java)
+
+        val session = Session("https://www.mozilla.org")
+        session.register(observer)
+
+        session.readerable = true
+
+        verify(observer).onReaderableStateUpdated(
+                eq(session),
+                eq(true))
+
+        // We want to notify observers every time readerability is determined,
+        // not only when the state changed.
+        session.readerable = true
+
+        verify(observer, times(2)).onReaderableStateUpdated(
+                eq(session),
+                eq(true))
+
+        assertTrue(session.readerable)
+    }
 }

components/feature/readerview/src/main/assets/extensions/readerview/readerview.js

@@ -5,9 +5,24 @@
 /* Avoid adding ID selector rules in this style sheet, since they could
  * inadvertently match elements in the article content. */
 
+const supportedProtocols = ["http:", "https:"];
+const blockedHosts = ["amazon.com", "github.com", "mail.google.com", "pinterest.com", "reddit.com", "twitter.com", "youtube.com"];
+
 class ReaderView {
 
   static isReaderable() {
+    if (!supportedProtocols.includes(location.protocol)) {
+      return false;
+    }
+
+    if (blockedHosts.some(blockedHost => location.hostname.endsWith(blockedHost))) {
+      return false;
+    }
+
+    if (location.pathname == "/") {
+      return false;
+    }
+
     return isProbablyReaderable(document);
   }
 
@@ -261,15 +276,24 @@ class ReaderView {
    }
 }
 
-// TODO remove (for testing purposes only)
 let port = browser.runtime.connectNative("mozacReaderview");
-port.postMessage(`Hello from ReaderView on page ${location.hostname}`);
 
-port.onMessage.addListener((response) => {
-  console.log(`Received: ${JSON.stringify(response)} on page ${location.hostname}`);
+port.onMessage.addListener((message) => {
+    switch (message.action) {
+      case 'show':
+        readerView.show({fontSize: 3, fontType: "serif", colorScheme: "light"});
+        break;
+      case 'hide':
+        readerView.hide();
+        break;
+      case 'checkReaderable':
+        port.postMessage({readerable: ReaderView.isReaderable()});
+        break;
+      default:
+        console.error(`Received invalid action ${message.action}`);
+    }
 });
 
-
 // TODO remove hostname check (for testing purposes only)
 // e.g. https://blog.mozilla.org/firefox/reader-view
 if (ReaderView.isReaderable() && location.hostname.endsWith("blog.mozilla.org")) {

components/feature/readerview/src/main/java/mozilla/components/feature/readerview/ReaderViewFeature.kt

@@ -38,8 +38,11 @@ typealias OnReaderViewAvailableChange = (available: Boolean) -> Unit
  * @property sessionManager a reference to the application's [SessionManager].
  * @property onReaderViewAvailableChange a callback invoked to indicate whether
  * or not reader view is available for the page loaded by the currently selected
- * (active) session.
+ * session. The callback will be invoked when a page is loaded or refreshed,
+ * on any navigation (back or forward), and when the selected session
+ * changes.
  */
+@Suppress("TooManyFunctions")
 class ReaderViewFeature(
     private val context: Context,
     private val engine: Engine,
@@ -93,6 +96,8 @@ class ReaderViewFeature(
             ReaderViewFeature.install(engine)
         }
 
+        checkReaderable()
+
         controlsInteractor.start()
     }
 
@@ -101,26 +106,24 @@ class ReaderViewFeature(
             return
         }
 
-        // TODO https://github.com/mozilla-mobile/android-components/issues/2624
         val messageHandler = object : MessageHandler {
             override fun onPortConnected(port: Port) {
-                ReaderViewFeature.ports[port.engineSession] = port
+                ports[port.engineSession] = port
+                checkReaderable()
             }
 
             override fun onPortDisconnected(port: Port) {
-                ReaderViewFeature.ports.remove(port.engineSession)
+                ports.remove(port.engineSession)
             }
 
             override fun onPortMessage(message: Any, port: Port) {
-                Logger.info("Received message: $message")
-                val response = JSONObject().apply {
-                    put("response", "Message received! Hello from Android!")
+                if (message is JSONObject) {
+                    activeSession?.readerable = message.optBoolean(READERABLE_RESPONSE_MESSAGE_KEY, false)
                 }
-                ReaderViewFeature.sendMessage(response, port.engineSession!!)
             }
         }
 
-        ReaderViewFeature.registerMessageHandler(sessionManager.getOrCreateEngineSession(session), messageHandler)
+        registerMessageHandler(sessionManager.getOrCreateEngineSession(session), messageHandler)
     }
 
     override fun stop() {
@@ -136,19 +139,32 @@ class ReaderViewFeature(
     override fun onSessionSelected(session: Session) {
         // TODO restore selected state of whether the controls are open or not
         registerContentMessageHandler(activeSession)
+        checkReaderable()
         super.onSessionSelected(session)
     }
 
     override fun onSessionRemoved(session: Session) {
-        ReaderViewFeature.ports.remove(sessionManager.getEngineSession(session))
+        ports.remove(sessionManager.getEngineSession(session))
+    }
+
+    override fun onUrlChanged(session: Session, url: String) {
+        checkReaderable()
+    }
+
+    override fun onReaderableStateUpdated(session: Session, readerable: Boolean) {
+        onReaderViewAvailableChange(readerable)
     }
 
     fun showReaderView() {
-        // TODO send message to show reader view (-> see ReaderView.show())
+        activeSession?.let {
+            sendMessage(JSONObject().put(ACTION_MESSAGE_KEY, ACTION_SHOW), it)
+        }
     }
 
     fun hideReaderView() {
-        // TODO send message to hide reader view (-> see ReaderView.hide())
+        activeSession?.let {
+            sendMessage(JSONObject().put(ACTION_MESSAGE_KEY, ACTION_HIDE), it)
+        }
     }
 
     /**
@@ -165,12 +181,43 @@ class ReaderViewFeature(
         controlsPresenter.hide()
     }
 
+    internal fun checkReaderable() {
+        activeSession?.let {
+            if (ports.containsKey(sessionManager.getEngineSession(it))) {
+                sendMessage(JSONObject().put(ACTION_MESSAGE_KEY, ACTION_CHECK_READERABLE), it)
+            }
+        }
+    }
+
+    private fun sendMessage(msg: Any, session: Session) {
+        val port = ports[sessionManager.getEngineSession(session)]
+        port?.postMessage(msg) ?: throw IllegalStateException("No port connected for the provided session")
+    }
+
     companion object {
-        @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+        @VisibleForTesting
         internal const val READER_VIEW_EXTENSION_ID = "mozacReaderview"
+
+        @VisibleForTesting
         internal const val READER_VIEW_EXTENSION_URL = "resource://android/assets/extensions/readerview/"
 
+        @VisibleForTesting
+        internal const val ACTION_MESSAGE_KEY = "action"
+
+        @VisibleForTesting
+        internal const val ACTION_SHOW = "show"
+
+        @VisibleForTesting
+        internal const val ACTION_HIDE = "hide"
+
+        @VisibleForTesting
+        internal const val ACTION_CHECK_READERABLE = "checkReaderable"
+
+        @VisibleForTesting
+        internal const val READERABLE_RESPONSE_MESSAGE_KEY = "readerable"
+
         @Volatile
+        @VisibleForTesting
         internal var installedWebExt: WebExtension? = null
 
         @Volatile
@@ -205,10 +252,5 @@ class ReaderViewFeature(
 
             installedWebExt?.let { registerContentMessageHandler(it) }
         }
-
-        fun sendMessage(msg: Any, engineSession: EngineSession) {
-            val port = ports[engineSession]
-            port?.postMessage(msg) ?: throw IllegalStateException("No port connected for the provided session")
-        }
     }
 }

components/feature/readerview/src/test/java/mozilla/components/feature/readerview/ReaderViewFeatureTest.kt

@@ -18,6 +18,8 @@ import mozilla.components.support.test.any
 import mozilla.components.support.test.argumentCaptor
 import mozilla.components.support.test.eq
 import mozilla.components.support.test.mock
+import org.json.JSONObject
+import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
@@ -28,6 +30,7 @@ import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers.anyInt
 import org.mockito.Mockito.`when`
 import org.mockito.Mockito.mock
+import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
@@ -100,7 +103,6 @@ class ReaderViewFeatureTest {
         val engineSession: EngineSession = mock()
         val ext: WebExtension = mock()
         val messageHandler = argumentCaptor<MessageHandler>()
-        val message: Any = mock()
 
         ReaderViewFeature.installedWebExt = ext
 
@@ -117,8 +119,9 @@ class ReaderViewFeatureTest {
         messageHandler.value.onPortConnected(port)
         assertTrue(ReaderViewFeature.ports.containsValue(port))
 
+        val message = JSONObject().put("readerable", true)
         messageHandler.value.onPortMessage(message, port)
-        verify(port).postMessage(any())
+        verify(session).readerable = true
 
         messageHandler.value.onPortDisconnected(port)
         assertFalse(ReaderViewFeature.ports.containsValue(port))
@@ -158,4 +161,67 @@ class ReaderViewFeatureTest {
 
         verify(view).hideControls()
     }
+
+    @Test
+    fun `check readerable on start`() {
+        val engine = mock(Engine::class.java)
+        val sessionManager: SessionManager = mock()
+        val view: ReaderViewControlsView = mock()
+
+        val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
+        readerViewFeature.start()
+
+        verify(readerViewFeature).checkReaderable()
+    }
+
+    @Test
+    fun `check readerable when session is selected`() {
+        val engine = mock(Engine::class.java)
+        val sessionManager: SessionManager = mock()
+        val view: ReaderViewControlsView = mock()
+
+        val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
+        readerViewFeature.onSessionSelected(mock())
+
+        verify(readerViewFeature).checkReaderable()
+    }
+
+    @Test
+    fun `check readerable when url changed`() {
+        val engine = mock(Engine::class.java)
+        val sessionManager: SessionManager = mock()
+        val view: ReaderViewControlsView = mock()
+
+        val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
+        readerViewFeature.onUrlChanged(mock(), "")
+
+        verify(readerViewFeature).checkReaderable()
+    }
+
+    @Test
+    fun `readerable check sends message to web extension`() {
+        val engine = mock(Engine::class.java)
+        val sessionManager: SessionManager = mock()
+        val view: ReaderViewControlsView = mock()
+        val ext: WebExtension = mock()
+        val session: Session = mock()
+        val engineSession: EngineSession = mock()
+        val port: Port = mock()
+        val message = argumentCaptor<JSONObject>()
+
+        `when`(sessionManager.selectedSession).thenReturn(session)
+        `when`(sessionManager.getEngineSession(session)).thenReturn(engineSession)
+        `when`(sessionManager.getOrCreateEngineSession(session)).thenReturn(engineSession)
+        val readerViewFeature = spy(ReaderViewFeature(context, engine, sessionManager, view))
+        ReaderViewFeature.installedWebExt = ext
+        ReaderViewFeature.ports[engineSession] = port
+
+        readerViewFeature.checkReaderable()
+        verify(port, never()).postMessage(eq(message))
+
+        readerViewFeature.observeSelected()
+        readerViewFeature.checkReaderable()
+        verify(port, times(1)).postMessage(message.capture())
+        assertEquals(ReaderViewFeature.ACTION_CHECK_READERABLE, message.value[ReaderViewFeature.ACTION_MESSAGE_KEY])
+    }
 }