Commit e38c71e1 authored by MozLando's avatar MozLando
Browse files

Merge #7717

7717: Closes #7702: Close unsupported extension pages on startup r=Amejia481 a=csadilek

Described the why here: https://github.com/mozilla-mobile/android-components/issues/7702#issue-656152408

 and in the comments below.

It's pretty easy to do, and aside from the described edge cases, this will also solve the problem when upgrading from GV 79 to 80 (from registerWebExtension to installBuiltIn) where we would otherwise end up with broken extension tabs (reader pages). Since that's the default behaviour too when uninstalling extensions, I would like to land this to prevent a bad user experience.
Co-authored-by: default avatarChristian Sadilek <christian.sadilek@gmail.com>
parents da30067b 447d6698
......@@ -5,13 +5,17 @@
package mozilla.components.support.webextensions
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.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.SessionState
import mozilla.components.browser.state.state.WebExtensionState
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
......@@ -24,7 +28,9 @@ import mozilla.components.concept.engine.webextension.WebExtensionDelegate
import mozilla.components.concept.engine.webextension.WebExtensionRuntime
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.ktx.kotlin.isExtensionUrl
import mozilla.components.support.ktx.kotlinx.coroutines.flow.filterChanged
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
import mozilla.components.support.webextensions.facts.emitWebExtensionsInitializedFact
import java.util.concurrent.ConcurrentHashMap
......@@ -290,6 +296,7 @@ object WebExtensionSupport {
onSuccess = {
extensions -> extensions.forEach { registerInstalledExtension(store, it) }
emitWebExtensionsInitializedFact(extensions)
closeUnsupportedTabs(store, extensions)
initializationResult.complete(Unit)
onExtensionsLoaded?.invoke(extensions.filter { !it.isBuiltIn() })
},
......@@ -318,6 +325,40 @@ object WebExtensionSupport {
}
}
/**
* Closes any leftover extensions tabs from extensions that are no longer
* installed/registered. When an extension is uninstalled, all extension
* pages will be closed. So, in theory, there should never be any
* leftover tabs. However, since we support temporary registered
* extensions and also recently migrated built-in extensions from the
* transient registerWebExtensions to the persistent installBuiltIn, we
* should handle this case to make sure we don't have any unloadable tabs
* around.
*/
private fun closeUnsupportedTabs(store: BrowserStore, extensions: List<WebExtension>) {
val supportedUrls = extensions.mapNotNull { it.getMetadata()?.baseUrl }
// We only need to do this a single time, once tabs are restored. We need to observe the
// store (instead of querying it directly), as tabs can be restored asynchronously on
// startup and might not be ready yet.
var scope: CoroutineScope? = null
scope = store.flowScoped { flow ->
flow.map { state -> state.tabs.filter { it.source == SessionState.Source.RESTORED }.size }
.ifChanged()
.collect { size ->
if (size > 0) {
store.state.tabs.forEach { tab ->
val tabUrl = tab.content.url
if (tabUrl.isExtensionUrl() && supportedUrls.none { tabUrl.startsWith(it) }) {
closeTab(tab.id, store, onCloseTabOverride)
}
}
scope?.cancel()
}
}
}
}
/**
* Marks the provided [updatedExtension] as updated in the [store].
*/
......@@ -400,7 +441,7 @@ object WebExtensionSupport {
id: String,
store: BrowserStore,
onCloseTabOverride: ((WebExtension?, String) -> Unit)? = null,
webExtension: WebExtension?
webExtension: WebExtension? = null
) {
onCloseTabOverride?.invoke(webExtension, id) ?: store.dispatch(TabListAction.RemoveTabAction(id))
}
......
......@@ -13,7 +13,9 @@ import mozilla.components.browser.state.action.ContentAction
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.createTab
import mozilla.components.browser.state.store.BrowserStore
......@@ -696,4 +698,50 @@ class WebExtensionSupportTest {
store.waitUntilIdle()
assertTrue(WebExtensionSupport.installedExtensions.isEmpty())
}
@Test
fun `closes unsupported extension`() {
val store = spy(BrowserStore(BrowserState(
tabs = listOf(
createTab(id = "1", url = "https://www.mozilla.org", source = SessionState.Source.RESTORED),
createTab(id = "2", url = "moz-extension://1234-5678/test", source = SessionState.Source.RESTORED),
createTab(id = "3", url = "moz-extension://1234-5678-9/", source = SessionState.Source.RESTORED)
)
)))
val ext1: WebExtension = mock()
val ext1Meta: Metadata = mock()
whenever(ext1Meta.baseUrl).thenReturn("moz-extension://1234-5678/")
whenever(ext1.id).thenReturn("1")
whenever(ext1.url).thenReturn("url1")
whenever(ext1.getMetadata()).thenReturn(ext1Meta)
whenever(ext1.isEnabled()).thenReturn(true)
whenever(ext1.isAllowedInPrivateBrowsing()).thenReturn(true)
val ext2: WebExtension = mock()
whenever(ext2.id).thenReturn("2")
whenever(ext2.url).thenReturn("url2")
whenever(ext2.isEnabled()).thenReturn(true)
whenever(ext2.isAllowedInPrivateBrowsing()).thenReturn(false)
val engine: Engine = mock()
val callbackCaptor = argumentCaptor<((List<WebExtension>) -> Unit)>()
whenever(engine.listInstalledWebExtensions(callbackCaptor.capture(), any())).thenAnswer {
callbackCaptor.value.invoke(listOf(ext1, ext2))
}
WebExtensionSupport.initialize(engine, store)
store.waitUntilIdle()
assertNotNull(store.state.findTab("1"))
assertNotNull(store.state.findTab("2"))
assertNull(store.state.findTab("3"))
// Make sure we're running a single cleanup and stop the scope after
store.dispatch(TabListAction.AddTabAction(createTab(id = "4", url = "moz-extension://1234-5678-90/")))
.joinBlocking()
store.waitUntilIdle()
assertNotNull(store.state.findTab("4"))
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment