Bug 1410749 - Start tab ID numbering from #1. r=geckoview-reviewers,esawin
authorJan Henning <jh+bugzilla@buttercookie.de>
Mon, 15 Apr 2019 21:20:47 +0000
changeset 469759 5519079b4dc0
parent 469742 7339545396ae
child 469760 bdd3825511f3
push id35882
push usercbrindusan@mozilla.com
push dateWed, 17 Apr 2019 15:54:01 +0000
treeherdermozilla-central@37185c0ae520 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, esawin
bugs1410749
milestone68.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 1410749 - Start tab ID numbering from #1. r=geckoview-reviewers,esawin It's easier this way than fixing who knows how many Webextension APIs that have learned from Desktop that there is no tab #0 and that therefore refuse to work in our first tab. We'll also make a similar change to GeckoView's stub implementation of the tab API because that affects Custom Tabs and PWAs in Fennec for now. The tests for tab ID 0 are therefore no longer required - they were added in a previous attempt to fix the Webextension APIs themselves, which was ultimately never carried through to completion, though. Differential Revision: https://phabricator.services.mozilla.com/D26431
mobile/android/base/java/org/mozilla/gecko/Tabs.java
mobile/android/components/extensions/test/mochitest/test_ext_browserAction_getTitle_setTitle.html
mobile/android/components/extensions/test/mochitest/test_ext_tabs_sendMessage.html
mobile/android/modules/geckoview/GeckoViewTab.jsm
--- a/mobile/android/base/java/org/mozilla/gecko/Tabs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tabs.java
@@ -90,17 +90,19 @@ public class Tabs implements BundleEvent
     public static final int LOADURL_START_EDITING = 1 << 9;
 
     private static final long PERSIST_TABS_AFTER_MILLISECONDS = 1000 * 2;
 
     public static final int INVALID_TAB_ID = -1;
     // Used to indicate a new tab should be appended to the current tabs.
     public static final int NEW_LAST_INDEX = -1;
 
-    private static final AtomicInteger sTabId = new AtomicInteger(0);
+    // Bug 1410749: Desktop numbers tabs starting from 1, and various Webextension bits have
+    // inherited that assumption and treat 0 as an invalid tab.
+    private static final AtomicInteger sTabId = new AtomicInteger(1);
     private volatile boolean mInitialTabsAdded;
 
     private Context mAppContext;
     private EventDispatcher mEventDispatcher;
     private GeckoView mGeckoView;
     private ContentObserver mBookmarksContentObserver;
     private PersistTabsRunnable mPersistTabsRunnable;
     private int mPrivateClearColor;
--- a/mobile/android/components/extensions/test/mochitest/test_ext_browserAction_getTitle_setTitle.html
+++ b/mobile/android/components/extensions/test/mochitest/test_ext_browserAction_getTitle_setTitle.html
@@ -44,28 +44,16 @@ add_task(async function test_setTitle_an
       // Set the title for the new tab and test that getTitle returns the correct title.
       await browser.browserAction.setTitle({tabId: tab.id, title});
       tabTitle = await browser.browserAction.getTitle({tabId: tab.id});
       browser.test.assertEq(title, tabTitle, "Expected the new tab title to be returned");
 
       return tab;
     }
 
-    // Test on tabId 0
-    // Test that the default title is returned before the title is set for the tab.
-    let tabTitle = await browser.browserAction.getTitle({tabId: 0});
-    browser.test.assertEq("Browser Action", tabTitle,
-                          "Expected the default title to be returned for tabId 0");
-
-    // Set the title for the new tab and test that getTitle returns the correct title.
-    await browser.browserAction.setTitle({tabId: 0, title: "tab 0"});
-    tabTitle = await browser.browserAction.getTitle({tabId: 0});
-    browser.test.assertEq("tab 0", tabTitle,
-                          "Expected the new tab title to be returned for tabId 0");
-
     // Create and test 3 new tabs.
     let tab1 = await createAndTestNewTab("tab 1", "about:blank");
     let tab2 = await createAndTestNewTab("tab 2", "about:blank");
     let tab3 = await createAndTestNewTab("tab 3", "about:blank");
 
     // Test the default title again.
     let title = await browser.browserAction.getTitle({});
     browser.test.assertEq("Browser Action", title, "Expected the default title to be returned");
--- a/mobile/android/components/extensions/test/mochitest/test_ext_tabs_sendMessage.html
+++ b/mobile/android/components/extensions/test/mochitest/test_ext_tabs_sendMessage.html
@@ -232,56 +232,12 @@ add_task(async function tabsSendMessageN
   await Promise.all([
     extension.startup(),
     extension.awaitFinish("tabs.sendMessage"),
   ]);
 
   await extension.unload();
 });
 
-
-add_task(async function tabsSendAndReceiveMessageTabId0() {
-  let extension = ExtensionTestUtils.loadExtension({
-    manifest: {
-      "permissions": ["tabs", "<all_urls>"],
-    },
-
-    async background() {
-      function contentScriptCode() {
-        browser.runtime.onMessage.addListener(msg => {
-          browser.test.assertEq(msg, "message to tabId 0",
-                                "Got the expected message from the background page");
-
-          return Promise.resolve("reply to background page");
-        });
-        browser.runtime.sendMessage("message from tabId 0");
-      }
-
-      await browser.runtime.onMessage.addListener(async (msg, sender) => {
-        browser.test.assertEq("message from tabId 0", msg,
-                              "Got the expected message from a content script");
-        browser.test.assertTrue(sender.tab,
-                                "Got a sender.tab object as expected");
-        browser.test.assertEq(0, sender.tab.id,
-                              "Got a sender.tab object with tab.id == 0");
-
-        const reply = await browser.tabs.sendMessage(0, "message to tabId 0");
-
-        browser.test.assertEq("reply to background page", reply);
-
-        browser.test.notifyPass("tabs.messaging.tab0");
-      });
-
-      await browser.tabs.executeScript(0, {code: `new ${contentScriptCode}`});
-    },
-  });
-
-  await extension.startup();
-
-  await extension.awaitFinish("tabs.messaging.tab0");
-
-  await extension.unload();
-});
-
 </script>
 
 </body>
 </html>
--- a/mobile/android/modules/geckoview/GeckoViewTab.jsm
+++ b/mobile/android/modules/geckoview/GeckoViewTab.jsm
@@ -18,17 +18,20 @@ class Tab {
   getActive() {
     return this.browser.docShellIsActive;
   }
 }
 
 // Stub BrowserApp implementation for WebExtensions support.
 class GeckoViewTab extends GeckoViewModule {
   onInit() {
-    const tab = new Tab(0, this.browser);
+    // As this is only a stub implementation, we hardcode a single tab ID.
+    // Because of bug 1410749, we can't use 0, though, and just to be safe
+    // we choose a value that is unlikely to overlap with Fennec's tab IDs.
+    const tab = new Tab(10001, this.browser);
 
     this.window.gBrowser = this.window.BrowserApp = {
       selectedBrowser: this.browser,
       tabs: [tab],
       selectedTab: tab,
 
       closeTab: function(aTab) {
         // not implemented