Bug 1447526 - Flush tab state in TPS after adding a tab. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Tue, 20 Mar 2018 18:48:16 -0700
changeset 770354 46d1dd92fefca6da381d4c92bf2b370a22db0369
parent 769888 827c686c570935483c3ad8022aa92b9e005574c3
push id103389
push userbmo:tchiovoloni@mozilla.com
push dateWed, 21 Mar 2018 02:22:19 +0000
reviewersmarkh
bugs1447526
milestone61.0a1
Bug 1447526 - Flush tab state in TPS after adding a tab. r?markh MozReview-Commit-ID: 6LSi2ToGPUu
services/sync/tps/extensions/tps/resource/modules/tabs.jsm
services/sync/tps/extensions/tps/resource/tps.jsm
--- a/services/sync/tps/extensions/tps/resource/modules/tabs.jsm
+++ b/services/sync/tps/extensions/tps/resource/modules/tabs.jsm
@@ -6,16 +6,17 @@
    Components.utils.import() and acts as a singleton.
    Only the following listed symbols will exposed on import, and only when
    and where imported. */
 
 const EXPORTED_SYMBOLS = ["BrowserTabs"];
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://services-sync/main.js");
+ChromeUtils.import("resource:///modules/sessionstore/TabStateFlusher.jsm");
 
 // Unfortunately, due to where TPS is run, we can't directly reuse the logic from
 // BrowserTestUtils.jsm. Moreover, we can't resolve the URI it loads the content
 // frame script from ("chrome://mochikit/content/tests/BrowserTestUtils/content-utils.js"),
 // hence the hackiness here and in BrowserTabs.Add.
 Cc["@mozilla.org/globalmessagemanager;1"]
 .getService(Ci.nsIMessageListenerManager)
 .loadFrameScript("data:application/javascript;charset=utf-8," + encodeURIComponent(`
@@ -25,34 +26,39 @@ Cc["@mozilla.org/globalmessagemanager;1"
     sendAsyncMessage("tps:loadEvent", {subframe: subframe, url: event.target.documentURI});
   }, true)`), true);
 
 var BrowserTabs = {
   /**
    * Add
    *
    * Opens a new tab in the current browser window for the
-   * given uri.  Throws on error.
+   * given uri. Rejects on error.
    *
    * @param uri The uri to load in the new tab
-   * @return nothing
+   * @return Promise
    */
-  Add(uri, fn) {
-
-    // Open the uri in a new tab in the current browser window, and calls
-    // the callback fn from the tab's onload handler.
+  async Add(uri) {
     let mainWindow = Services.wm.getMostRecentWindow("navigator:browser");
     let browser = mainWindow.getBrowser();
-    let mm = browser.ownerGlobal.messageManager;
-    mm.addMessageListener("tps:loadEvent", function onLoad(msg) {
-      mm.removeMessageListener("tps:loadEvent", onLoad);
-      fn();
+    let newtab = browser.addTab(uri);
+
+    // Wait for the tab to load.
+    await new Promise(resolve => {
+      let mm = browser.ownerGlobal.messageManager;
+      mm.addMessageListener("tps:loadEvent", function onLoad(msg) {
+        mm.removeMessageListener("tps:loadEvent", onLoad);
+        resolve();
+      });
     });
-    let newtab = browser.addTab(uri);
+
     browser.selectedTab = newtab;
+    // We might sync before SessionStore is done recording information, so try
+    // and force it to record everything. This is overkill, but effective.
+    await TabStateFlusher.flushWindow(mainWindow);
   },
 
   /**
    * Find
    *
    * Finds the specified uri and title in Weave's list of remote tabs
    * for the specified profile.
    *
--- a/services/sync/tps/extensions/tps/resource/tps.jsm
+++ b/services/sync/tps/extensions/tps/resource/tps.jsm
@@ -318,19 +318,17 @@ var TPS = {
   },
 
   async HandleTabs(tabs, action) {
     for (let tab of tabs) {
       Logger.logInfo("executing action " + action.toUpperCase() +
                      " on tab " + JSON.stringify(tab));
       switch (action) {
         case ACTION_ADD:
-          await new Promise(resolve => {
-            BrowserTabs.Add(tab.uri, resolve);
-          });
+          await BrowserTabs.Add(tab.uri);
           break;
         case ACTION_VERIFY:
           Logger.AssertTrue(typeof(tab.profile) != "undefined",
             "profile must be defined when verifying tabs");
           Logger.AssertTrue(
             BrowserTabs.Find(tab.uri, tab.title, tab.profile), "error locating tab");
           break;
         case ACTION_VERIFY_NOT: