Bug 1246076 (part 2) - optionally display a remote tab's favicon if available. r=mak
authorMark Hammond <mhammond@skippinet.com.au>
Mon, 22 Feb 2016 15:43:47 +1100
changeset 321358 8a4eed67be3b2a60bace43c79d49253247769962
parent 321357 dbb6c118f854db9492438957af5765115b8411fe
child 321359 675ef3c3fb457fff86b31ee0cc8b74d7fb5066d7
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1246076
milestone47.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 1246076 (part 2) - optionally display a remote tab's favicon if available. r=mak
toolkit/components/places/PlacesRemoteTabsAutocompleteProvider.jsm
toolkit/components/places/tests/unifiedcomplete/test_remotetabmatches.js
--- a/toolkit/components/places/PlacesRemoteTabsAutocompleteProvider.jsm
+++ b/toolkit/components/places/PlacesRemoteTabsAutocompleteProvider.jsm
@@ -58,41 +58,59 @@ let _items = null;
 // Ensure the cache is good.
 function ensureItems() {
   if (!_items) {
     _items = buildItems();
   }
   return _items;
 }
 
-// An observer to invalidate _items.
+// A preference used to disable the showing of icons in remote tab records.
+const PREF_SHOW_REMOTE_ICONS = "services.sync.syncedTabs.showRemoteIcons";
+let showRemoteIcons;
+
+// An observer to invalidate _items and watch for changed prefs.
 function observe(subject, topic, data) {
   switch (topic) {
     case "weave:engine:sync:finish":
       if (data == "tabs") {
         // The tabs engine just finished syncing, so may have a different list
         // of tabs then we previously cached.
         _items = null;
       }
       break;
 
     case "weave:service:start-over":
       // Sync is being reset due to the user disconnecting - we must invalidate
       // the cache so we don't supply tabs from a different user.
       _items = null;
       break;
 
+    case "nsPref:changed":
+      if (data == PREF_SHOW_REMOTE_ICONS) {
+        try {
+          showRemoteIcons = Services.prefs.getBoolPref(PREF_SHOW_REMOTE_ICONS);
+        } catch(_) {
+          showRemoteIcons = true; // no such pref - default is to show the icons.
+        }
+      }
+      break;
+
     default:
       break;
   }
 }
 
 Services.obs.addObserver(observe, "weave:engine:sync:finish", false);
 Services.obs.addObserver(observe, "weave:service:start-over", false);
 
+// Observe the pref for showing remote icons and prime our bool that reflects its value.
+Services.prefs.addObserver(PREF_SHOW_REMOTE_ICONS, observe, false);
+observe(null, "nsPref:changed", PREF_SHOW_REMOTE_ICONS);
+
 // This public object is a static singleton.
 this.PlacesRemoteTabsAutocompleteProvider = {
   // a promise that resolves with an array of matching remote tabs.
   getMatches(searchString) {
     // If Sync isn't configured we bail early.
     if (!Services.prefs.prefHasUserValue("services.sync.username")) {
       return Promise.resolve([]);
     }
@@ -100,20 +118,20 @@ this.PlacesRemoteTabsAutocompleteProvide
     let re = new RegExp(escapeRegExp(searchString), "i");
     let matches = [];
     let { tabs, clients } = ensureItems();
     for (let [url, { clientId, tab }] of tabs) {
       let title = tab.title;
       if (url.match(re) || (title && title.match(re))) {
         // lookup the client record.
         let client = clients.get(clientId);
+        let icon = showRemoteIcons ? tab.icon : null;
         // create the record we return for auto-complete.
         let record = {
-          url, title,
-          icon: tab.icon,
+          url, title, icon,
           deviceClass: Weave.Service.clientsEngine.isMobile(clientId) ? "mobile" : "desktop",
           deviceName: client.clientName,
         };
         matches.push(record);
       }
     }
     return Promise.resolve(matches);
   },
--- a/toolkit/components/places/tests/unifiedcomplete/test_remotetabmatches.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_remotetabmatches.js
@@ -49,16 +49,17 @@ function configureEngine(clients) {
 }
 
 // Make a match object suitable for passing to check_autocomplete.
 function makeRemoteTabMatch(url, deviceName, extra = {}) {
   return {
     uri: makeActionURI("remotetab", {url, deviceName}),
     title: extra.title || url,
     style: [ "action" ],
+    icon: extra.icon,
   }
 }
 
 // The tests.
 add_task(function* test_nomatch() {
   // Nothing matches.
   configureEngine({
     guid_desktop: {
@@ -110,22 +111,49 @@ add_task(function* test_maximal() {
   });
 
   yield check_autocomplete({
     search: "ex",
     searchParam: "enable-actions",
     matches: [ makeSearchMatch("ex", { heuristic: true }),
                makeRemoteTabMatch("http://example.com/", "My Phone",
                                   { title: "An Example",
-                                    icon: "moz-anno:favicon:http://favicon"
+                                    icon: "moz-anno:favicon:http://favicon/"
                                   }),
              ],
   });
 });
 
+add_task(function* test_noShowIcons() {
+  Services.prefs.setBoolPref("services.sync.syncedTabs.showRemoteIcons", false);
+  configureEngine({
+    guid_mobile: {
+      clientName: "My Phone",
+      tabs: [{
+        urlHistory: ["http://example.com/"],
+        title: "An Example",
+        icon: "http://favicon",
+      }],
+    }
+  });
+
+  yield check_autocomplete({
+    search: "ex",
+    searchParam: "enable-actions",
+    matches: [ makeSearchMatch("ex", { heuristic: true }),
+               makeRemoteTabMatch("http://example.com/", "My Phone",
+                                  { title: "An Example",
+                                    // expecting the default favicon due to that pref.
+                                    icon: PlacesUtils.favicons.defaultFavicon.spec,
+                                  }),
+             ],
+  });
+  Services.prefs.clearUserPref("services.sync.syncedTabs.showRemoteIcons");
+});
+
 add_task(function* test_matches_title() {
   // URL doesn't match search expression, should still match the title.
   configureEngine({
     guid_mobile: {
       clientName: "My Phone",
       tabs: [{
         urlHistory: ["http://foo.com/"],
         title: "An Example",