Skip to content

Commit

Permalink
Closes mozilla-mobile#9210: White page shown in custom tab when ubloc…
Browse files Browse the repository at this point in the history
…k blocks page
  • Loading branch information
csadilek committed Dec 16, 2020
1 parent 5aeacf1 commit 9e14c7b
Show file tree
Hide file tree
Showing 2 changed files with 173 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@

package mozilla.components.support.webextensions

import androidx.annotation.VisibleForTesting
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import mozilla.components.browser.state.action.CustomTabListAction
import mozilla.components.browser.state.action.EngineAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.action.WebExtensionAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.allTabs
import mozilla.components.browser.state.selector.findTabOrCustomTab
import mozilla.components.browser.state.state.CustomTabSessionState
import mozilla.components.browser.state.state.SessionState
import mozilla.components.browser.state.state.WebExtensionState
import mozilla.components.browser.state.state.createTab
Expand Down Expand Up @@ -104,9 +108,9 @@ object WebExtensionSupport {
) : TabHandler {

override fun onCloseTab(webExtension: WebExtension, engineSession: EngineSession): Boolean {
val tab = store.state.findTab(sessionId)
val tab = store.state.findTabOrCustomTab(sessionId)
return if (tab != null) {
closeTab(tab.id, store, onCloseTabOverride, webExtension)
closeTab(tab.id, tab.isCustomTab(), store, onCloseTabOverride, webExtension)
true
} else {
false
Expand All @@ -119,9 +123,9 @@ object WebExtensionSupport {
active: Boolean,
url: String?
): Boolean {
val tab = store.state.findTab(sessionId)
val tab = store.state.findTabOrCustomTab(sessionId)
return if (tab != null) {
if (active) {
if (active && !tab.isCustomTab()) {
onSelectTabOverride?.invoke(webExtension, tab.id)
?: store.dispatch(TabListAction.SelectTabAction(tab.id))
}
Expand Down Expand Up @@ -213,7 +217,7 @@ object WebExtensionSupport {
val popupSessionId = store.state.extensions[extension.id]?.popupSessionId
if (popupSessionId != null && store.state.tabs.find { it.id == popupSessionId } != null) {
if (popupSessionId == store.state.selectedTabId) {
closeTab(popupSessionId, store, onCloseTabOverride, extension)
closeTab(popupSessionId, false, store, onCloseTabOverride, extension)
} else {
onSelectTabOverride?.invoke(extension, popupSessionId)
?: store.dispatch(TabListAction.SelectTabAction(popupSessionId))
Expand Down Expand Up @@ -317,7 +321,7 @@ object WebExtensionSupport {
// Register action handler for all existing engine sessions on the new extension,
// an issue was filed to get us an API, so we don't have to do this per extension:
// https://bugzilla.mozilla.org/show_bug.cgi?id=1603559
store.state.tabs
store.state.allTabs
.forEach { tab ->
tab.engineState.engineSession?.let { session ->
registerSessionHandlers(webExtension, store, session, tab.id)
Expand Down Expand Up @@ -350,7 +354,7 @@ object WebExtensionSupport {
store.state.tabs.forEach { tab ->
val tabUrl = tab.content.url
if (tabUrl.isExtensionUrl() && supportedUrls.none { tabUrl.startsWith(it) }) {
closeTab(tab.id, store, onCloseTabOverride)
closeTab(tab.id, false, store, onCloseTabOverride)
}
}
scope?.cancel()
Expand All @@ -367,7 +371,7 @@ object WebExtensionSupport {
store.dispatch(WebExtensionAction.UpdateWebExtensionAction(updatedExtension.toState()))

// Register action handler for all existing engine sessions on the new extension
store.state.tabs.forEach { tab ->
store.state.allTabs.forEach { tab ->
tab.engineState.engineSession?.let { session ->
registerSessionHandlers(updatedExtension, store, session, tab.id)
}
Expand All @@ -382,7 +386,7 @@ object WebExtensionSupport {
// We need to observe for the entire lifetime of the application,
// as web extension support is not tied to any particular view.
store.flowScoped { flow ->
flow.mapNotNull { state -> state.tabs }
flow.mapNotNull { state -> state.allTabs }
.filterChanged {
it.engineState.engineSession
}
Expand Down Expand Up @@ -439,19 +443,33 @@ object WebExtensionSupport {

private fun closeTab(
id: String,
customTab: Boolean,
store: BrowserStore,
onCloseTabOverride: ((WebExtension?, String) -> Unit)? = null,
webExtension: WebExtension? = null
) {
onCloseTabOverride?.invoke(webExtension, id) ?: store.dispatch(TabListAction.RemoveTabAction(id))
if (onCloseTabOverride != null) {
onCloseTabOverride.invoke(webExtension, id)
} else {
val action = if (customTab) {
CustomTabListAction.RemoveCustomTabAction(id)
} else {
TabListAction.RemoveTabAction(id)
}

store.dispatch(action)
}
}

private fun WebExtension.toState() =
@VisibleForTesting
internal fun WebExtension.toState() =
WebExtensionState(
id,
url,
getMetadata()?.name,
isEnabled(),
isAllowedInPrivateBrowsing()
)

private fun SessionState.isCustomTab() = this is CustomTabSessionState
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import kotlinx.coroutines.test.TestCoroutineDispatcher
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.setMain
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.action.CustomTabListAction
import mozilla.components.browser.state.action.EngineAction
import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.action.WebExtensionAction
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.SessionState
import mozilla.components.browser.state.state.WebExtensionState
import mozilla.components.browser.state.state.createCustomTab
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.Engine
Expand All @@ -36,12 +38,14 @@ import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.mock
import mozilla.components.support.test.whenever
import mozilla.components.support.webextensions.WebExtensionSupport.toState
import mozilla.components.support.webextensions.facts.WebExtensionFacts.Items.WEB_EXTENSIONS_INITIALIZED
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertSame
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -208,6 +212,39 @@ class WebExtensionSupportTest {
verify(store).dispatch(TabListAction.RemoveTabAction(tabId))
}

@Test
fun `reacts to custom tab being closed by removing tab from store`() {
val engine: Engine = mock()
val ext: WebExtension = mock()
whenever(ext.id).thenReturn("test")
whenever(ext.isEnabled()).thenReturn(true)
whenever(ext.hasTabHandler(any())).thenReturn(false, true)
val engineSession: EngineSession = mock()
val tabId = "testTabId"
val store = spy(
BrowserStore(
BrowserState(
customTabs = listOf(
createCustomTab(id = tabId, url = "https://www.mozilla.org", engineSession = engineSession)
)
)
)
)
val installedList = mutableListOf(ext)
val callbackCaptor = argumentCaptor<((List<WebExtension>) -> Unit)>()
whenever(engine.listInstalledWebExtensions(callbackCaptor.capture(), any())).thenAnswer {
callbackCaptor.value.invoke(installedList)
}

val tabHandlerCaptor = argumentCaptor<TabHandler>()
WebExtensionSupport.initialize(engine, store)

store.waitUntilIdle()
verify(ext).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())
tabHandlerCaptor.value.onCloseTab(ext, engineSession)
verify(store).dispatch(CustomTabListAction.RemoveCustomTabAction(tabId))
}

@Test
fun `allows overriding onCloseTab behaviour`() {
val engine: Engine = mock()
Expand Down Expand Up @@ -289,15 +326,62 @@ class WebExtensionSupportTest {
assertFalse(tabHandlerCaptor.value.onUpdateTab(ext, engineSession, true, "url"))
}

@Test
fun `reacts to custom tab being updated`() {
val engine: Engine = mock()
val ext: WebExtension = mock()
whenever(ext.id).thenReturn("test")
whenever(ext.isEnabled()).thenReturn(true)
whenever(ext.hasTabHandler(any())).thenReturn(false, true)
val engineSession: EngineSession = mock()
val tabId = "testTabId"
val store = BrowserStore(
BrowserState(
customTabs = listOf(
createCustomTab(id = tabId, url = "https://www.mozilla.org", engineSession = engineSession)
)
)
)

val installedList = mutableListOf(ext)
val callbackCaptor = argumentCaptor<((List<WebExtension>) -> Unit)>()
whenever(engine.listInstalledWebExtensions(callbackCaptor.capture(), any())).thenAnswer {
callbackCaptor.value.invoke(installedList)
}

val tabHandlerCaptor = argumentCaptor<TabHandler>()
WebExtensionSupport.initialize(engine, store)

// Update tab to select it
verify(ext).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())
assertNull(store.state.selectedTabId)
assertTrue(tabHandlerCaptor.value.onUpdateTab(ext, engineSession, true, null))
store.waitUntilIdle()

// Update URL of tab
assertTrue(tabHandlerCaptor.value.onUpdateTab(ext, engineSession, false, "url"))
verify(engineSession).loadUrl("url")

// Update non-existing tab
store.dispatch(CustomTabListAction.RemoveCustomTabAction(tabId)).joinBlocking()
assertFalse(tabHandlerCaptor.value.onUpdateTab(ext, engineSession, true, "url"))
}

@Test
fun `reacts to new extension being installed`() {
val engineSession: EngineSession = mock()
val tab =
createTab(id = "1", url = "https://www.mozilla.org", engineSession = engineSession)

val customTabEngineSession: EngineSession = mock()
val customTab =
createCustomTab(id = "2", url = "https://www.mozilla.org", engineSession = customTabEngineSession)

val store = spy(
BrowserStore(
BrowserState(
tabs = listOf(
createTab(id = "1", url = "https://www.mozilla.org", engineSession = engineSession)
)
tabs = listOf(tab),
customTabs = listOf(customTab)
)
)
)
Expand Down Expand Up @@ -326,12 +410,15 @@ class WebExtensionSupportTest {
val webExtensionActionCaptor = argumentCaptor<WebExtensionAction>()
val tabHandlerCaptor = argumentCaptor<TabHandler>()
val selectTabActionCaptor = argumentCaptor<TabListAction.SelectTabAction>()
verify(ext).registerActionHandler(eq(customTabEngineSession), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(customTabEngineSession), tabHandlerCaptor.capture())
verify(ext).registerActionHandler(eq(engineSession), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())

// Verify we only register the handlers once
whenever(ext.hasActionHandler(engineSession)).thenReturn(true)
whenever(ext.hasTabHandler(engineSession)).thenReturn(true)

// Verify we only register the handlers once
store.dispatch(ContentAction.UpdateUrlAction(sessionId = "1", url = "https://www.firefox.com")).joinBlocking()
verify(ext, times(1)).registerActionHandler(eq(engineSession), actionHandlerCaptor.capture())
verify(ext, times(1)).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())
Expand Down Expand Up @@ -431,12 +518,13 @@ class WebExtensionSupportTest {

@Test
fun `observes store and registers handlers on new engine sessions`() {
val tab = createTab(id = "1", url = "https://www.mozilla.org")
val customTab = createCustomTab(id = "2", url = "https://www.mozilla.org")
val store = spy(
BrowserStore(
BrowserState(
tabs = listOf(
createTab(id = "1", url = "https://www.mozilla.org")
)
tabs = listOf(tab),
customTabs = listOf(customTab)
)
)
)
Expand All @@ -454,14 +542,20 @@ class WebExtensionSupportTest {
delegateCaptor.value.onInstalled(ext)

// Verify that action/tab handler is registered when a new engine session is created
val engineSession: EngineSession = mock()
val actionHandlerCaptor = argumentCaptor<ActionHandler>()
val tabHandlerCaptor = argumentCaptor<TabHandler>()
verify(ext, never()).registerActionHandler(any(), any())
verify(ext, never()).registerTabHandler(any(), any())
store.dispatch(EngineAction.LinkEngineSessionAction("1", engineSession)).joinBlocking()
verify(ext).registerActionHandler(eq(engineSession), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())

val engineSession1: EngineSession = mock()
store.dispatch(EngineAction.LinkEngineSessionAction(tab.id, engineSession1)).joinBlocking()
verify(ext).registerActionHandler(eq(engineSession1), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(engineSession1), tabHandlerCaptor.capture())

val engineSession2: EngineSession = mock()
store.dispatch(EngineAction.LinkEngineSessionAction(customTab.id, engineSession2)).joinBlocking()
verify(ext).registerActionHandler(eq(engineSession2), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(engineSession2), tabHandlerCaptor.capture())
}

@Test
Expand Down Expand Up @@ -731,7 +825,7 @@ class WebExtensionSupportTest {
}

@Test
fun `closes unsupported extension`() {
fun `closes tabs from unsupported extensions`() {
val store = BrowserStore(BrowserState(
tabs = listOf(
createTab(id = "1", url = "https://www.mozilla.org", source = SessionState.Source.RESTORED),
Expand Down Expand Up @@ -775,4 +869,41 @@ class WebExtensionSupportTest {
store.waitUntilIdle()
assertNotNull(store.state.findTab("4"))
}

@Test
fun `marks extensions as updated`() {
val engineSession: EngineSession = mock()
val tab =
createTab(id = "1", url = "https://www.mozilla.org", engineSession = engineSession)

val customTabEngineSession: EngineSession = mock()
val customTab =
createCustomTab(id = "2", url = "https://www.mozilla.org", engineSession = customTabEngineSession)

val store = spy(
BrowserStore(
BrowserState(
tabs = listOf(tab),
customTabs = listOf(customTab)
)
)
)

val ext: WebExtension = mock()
whenever(ext.id).thenReturn("extensionId")
whenever(ext.url).thenReturn("url")
whenever(ext.supportActions).thenReturn(true)

WebExtensionSupport.markExtensionAsUpdated(store, ext)
assertSame(ext, WebExtensionSupport.installedExtensions[ext.id])
verify(store).dispatch(WebExtensionAction.UpdateWebExtensionAction(ext.toState()))

// Verify that we register new action and tab handlers for the updated extension
val actionHandlerCaptor = argumentCaptor<ActionHandler>()
val tabHandlerCaptor = argumentCaptor<TabHandler>()
verify(ext).registerActionHandler(eq(customTabEngineSession), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(customTabEngineSession), tabHandlerCaptor.capture())
verify(ext).registerActionHandler(eq(engineSession), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())
}
}

0 comments on commit 9e14c7b

Please sign in to comment.