Bug 1596186 - Close all extension pages when unregistering an extension. r=snorp
authorAgi Sferro <agi@sferro.dev>
Thu, 14 Nov 2019 17:05:52 +0000
changeset 501988 738801392270a0162d381b804fdb7f8111a57a42
parent 501987 fa5a4ec77dc13c221dfc9389a1fd204af6a34d1f
child 501989 ee728f3ea085900757752d009ea271d21087096a
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1596186
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1596186 - Close all extension pages when unregistering an extension. r=snorp Differential Revision: https://phabricator.services.mozilla.com/D52894
mobile/android/components/extensions/ext-android.js
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt
--- a/mobile/android/components/extensions/ext-android.js
+++ b/mobile/android/components/extensions/ext-android.js
@@ -35,21 +35,25 @@ global.tabGetSender = getSender;
 extensions.on("page-shutdown", (type, context) => {
   if (context.viewType == "tab") {
     if (context.extension.id !== context.xulBrowser.contentPrincipal.addonId) {
       // Only close extension tabs.
       // This check prevents about:addons from closing when it contains a
       // WebExtension as an embedded inline options page.
       return;
     }
-    let { BrowserApp } = context.xulBrowser.ownerGlobal;
+    const window = context.xulBrowser.ownerGlobal;
+    let { BrowserApp } = window;
     if (BrowserApp) {
       let nativeTab = BrowserApp.getTabForBrowser(context.xulBrowser);
       if (nativeTab) {
-        BrowserApp.closeTab(nativeTab);
+        GeckoViewTabBridge.closeTab({
+          window,
+          extensionId: context.extension.id,
+        });
       }
     }
   }
 });
 /* eslint-enable mozilla/balanced-listeners */
 
 global.openOptionsPage = extension => {
   let window = windowTracker.topWindow;
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExtensionTest.kt
@@ -13,16 +13,17 @@ import org.hamcrest.core.IsEqual.equalTo
 import org.json.JSONObject
 import org.junit.Assert
 import org.junit.Assert.assertTrue
 import org.junit.Assert.fail
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.geckoview.*
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
+import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.AssertCalled
 import org.mozilla.geckoview.test.util.Callbacks
 import org.mozilla.geckoview.test.util.HttpBin
 import java.net.URI
 
 import java.util.UUID
 
 @RunWith(AndroidJUnit4::class)
 @MediumTest
@@ -155,32 +156,35 @@ class WebExtensionTest : BaseSessionTest
     // - TabDelegate handles closing of newly created tab
     // - Verify that close request came from right extension and targeted session
     @Test
     fun testBrowserTabsCreateBrowserTabsRemove() {
         val onCloseRequestResult = GeckoResult<Void>()
         var tabsExtension : WebExtension? = null
         var extensionCreatedSession : GeckoSession? = null
 
-        val tabDelegate = object : WebExtensionController.TabDelegate {
+        sessionRule.addExternalDelegateUntilTestEnd(
+                WebExtensionController.TabDelegate::class,
+                sessionRule.runtime.webExtensionController::setTabDelegate,
+                { sessionRule.runtime.webExtensionController.tabDelegate = null },
+                object : WebExtensionController.TabDelegate {
             override fun onNewTab(source: WebExtension?, uri: String?): GeckoResult<GeckoSession> {
                 extensionCreatedSession = GeckoSession(sessionRule.session.settings)
                 return GeckoResult.fromValue(extensionCreatedSession)
             }
 
             override fun onCloseTab(source: WebExtension?, session: GeckoSession): GeckoResult<AllowOrDeny> {
                 Assert.assertEquals(tabsExtension, source)
                 Assert.assertNotEquals(null, extensionCreatedSession)
                 Assert.assertEquals(extensionCreatedSession, session)
                 onCloseRequestResult.complete(null)
                 return GeckoResult.ALLOW;
             }
-        }
+        })
 
-        sessionRule.runtime.webExtensionController.tabDelegate = tabDelegate
         tabsExtension = WebExtension(TABS_CREATE_REMOVE_BACKGROUND)
 
         sessionRule.waitForResult(sessionRule.runtime.registerWebExtension(tabsExtension))
         sessionRule.waitForResult(onCloseRequestResult)
 
         sessionRule.waitForResult(sessionRule.runtime.unregisterWebExtension(tabsExtension))
     }
 
@@ -192,28 +196,31 @@ class WebExtensionTest : BaseSessionTest
     // - Extension finds the tab by url and removes it
     // - TabDelegate handles closing of the tab
     // - Verify that request targets previously created GeckoSession
     @Test
     fun testBrowserTabsRemove() {
         val onCloseRequestResult = GeckoResult<Void>()
         val existingSession = sessionRule.createOpenSession()
 
-        val tabDelegate = object : WebExtensionController.TabDelegate {
+        existingSession.loadTestPath("$HELLO_HTML_PATH?tabToClose")
+        existingSession.waitForPageStop()
+
+        sessionRule.addExternalDelegateUntilTestEnd(
+                WebExtensionController.TabDelegate::class,
+                sessionRule.runtime.webExtensionController::setTabDelegate,
+                { sessionRule.runtime.webExtensionController.tabDelegate = null },
+                object : WebExtensionController.TabDelegate {
             override fun onCloseTab(source: WebExtension?, session: GeckoSession): GeckoResult<AllowOrDeny> {
                 Assert.assertEquals(existingSession, session)
                 onCloseRequestResult.complete(null)
                 return GeckoResult.ALLOW;
             }
-        }
+        })
 
-        existingSession.loadTestPath("$HELLO_HTML_PATH?tabToClose")
-        existingSession.waitForPageStop()
-
-        sessionRule.runtime.webExtensionController.tabDelegate = tabDelegate
         val tabsExtension = WebExtension(TABS_REMOVE_BACKGROUND)
 
         sessionRule.waitForResult(sessionRule.runtime.registerWebExtension(tabsExtension))
         sessionRule.waitForResult(onCloseRequestResult)
 
         sessionRule.waitForResult(sessionRule.runtime.unregisterWebExtension(tabsExtension))
     }
 
@@ -611,17 +618,36 @@ class WebExtensionTest : BaseSessionTest
         // Make sure the page loaded successfully
         sessionRule.waitForResult(pageStop)
 
         assertThat("Url should load WebExtension page", page, endsWith("/tab.html"))
 
         assertThat("WebExtension page should have access to privileged APIs",
             sessionRule.waitForResult(result), equalTo("HELLO_FROM_PAGE"))
 
-        sessionRule.waitForResult(sessionRule.runtime.unregisterWebExtension(extension))
+        // Test that after unregistering an extension, all its pages get closed
+        sessionRule.addExternalDelegateUntilTestEnd(
+                WebExtensionController.TabDelegate::class,
+                sessionRule.runtime.webExtensionController::setTabDelegate,
+                { sessionRule.runtime.webExtensionController.tabDelegate = null },
+                object : WebExtensionController.TabDelegate {})
+
+        val unregister = sessionRule.runtime.unregisterWebExtension(extension)
+
+        sessionRule.waitUntilCalled(object : WebExtensionController.TabDelegate {
+            @AssertCalled
+            override fun onCloseTab(source: WebExtension?,
+                                    session: GeckoSession): GeckoResult<AllowOrDeny> {
+                Assert.assertEquals(null, source)
+                Assert.assertEquals(mainSession, session)
+                return GeckoResult.ALLOW
+            }
+        })
+
+        sessionRule.waitForResult(unregister)
     }
 
     @Test
     fun badFileType() {
         testRegisterError("resource://android/bad/location/error",
                 "does not point to a folder or an .xpi")
     }