Bug 1513162 - Use guid with known prefix for bookmarks from distribution.ini. r=mak,mkaply
authorHector Zhao <bzhao@mozilla.com>
Thu, 10 Jan 2019 20:54:53 +0000
changeset 453428 0117afe25cfe6a78af26a2a352cc60bf5cbbc1e4
parent 453427 ee1c9afb353f2b5bffe498b7a71c60e9b78927cc
child 453429 45d4ae4551a1e07cb5fa97d8405e6f9badcf7554
push id35357
push usernerli@mozilla.com
push dateFri, 11 Jan 2019 21:54:07 +0000
treeherdermozilla-central@0ce024c91511 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, mkaply
bugs1513162
milestone66.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 1513162 - Use guid with known prefix for bookmarks from distribution.ini. r=mak,mkaply Also stop setting keyword with distribution.ini. Differential Revision: https://phabricator.services.mozilla.com/D15489
browser/components/distribution.js
browser/components/enterprisepolicies/helpers/BookmarksPolicies.jsm
browser/components/places/tests/unit/distribution.ini
browser/components/places/tests/unit/test_browserGlue_distribution.js
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
--- a/browser/components/distribution.js
+++ b/browser/components/distribution.js
@@ -17,16 +17,21 @@ ChromeUtils.defineModuleGetter(this, "Pr
                                "resource://gre/modules/Preferences.jsm");
 ChromeUtils.defineModuleGetter(this, "PlacesUtils",
                                "resource://gre/modules/PlacesUtils.jsm");
 
 function DistributionCustomizer() {
 }
 
 DistributionCustomizer.prototype = {
+  // These prefixes must only contain characters
+  // allowed by PlacesUtils.isValidGuid
+  BOOKMARK_GUID_PREFIX: "DstB-",
+  FOLDER_GUID_PREFIX: "DstF-",
+
   get _iniFile() {
     // For parallel xpcshell testing purposes allow loading the distribution.ini
     // file from the profile folder through an hidden pref.
     let loadFromProfile = Services.prefs.getBoolPref("distribution.testing.loadFromProfile", false);
 
     let iniFile;
     try {
       iniFile = loadFromProfile ? Services.dirsvc.get("ProfD", Ci.nsIFile)
@@ -142,16 +147,17 @@ DistributionCustomizer.prototype = {
         break;
 
       case "folder":
         if (itemIndex < defaultIndex)
           index = prependIndex++;
 
         let folder = await PlacesUtils.bookmarks.insert({
           type: PlacesUtils.bookmarks.TYPE_FOLDER,
+          guid: PlacesUtils.generateGuidWithPrefix(this.FOLDER_GUID_PREFIX),
           parentGuid, index, title: item.title,
         });
 
         await this._parseBookmarksSection(folder.guid,
                                           "BookmarksFolder-" + item.folderId);
         break;
 
       case "separator":
@@ -180,16 +186,17 @@ DistributionCustomizer.prototype = {
         break;
 
       case "bookmark":
       default:
         if (itemIndex < defaultIndex)
           index = prependIndex++;
 
         await PlacesUtils.bookmarks.insert({
+          guid: PlacesUtils.generateGuidWithPrefix(this.BOOKMARK_GUID_PREFIX),
           parentGuid, index, title: item.title, url: item.link,
         });
 
         if (item.icon && item.iconData) {
           try {
             let faviconURI = Services.io.newURI(item.icon);
             PlacesUtils.favicons.replaceFaviconDataFromDataURL(
               faviconURI, item.iconData, 0,
@@ -199,25 +206,16 @@ DistributionCustomizer.prototype = {
               Services.io.newURI(item.link), faviconURI, false,
               PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE, null,
               Services.scriptSecurityManager.getSystemPrincipal());
           } catch (e) {
             Cu.reportError(e);
           }
         }
 
-        if (item.keyword) {
-          try {
-            await PlacesUtils.keywords.insert({ keyword: item.keyword,
-                                                url: item.link });
-          } catch (e) {
-            Cu.reportError(e);
-          }
-        }
-
         break;
       }
     }
   },
 
   _newProfile: false,
   _customizationsApplied: false,
   applyCustomizations: function DIST_applyCustomizations() {
--- a/browser/components/enterprisepolicies/helpers/BookmarksPolicies.jsm
+++ b/browser/components/enterprisepolicies/helpers/BookmarksPolicies.jsm
@@ -190,17 +190,17 @@ async function calculateLists(specifiedB
 
 async function insertBookmark(bookmark) {
   let parentGuid = await getParentGuid(bookmark.Placement,
                                        bookmark.Folder);
 
   await PlacesUtils.bookmarks.insert({
     url: Services.io.newURI(bookmark.URL.href),
     title: bookmark.Title,
-    guid: generateGuidWithPrefix(BookmarksPolicies.BOOKMARK_GUID_PREFIX),
+    guid: PlacesUtils.generateGuidWithPrefix(BookmarksPolicies.BOOKMARK_GUID_PREFIX),
     parentGuid,
   });
 
   if (bookmark.Favicon) {
     await setFaviconForBookmark(bookmark).catch(
       () => log.error(`Error setting favicon for ${bookmark.Title}`));
   }
 }
@@ -241,23 +241,16 @@ async function setFaviconForBookmark(boo
       false, /* forceReload */
       PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE,
       resolve,
       nullPrincipal
     );
   });
 }
 
-function generateGuidWithPrefix(prefix) {
-  // Generates a random GUID and replace its beginning with the given
-  // prefix. We do this instead of just prepending the prefix to keep
-  // the correct character length.
-  return prefix + PlacesUtils.history.makeGuid().substring(prefix.length);
-}
-
 // Cache of folder names to guids to be used by the getParentGuid
 // function. The name consists in the parentGuid (which should always
 // be the menuGuid or the toolbarGuid) + the folder title. This is to
 // support having the same folder name in both the toolbar and menu.
 XPCOMUtils.defineLazyGetter(this, "gFoldersMapPromise", () => {
   return new Promise(resolve => {
     let foldersMap = new Map();
     return PlacesUtils.bookmarks.fetch(
@@ -285,17 +278,17 @@ async function getParentGuid(placement, 
 
   let foldersMap = await gFoldersMapPromise;
   let folderName = `${parentGuid}|${folderTitle}`;
 
   if (foldersMap.has(folderName)) {
     return foldersMap.get(folderName);
   }
 
-  let guid = generateGuidWithPrefix(BookmarksPolicies.FOLDER_GUID_PREFIX);
+  let guid = PlacesUtils.generateGuidWithPrefix(BookmarksPolicies.FOLDER_GUID_PREFIX);
   await PlacesUtils.bookmarks.insert({
     type: PlacesUtils.bookmarks.TYPE_FOLDER,
     title: folderTitle,
     guid,
     parentGuid,
   });
 
   foldersMap.set(folderName, guid);
--- a/browser/components/places/tests/unit/distribution.ini
+++ b/browser/components/places/tests/unit/distribution.ini
@@ -4,24 +4,27 @@
 [Global]
 id=516444
 version=1.0
 about=Test distribution file
 
 [BookmarksToolbar]
 item.1.title=Toolbar Link Before
 item.1.link=https://example.org/toolbar/before/
-item.1.keyword=e:t:b
 item.1.icon=https://example.org/favicon.png
 item.1.iconData=data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==
 item.2.type=default
-item.3.title=Toolbar Link After
-item.3.link=https://example.org/toolbar/after/
-item.3.keyword=e:t:a
+item.3.type=folder
+item.3.title=Toolbar Folder After
+item.3.folderId=1
 
 [BookmarksMenu]
 item.1.title=Menu Link Before
 item.1.link=https://example.org/menu/before/
 item.1.icon=https://example.org/favicon.png
 item.1.iconData=data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVQI12NgAAAAAgAB4iG8MwAAAABJRU5ErkJggg==
 item.2.type=default
 item.3.title=Menu Link After
 item.3.link=https://example.org/menu/after/
+
+[BookmarksFolder-1]
+item.1.title=Toolbar Link Folder
+item.1.link=https://example.org/toolbar/folder/
--- a/browser/components/places/tests/unit/test_browserGlue_distribution.js
+++ b/browser/components/places/tests/unit/test_browserGlue_distribution.js
@@ -42,16 +42,19 @@ registerCleanupFunction(function() {
   iniFile.append("distribution.ini");
   if (iniFile.exists()) {
     iniFile.remove(false);
   }
   Assert.ok(!iniFile.exists());
 });
 
 add_task(async function() {
+  let {DistributionCustomizer} = ChromeUtils.import("resource:///modules/distribution.js", {});
+  let distribution = new DistributionCustomizer();
+
   let glue = Cc["@mozilla.org/browser/browserglue;1"].getService(Ci.nsIObserver);
   // Initialize Places through the History Service and check that a new
   // database has been created.
   Assert.equal(PlacesUtils.history.databaseStatus,
                PlacesUtils.history.DATABASE_STATUS_CREATE);
   // Force distribution.
   glue.observe(null, TOPIC_BROWSERGLUE_TEST, TOPICDATA_DISTRIBUTION_CUSTOMIZATION);
 
@@ -59,62 +62,55 @@ add_task(async function() {
   await promiseTopicObserved(TOPIC_CUSTOMIZATION_COMPLETE);
 
   // Check the custom bookmarks exist on menu.
   let menuItem = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.menuGuid,
     index: 0,
   });
   Assert.equal(menuItem.title, "Menu Link Before");
+  Assert.ok(menuItem.guid.startsWith(distribution.BOOKMARK_GUID_PREFIX),
+    "Guid of this bookmark has expected prefix");
 
   menuItem = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.menuGuid,
     index: 1 + DEFAULT_BOOKMARKS_ON_MENU,
   });
   Assert.equal(menuItem.title, "Menu Link After");
 
-  // Check no favicon or keyword exists for this bookmark
+  // Check no favicon exists for this bookmark
   await Assert.rejects(waitForResolvedPromise(() => {
     return PlacesUtils.promiseFaviconData(menuItem.url.href);
   }, "Favicon not found", 10), /Favicon\snot\sfound/, "Favicon not found");
 
-  let keywordItem = await PlacesUtils.keywords.fetch({
-    url: menuItem.url.href,
-  });
-  Assert.strictEqual(keywordItem, null);
-
   // Check the custom bookmarks exist on toolbar.
   let toolbarItem = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,
     index: 0,
   });
   Assert.equal(toolbarItem.title, "Toolbar Link Before");
 
-  // Check the custom favicon and keyword exist for this bookmark
+  // Check the custom favicon exist for this bookmark
   let faviconItem = await waitForResolvedPromise(() => {
     return PlacesUtils.promiseFaviconData(toolbarItem.url.href);
   }, "Favicon not found", 10);
   Assert.equal(faviconItem.uri.spec, "https://example.org/favicon.png");
   Assert.greater(faviconItem.dataLen, 0);
   Assert.equal(faviconItem.mimeType, "image/png");
 
   let base64Icon = "data:image/png;base64," +
       base64EncodeString(String.fromCharCode.apply(String, faviconItem.data));
   Assert.equal(base64Icon, SMALLPNG_DATA_URI.spec);
 
-  keywordItem = await PlacesUtils.keywords.fetch({
-    url: toolbarItem.url.href,
-  });
-  Assert.notStrictEqual(keywordItem, null);
-  Assert.equal(keywordItem.keyword, "e:t:b");
-
   toolbarItem = await PlacesUtils.bookmarks.fetch({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,
     index: 1 + DEFAULT_BOOKMARKS_ON_TOOLBAR,
   });
-  Assert.equal(toolbarItem.title, "Toolbar Link After");
+  Assert.equal(toolbarItem.title, "Toolbar Folder After");
+  Assert.ok(toolbarItem.guid.startsWith(distribution.FOLDER_GUID_PREFIX),
+    "Guid of this folder has expected prefix");
 
   // Check the bmprocessed pref has been created.
   Assert.ok(Services.prefs.getBoolPref(PREF_BMPROCESSED));
 
   // Check distribution prefs have been created.
   Assert.equal(Services.prefs.getCharPref(PREF_DISTRIBUTION_ID), "516444");
 });
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -466,16 +466,28 @@ var PlacesUtils = {
    * @return (Boolean)
    */
    isValidGuidPrefix(guidPrefix) {
      return typeof guidPrefix == "string" && guidPrefix &&
             (/^[a-zA-Z0-9\-_]{1,11}$/.test(guidPrefix));
    },
 
   /**
+   * Generates a random GUID and replace its beginning with the given
+   * prefix. We do this instead of just prepending the prefix to keep
+   * the correct character length.
+   *
+   * @param prefix: (String)
+   * @return (String)
+   */
+  generateGuidWithPrefix(prefix) {
+    return prefix + this.history.makeGuid().substring(prefix.length);
+  },
+
+  /**
    * Converts a string or n URL object to an nsIURI.
    *
    * @param url (URL) or (String)
    *        the URL to convert.
    * @return nsIURI for the given URL.
    */
   toURI(url) {
     url = (url instanceof URL) ? url.href : url;
--- a/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
+++ b/toolkit/components/places/tests/bookmarks/test_bookmarks_fetch.js
@@ -189,39 +189,35 @@ add_task(async function fetch_separator(
   Assert.equal(bm2.type, PlacesUtils.bookmarks.TYPE_SEPARATOR);
   Assert.ok(!("url" in bm2));
   Assert.strictEqual(bm2.title, "");
 
   await PlacesUtils.bookmarks.remove(bm1.guid);
 });
 
 add_task(async function fetch_byguid_prefix() {
-  function generateGuidWithPrefix(prefix) {
-    return prefix + PlacesUtils.history.makeGuid().substring(prefix.length);
-  }
-
   const PREFIX = "PREFIX-";
 
   let bm1 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 guid: generateGuidWithPrefix(PREFIX),
+                                                 guid: PlacesUtils.generateGuidWithPrefix(PREFIX),
                                                  url: "http://bm1.example.com/",
                                                  title: "bookmark 1" });
   checkBookmarkObject(bm1);
   Assert.ok(bm1.guid.startsWith(PREFIX));
 
   let bm2 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-                                                 guid: generateGuidWithPrefix(PREFIX),
+                                                 guid: PlacesUtils.generateGuidWithPrefix(PREFIX),
                                                  url: "http://bm2.example.com/",
                                                  title: "bookmark 2" });
   checkBookmarkObject(bm2);
   Assert.ok(bm2.guid.startsWith(PREFIX));
 
   let bm3 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  type: PlacesUtils.bookmarks.TYPE_FOLDER,
-                                                 guid: generateGuidWithPrefix(PREFIX),
+                                                 guid: PlacesUtils.generateGuidWithPrefix(PREFIX),
                                                  title: "a folder" });
   checkBookmarkObject(bm3);
   Assert.ok(bm3.guid.startsWith(PREFIX));
 
   // Bookmark 4 doesn't have the same guid prefix, so it shouldn't be returned in the results.
   let bm4 = await PlacesUtils.bookmarks.insert({ parentGuid: PlacesUtils.bookmarks.unfiledGuid,
                                                  url: "http://bm3.example.com/",
                                                  title: "bookmark 4" });