Backed out changesets 48d6706662c0, 53236b4393f2, and 9125083b8dfe (bug 1268912, bug 1244597) for test_clients_engine.js perma-timeouts.
authorRyan VanderMeulen <ryanvm@gmail.com>
Fri, 15 Jul 2016 19:48:14 -0400
changeset 305225 98979093d45f152ee94a39ed97ccb02d8a8ddcb4
parent 305224 48d6706662c08c0aef5a657259fbaaf9c3176d1f
child 305226 0c27d48b278dba5493f9c98581acfd71889f7b63
push id79518
push usercbook@mozilla.com
push dateSun, 17 Jul 2016 08:09:59 +0000
treeherdermozilla-inbound@711963e8daa3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1268912, 1244597
milestone50.0a1
backs out48d6706662c08c0aef5a657259fbaaf9c3176d1f
53236b4393f2da5da9925b511d6e702860f9319f
9125083b8dfeb57a6ac6b16e26a020987e3e7295
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
Backed out changesets 48d6706662c0, 53236b4393f2, and 9125083b8dfe (bug 1268912, bug 1244597) for test_clients_engine.js perma-timeouts. CLOSED TREE
browser/base/content/test/general/browser_misused_characters_in_strings.js
browser/components/nsBrowserGlue.js
browser/locales/en-US/chrome/browser/accounts.properties
services/cloudsync/CloudSyncLocal.jsm
services/sync/locales/en-US/sync.properties
services/sync/modules/engines/clients.js
services/sync/modules/util.js
services/sync/tests/unit/head_helpers.js
--- a/browser/base/content/test/general/browser_misused_characters_in_strings.js
+++ b/browser/base/content/test/general/browser_misused_characters_in_strings.js
@@ -95,16 +95,20 @@ let gWhitelist = [{
   }, {
     file: "netError.dtd",
     key: "inadequateSecurityError.longDesc",
     type: "single-quote"
   }, {
     file: "netErrorApp.dtd",
     key: "securityOverride.warningContent",
     type: "single-quote"
+  }, {
+    file: "sync.properties",
+    key: "client.name2",
+    type: "apostrophe"
   }
 ];
 
 var moduleLocation = gTestPath.replace(/\/[^\/]*$/i, "/parsingTestHelpers.jsm");
 var {generateURIsFromDirTree} = Cu.import(moduleLocation, {});
 
 /**
  * Check if an error should be ignored due to matching one of the whitelist
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -310,18 +310,18 @@ BrowserGlue.prototype = {
         this._setSyncAutoconnectDelay();
         break;
       case "fxaccounts:onverified":
         this._showSyncStartedDoorhanger();
         break;
       case "fxaccounts:device_disconnected":
         this._onDeviceDisconnected();
         break;
-      case "weave:engine:clients:display-uris":
-        this._onDisplaySyncURIs(subject);
+      case "weave:engine:clients:display-uri":
+        this._onDisplaySyncURI(subject);
         break;
       case "session-save":
         this._setPrefToSaveSession(true);
         subject.QueryInterface(Ci.nsISupportsPRBool);
         subject.data = true;
         break;
       case "places-init-complete":
         if (!this._migrationImportsDefaultBookmarks)
@@ -529,17 +529,17 @@ BrowserGlue.prototype = {
     os.addObserver(this, "quit-application-granted", false);
     if (OBSERVE_LASTWINDOW_CLOSE_TOPICS) {
       os.addObserver(this, "browser-lastwindow-close-requested", false);
       os.addObserver(this, "browser-lastwindow-close-granted", false);
     }
     os.addObserver(this, "weave:service:ready", false);
     os.addObserver(this, "fxaccounts:onverified", false);
     os.addObserver(this, "fxaccounts:device_disconnected", false);
-    os.addObserver(this, "weave:engine:clients:display-uris", false);
+    os.addObserver(this, "weave:engine:clients:display-uri", false);
     os.addObserver(this, "session-save", false);
     os.addObserver(this, "places-init-complete", false);
     this._isPlacesInitObserver = true;
     os.addObserver(this, "places-database-locked", false);
     this._isPlacesLockedObserver = true;
     os.addObserver(this, "distribution-customization-complete", false);
     os.addObserver(this, "places-shutdown", false);
     this._isPlacesShutdownObserver = true;
@@ -596,17 +596,17 @@ BrowserGlue.prototype = {
     os.removeObserver(this, "restart-in-safe-mode");
     if (OBSERVE_LASTWINDOW_CLOSE_TOPICS) {
       os.removeObserver(this, "browser-lastwindow-close-requested");
       os.removeObserver(this, "browser-lastwindow-close-granted");
     }
     os.removeObserver(this, "weave:service:ready");
     os.removeObserver(this, "fxaccounts:onverified");
     os.removeObserver(this, "fxaccounts:device_disconnected");
-    os.removeObserver(this, "weave:engine:clients:display-uris");
+    os.removeObserver(this, "weave:engine:clients:display-uri");
     os.removeObserver(this, "session-save");
     if (this._bookmarksBackupIdleTime) {
       this._idleService.removeIdleObserver(this, this._bookmarksBackupIdleTime);
       delete this._bookmarksBackupIdleTime;
     }
     if (this._isPlacesInitObserver)
       os.removeObserver(this, "places-init-complete");
     if (this._isPlacesLockedObserver)
@@ -2429,69 +2429,34 @@ BrowserGlue.prototype = {
       return;
     }
 
     let chromeWindow = RecentWindow.getMostRecentBrowserWindow();
     chromeWindow.openPreferences(...args);
   },
 
   /**
-   * Called as an observer when Sync's "display URIs" notification is fired.
+   * Called as an observer when Sync's "display URI" notification is fired.
+   *
+   * We open the received URI in a background tab.
    *
-   * We open the received URIs in background tabs.
+   * Eventually, this will likely be replaced by a more robust tab syncing
+   * feature. This functionality is considered somewhat evil by UX because it
+   * opens a new tab automatically without any prompting. However, it is a
+   * lesser evil than sending a tab to a specific device (from e.g. Fennec)
+   * and having nothing happen on the receiving end.
    */
-  _onDisplaySyncURIs: function _onDisplaySyncURIs(data) {
+  _onDisplaySyncURI: function _onDisplaySyncURI(data) {
     try {
+      let tabbrowser = RecentWindow.getMostRecentBrowserWindow({private: false}).gBrowser;
+
       // The payload is wrapped weirdly because of how Sync does notifications.
-      const URIs = data.wrappedJSObject.object;
-
-      const findWindow = () => RecentWindow.getMostRecentBrowserWindow({private: false});
-
-      // win can be null, but it's ok, we'll assign it later in openTab()
-      let win = findWindow();
-
-      const openTab = URI => {
-        let tab;
-        if (!win) {
-          Services.appShell.hiddenDOMWindow.open(URI.uri);
-          win = findWindow();
-          tab = win.gBrowser.tabs[0];
-        } else {
-          tab = win.gBrowser.addTab(URI.uri);
-        }
-        tab.setAttribute("attention", true);
-        return tab;
-      };
-
-      const firstTab = openTab(URIs[0]);
-      URIs.slice(1).forEach(URI => openTab(URI));
-
-      let title, body;
-      const deviceName = Weave.Service.clientsEngine.getClientName(URIs[0].clientId);
-      const bundle = Services.strings.createBundle("chrome://browser/locale/accounts.properties");
-      if (URIs.length == 1) {
-        title = bundle.GetStringFromName("tabArrivingNotification.title");
-        const pageTitle = URIs[0].title || firstTab.linkedBrowser.contentTitle
-                          || URIs[0].uri;
-        body = bundle.formatStringFromName("tabArrivingNotification.body", [pageTitle, deviceName], 2);
-      } else {
-        title = bundle.GetStringFromName("tabsArrivingNotification.title");
-        const tabArrivingBody = URIs.every(URI => URI.clientId == URIs[0].clientId) ?
-                                "tabsArrivingNotification.body" : "tabsArrivingNotificationMultiple.body";
-        body = bundle.formatStringFromName(tabArrivingBody, [URIs.length, deviceName], 2);
-      }
-
-      const clickCallback = (subject, topic, data) => {
-        if (topic == "alertclickcallback") {
-          win.gBrowser.selectedTab = firstTab;
-        }
-      }
-      AlertsService.showAlertNotification(null, title, body, true, null, clickCallback);
+      tabbrowser.addTab(data.wrappedJSObject.object.uri);
     } catch (ex) {
-      Cu.reportError("Error displaying tab(s) received by Sync: " + ex);
+      Cu.reportError("Error displaying tab received by Sync: " + ex);
     }
   },
 
   _onDeviceDisconnected() {
     let bundle = Services.strings.createBundle("chrome://browser/locale/accounts.properties");
     let title = bundle.GetStringFromName("deviceDisconnectedNotification.title");
     let body = bundle.GetStringFromName("deviceDisconnectedNotification.body");
 
--- a/browser/locales/en-US/chrome/browser/accounts.properties
+++ b/browser/locales/en-US/chrome/browser/accounts.properties
@@ -32,23 +32,8 @@ syncStartNotification.body = Firefox wil
 # LOCALIZATION NOTE (deviceDisconnectedNotification.title, deviceDisconnectedNotification.body)
 # These strings are used in a notification shown after Sync was disconnected remotely.
 deviceDisconnectedNotification.title = Sync disconnected
 deviceDisconnectedNotification.body = This computer has been successfully disconnected from Firefox Sync.
 
 # LOCALIZATION NOTE (sendTabToAllDevices.menuitem)
 # Displayed in the Send Tabs context menu when right clicking a tab, a page or a link.
 sendTabToAllDevices.menuitem = All Devices
-
-# LOCALIZATION NOTE (tabArrivingNotification.title, tabArrivingNotification.body,
-# tabsArrivingNotification.title, tabsArrivingNotification.body)
-# These strings are used in a notification shown when we're opening tab(s) another device sent us to display.
-tabArrivingNotification.title = Tab received
-# LOCALIZATION NOTE (tabArrivingNotification.body) %1 is the title of the tab and %2 is the device name.
-tabArrivingNotification.body = “%1$S” has arrived from %2$S.
-
-tabsArrivingNotification.title = Multiple tabs received
-# LOCALIZATION NOTE (tabsArrivingNotification.body) %1 is the number of tabs received and %2 is the device name.
-tabsArrivingNotification.body = %1$S tabs have arrived from %2$S.
-# LOCALIZATION NOTE (tabsArrivingNotificationMultiple.body)
-# This string is used in a notification shown when we're opening tab(s) that several devices sent us to display.
-# %S is the number of tabs received
-tabsArrivingNotificationMultiple.body = %S tabs have arrived from your connected devices.
--- a/services/cloudsync/CloudSyncLocal.jsm
+++ b/services/cloudsync/CloudSyncLocal.jsm
@@ -72,16 +72,16 @@ Local.prototype = {
     let system =
       // 'device' is defined on unix systems
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("device") ||
       // hostname of the system, usually assigned by the user or admin
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("host") ||
       // fall back on ua info string
       Cc["@mozilla.org/network/protocol;1?name=http"].getService(Ci.nsIHttpProtocolHandler).oscpu;
 
-    return this.name = Str.sync.get("client.name3", [user, appName, system]);
+    return this.name = Str.sync.get("client.name2", [user, appName, system]);
   },
 
   set name(value) {
     this.prefs.set("client.name", value);
   },
 };
 
--- a/services/sync/locales/en-US/sync.properties
+++ b/services/sync/locales/en-US/sync.properties
@@ -1,14 +1,14 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 # %1: the user name (Ed), %2: the app name (Firefox), %3: the operating system (Android)
-client.name3 = %1$S’s %2$S on %3$S
+client.name2 = %1$S's %2$S on %3$S
 
 # %S is the date and time at which the last sync successfully completed
 lastSync2.label = Last sync: %S
 
 # signInToSync.description is the tooltip for the Sync buttons when Sync is
 # not configured.
 signInToSync.description = Sign In To Sync
 
--- a/services/sync/modules/engines/clients.js
+++ b/services/sync/modules/engines/clients.js
@@ -378,17 +378,16 @@ ClientEngine.prototype = {
 
       // Immediately clear out the commands as we've got them locally.
       this.clearCommands();
 
       // Process each command in order.
       if (!commands) {
         return true;
       }
-      let URIsToDisplay = [];
       for (let key in commands) {
         let {command, args} = commands[key];
         this._log.debug("Processing command: " + command + "(" + args + ")");
 
         let engines = [args[0]];
         switch (command) {
           case "resetAll":
             engines = null;
@@ -401,27 +400,23 @@ ClientEngine.prototype = {
             // Fallthrough
           case "wipeEngine":
             this.service.wipeClient(engines);
             break;
           case "logout":
             this.service.logout();
             return false;
           case "displayURI":
-            let [uri, clientId, title] = args;
-            URIsToDisplay.push({ uri, clientId, title });
+            this._handleDisplayURI.apply(this, args);
             break;
           default:
             this._log.debug("Received an unknown command: " + command);
             break;
         }
       }
-      if (URIsToDisplay.length) {
-        this._handleDisplayURIs(URIsToDisplay);
-      }
 
       return true;
     })();
   },
 
   /**
    * Validates and sends a command to a client or all clients.
    *
@@ -481,40 +476,42 @@ ClientEngine.prototype = {
     this._log.info("Sending URI to client: " + uri + " -> " +
                    clientId + " (" + title + ")");
     this.sendCommand("displayURI", [uri, this.localID, title], clientId);
 
     this._tracker.score += SCORE_INCREMENT_XLARGE;
   },
 
   /**
-   * Handle a bunch of received 'displayURI' commands.
+   * Handle a single received 'displayURI' command.
    *
-   * Interested parties should observe the "weave:engine:clients:display-uris"
-   * topic. The callback will receive an array as the subject parameter
-   * containing objects with the following keys:
+   * Interested parties should observe the "weave:engine:clients:display-uri"
+   * topic. The callback will receive an object as the subject parameter with
+   * the following keys:
    *
    *   uri       URI (string) that is requested for display.
    *   clientId  ID of client that sent the command.
    *   title     Title of page that loaded URI (likely) corresponds to.
    *
    * The 'data' parameter to the callback will not be defined.
    *
-   * @param uris
-   *        An array containing URI objects to display
-   * @param uris[].uri
+   * @param uri
    *        String URI that was received
-   * @param uris[].clientId
+   * @param clientId
    *        ID of client that sent URI
-   * @param uris[].title
+   * @param title
    *        String title of page that URI corresponds to. Older clients may not
    *        send this.
    */
-  _handleDisplayURIs: function _handleDisplayURIs(uris) {
-    Svc.Obs.notify("weave:engine:clients:display-uris", uris);
+  _handleDisplayURI: function _handleDisplayURI(uri, clientId, title) {
+    this._log.info("Received a URI for display: " + uri + " (" + title +
+                   ") from " + clientId);
+
+    let subject = {uri: uri, client: clientId, title: title};
+    Svc.Obs.notify("weave:engine:clients:display-uri", subject);
   },
 
   _removeRemoteClient(id) {
     delete this._store._remoteClients[id];
     this._tracker.removeChangedID(id);
   },
 };
 
--- a/services/sync/modules/util.js
+++ b/services/sync/modules/util.js
@@ -666,17 +666,17 @@ this.Utils = {
     let system =
       // 'device' is defined on unix systems
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("device") ||
       // hostname of the system, usually assigned by the user or admin
       Cc["@mozilla.org/system-info;1"].getService(Ci.nsIPropertyBag2).get("host") ||
       // fall back on ua info string
       Cc["@mozilla.org/network/protocol;1?name=http"].getService(Ci.nsIHttpProtocolHandler).oscpu;
 
-    return Str.sync.get("client.name3", [user, appName, system]);
+    return Str.sync.get("client.name2", [user, appName, system]);
   },
 
   getDeviceName() {
     const deviceName = Svc.Prefs.get("client.name", "");
 
     if (deviceName === "") {
       return this.getDefaultDeviceName();
     }
--- a/services/sync/tests/unit/head_helpers.js
+++ b/services/sync/tests/unit/head_helpers.js
@@ -1,15 +1,14 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 Cu.import("resource://services-common/async.js");
 Cu.import("resource://testing-common/services/common/utils.js");
 Cu.import("resource://testing-common/PlacesTestUtils.jsm");
-Cu.import("resource://services-sync/util.js");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyGetter(this, 'SyncPingSchema', function() {
   let ns = {};
   Cu.import("resource://gre/modules/FileUtils.jsm", ns);
   let stream = Cc["@mozilla.org/network/file-input-stream;1"]
                .createInstance(Ci.nsIFileInputStream);
   let jsonReader = Cc["@mozilla.org/dom/json;1"]
@@ -386,17 +385,8 @@ function sync_engine_and_validate_telem(
     }
     if (caughtError) {
       Svc.Obs.notify("weave:service:sync:error", caughtError);
     } else {
       Svc.Obs.notify("weave:service:sync:finish");
     }
   });
 }
-
-// Avoid an issue where `client.name3` containing unicode characters causes
-// a number of tests to fail, due to them assuming that we do not need to utf-8
-// encode or decode data sent through the mocked server (see bug 1268912).
-Utils.getDefaultDeviceName = function() {
-  return "Test device name";
-};
-
-