Bug 1439220 - Align smart bookmark initialisation with browser. r=IanN a=IanN
authorFrank-Rainer Grahl <frgrahl@gmx.net>
Mon, 07 May 2018 12:41:29 +0200
changeset 31465 faa0beaa7193
parent 31464 6320dec21ce6
child 31466 85fef84f3604
push id383
push userclokep@gmail.com
push date2018-05-07 21:52 +0000
reviewersIanN, IanN
bugs1439220, 1437040, 1094888
Bug 1439220 - Align smart bookmark initialisation with browser. r=IanN a=IanN Port Bug 1437040 [Remove synchronous Bookmarks::GetItemIndex" to SeaMonkey]. Port Bug 1094888 [Smart bookmarks creation in nsBrowserGlue should use the new Bookmarks.jsm API].
suite/common/public/nsISuiteGlue.idl
suite/common/src/nsSuiteGlue.js
--- a/suite/common/public/nsISuiteGlue.idl
+++ b/suite/common/public/nsISuiteGlue.idl
@@ -18,30 +18,25 @@ interface nsIDOMWindow;
  * Dued to its nature and origin, this interface won't probably be the most
  * elegant or stable in the mozilla codebase, but its aim is rather pragmatic:
  * 1) reducing the performance overhead which affects browser window load;
  * 2) allow global hooks (e.g. startup and shutdown observers) which survive
  * suite windows to accomplish suite-related activities, such as shutdown
  * sanitization (see bug #284086)
  */
 
-[scriptable, uuid(2559bb36-2eef-4190-9df1-8afcfbd35bd3)]
+[scriptable, uuid(b3a787fd-4c05-4518-98e3-20bc10a0f586)]
 interface nsISuiteGlue : nsISupports
 {
   /**
    * Opens the Download Manager.
    */
   void showDownloadManager();
 
   /**
    * Deletes privacy sensitive data according to user preferences
    *
    * @param aParentWindow an optionally null window which is the parent of the
    *        sanitization dialog (if it has to be shown per user preferences)
    *
    */
   void sanitize(in nsIDOMWindow aParentWindow);
-
-  /**
-   * Add Smart Bookmarks special queries to bookmarks menu and toolbar folder.
-   */
-  void ensurePlacesDefaultQueriesInitialized();
 };
--- a/suite/common/src/nsSuiteGlue.js
+++ b/suite/common/src/nsSuiteGlue.js
@@ -1020,25 +1020,28 @@ SuiteGlue.prototype = {
         }
         else {
           // ...otherwise we will restore defaults.
           restoreDefaultBookmarks = true;
         }
       }
     }
 
-    // If bookmarks are not imported, then initialize smart bookmarks.  This
+    // If bookmarks are not imported, then initialize smart bookmarks. This
     // happens during a common startup.
-    // Otherwise, if any kind of import runs, smart bookmarks creation should be
-    // delayed till the import operations has finished.  Not doing so would
+    // Otherwise, if any kind of import runs, smart bookmarks creation should
+    // be delayed till the import operations has finished. Not doing so would
     // cause them to be overwritten by the newly imported bookmarks.
     if (!importBookmarks) {
-      this.ensurePlacesDefaultQueriesInitialized();
-    }
-    else {
+      try {
+        await this.ensurePlacesDefaultQueriesInitialized();
+      } catch (e) {
+        Cu.reportError(e);
+      }
+    } else {
       // An import operation is about to run.
       // Don't try to recreate smart bookmarks if autoExportHTML is true or
       // smart bookmarks are disabled.
       var autoExportHTML = false;
       try {
         autoExportHTML = Services.prefs.getBoolPref("browser.bookmarks.autoExportHTML");
       } catch(ex) {}
       var smartBookmarksVersion = 0;
@@ -1065,17 +1068,17 @@ SuiteGlue.prototype = {
         try {
           BookmarkHTMLUtils.importFromURL(bookmarksURI.spec, true).then(null,
             function onFailure() {
               Cu.reportError("Bookmarks.html file could be corrupt.");
             }
           ).then(
             // Ensure that smart bookmarks are created once the operation is
             // complete.
-            this.ensurePlacesDefaultQueriesInitialized.bind(this)
+            await this.ensurePlacesDefaultQueriesInitialized.bind(this)
           );
         }
         catch(ex) {
           Cu.reportError("bookmarks.html file could be corrupt. " + ex);
         }
       }
       else {
         Cu.reportError("Unable to find bookmarks.html file.");
@@ -1370,159 +1373,149 @@ SuiteGlue.prototype = {
 
   sanitize: function(aParentWindow)
   {
     // call the Sanitizer object's sanitize, which might return errors
     // but do not forward them anywhere, as we are defined as void here
     Sanitizer.sanitize(aParentWindow);
   },
 
-  ensurePlacesDefaultQueriesInitialized:
-  function BG_ensurePlacesDefaultQueriesInitialized() {
-    // This is actual version of the smart bookmarks, must be increased every
-    // time smart bookmarks change.
+  async ensurePlacesDefaultQueriesInitialized() {
+    // This is the current smart bookmarks version, it must be increased every
+    // time they change.
     // When adding a new smart bookmark below, its newInVersion property must
-    // be set to the version it has been added in, we will compare its value
+    // be set to the version it has been added in.  We will compare its value
     // to users' smartBookmarksVersion and add new smart bookmarks without
     // recreating old deleted ones.
-    const SMART_BOOKMARKS_VERSION = 4;
+    const SMART_BOOKMARKS_VERSION = 7;
     const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";
     const SMART_BOOKMARKS_PREF = "browser.places.smartBookmarksVersion";
 
     // TODO bug 399268: should this be a pref?
     const MAX_RESULTS = 10;
 
     // Get current smart bookmarks version.  If not set, create them.
-    let smartBookmarksCurrentVersion = 0;
-    try {
-      smartBookmarksCurrentVersion = Services.prefs.getIntPref(SMART_BOOKMARKS_PREF);
-    } catch(ex) {}
+    let smartBookmarksCurrentVersion = Services.prefs.getIntPref(SMART_BOOKMARKS_PREF, 0);
 
-    // If version is current or smart bookmarks are disabled, just bail out.
+    // If version is current, or smart bookmarks are disabled, bail out.
     if (smartBookmarksCurrentVersion == -1 ||
         smartBookmarksCurrentVersion >= SMART_BOOKMARKS_VERSION) {
       return;
     }
 
-    let batch = {
-      runBatched: function BG_EPDQI_runBatched() {
-        let menuIndex = 0;
-        let toolbarIndex = 0;
-        let bundle = Services.strings.createBundle("chrome://communicator/locale/places/places.properties");
+    try {
+      let menuIndex = 0;
+      let toolbarIndex = 0;
+      let bundle = Services.strings.createBundle("chrome://communicator/locale/places/places.properties");
+      let queryOptions = Ci.nsINavHistoryQueryOptions;
 
-        let smartBookmarks = {
-          MostVisited: {
-            title: bundle.GetStringFromName("mostVisitedTitle"),
-            uri: NetUtil.newURI("place:sort=" +
-                                Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING +
-                                "&maxResults=" + MAX_RESULTS),
-            parent: PlacesUtils.toolbarFolderId,
-            position: toolbarIndex++,
-            newInVersion: 1
-          },
-          RecentlyBookmarked: {
-            title: bundle.GetStringFromName("recentlyBookmarkedTitle"),
-            uri: NetUtil.newURI("place:folder=BOOKMARKS_MENU" +
-                                "&folder=UNFILED_BOOKMARKS" +
-                                "&folder=TOOLBAR" +
-                                "&queryType=" +
-                                Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
-                                "&sort=" +
-                                Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING +
-                                "&maxResults=" + MAX_RESULTS +
-                                "&excludeQueries=1"),
-            parent: PlacesUtils.bookmarksMenuFolderId,
-            position: menuIndex++,
-            newInVersion: 1
-          },
-          RecentTags: {
-            title: bundle.GetStringFromName("recentTagsTitle"),
-            uri: NetUtil.newURI("place:"+
-                                "type=" +
-                                Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY +
-                                "&sort=" +
-                                Ci.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING +
-                                "&maxResults=" + MAX_RESULTS),
-            parent: PlacesUtils.bookmarksMenuFolderId,
-            position: menuIndex++,
-            newInVersion: 1
-          }
-        };
+      let smartBookmarks = {
+        MostVisited: {
+          title: bundle.GetStringFromName("mostVisitedTitle"),
+          url: "place:sort=" + queryOptions.SORT_BY_VISITCOUNT_DESCENDING +
+               "&maxResults=" + MAX_RESULTS,
+          parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+          newInVersion: 1
+        },
+        RecentlyBookmarked: {
+          title: bundle.GetStringFromName("recentlyBookmarkedTitle"),
+          url: "place:folder=BOOKMARKS_MENU" + "&folder=UNFILED_BOOKMARKS" +
+               "&folder=TOOLBAR" +
+               "&queryType=" + queryOptions.QUERY_TYPE_BOOKMARKS +
+               "&sort=" + queryOptions.SORT_BY_DATEADDED_DESCENDING +
+               "&maxResults=" + MAX_RESULTS +
+               "&excludeQueries=1",
+          parentGuid: PlacesUtils.bookmarks.menuGuid,
+          newInVersion: 1
+        },
+        RecentTags: {
+          title: bundle.GetStringFromName("recentTagsTitle"),
+          url: "place:type=" + queryOptions.RESULTS_AS_TAG_QUERY +
+               "&sort=" + queryOptions.SORT_BY_LASTMODIFIED_DESCENDING +
+               "&maxResults=" + MAX_RESULTS,
+          parentGuid: PlacesUtils.bookmarks.menuGuid,
+          newInVersion: 1
+        },
+      };
 
-        // Set current itemId, parent and position if Smart Bookmark exists,
-        // we will use these informations to create the new version at the same
-        // position.
-        let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO);
-        smartBookmarkItemIds.forEach(function (itemId) {
-          let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
-          if (queryId in smartBookmarks) {
-            let smartBookmark = smartBookmarks[queryId];
-            smartBookmark.itemId = itemId;
-            smartBookmark.parent = PlacesUtils.bookmarks.getFolderIdForItem(itemId);
-            smartBookmark.position = PlacesUtils.bookmarks.getItemIndex(itemId);
-          } else {
-            // We don't remove old Smart Bookmarks because user could still
-            // find them useful, or could have personalized them.
-            // Instead we remove the Smart Bookmark annotation.
-            PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
-          }
-        });
+      // Set current guid, parentGuid and index of existing Smart Bookmarks.
+      // We will use those to create a new version of the bookmark at the same
+      // position.
+      let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO);
+      for (let itemId of smartBookmarkItemIds) {
+        let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
+        if (queryId in smartBookmarks) {
+          // Known smart bookmark.
+          let smartBookmark = smartBookmarks[queryId];
+          smartBookmark.guid = await PlacesUtils.promiseItemGuid(itemId);
 
-        for (let queryId in smartBookmarks) {
-          let smartBookmark = smartBookmarks[queryId];
-
-          // We update or create only changed or new smart bookmarks.
-          // Also we respect user choices, so we won't try to create a smart
-          // bookmark if it has been removed.
-          if (smartBookmarksCurrentVersion > 0 &&
-              smartBookmark.newInVersion <= smartBookmarksCurrentVersion &&
-              !smartBookmark.itemId)
+          if (!smartBookmark.url) {
+            await PlacesUtils.bookmarks.remove(smartBookmark.guid);
             continue;
-
-          // Remove old version of the smart bookmark if it exists, since it
-          // will be replaced in place.
-          if (smartBookmark.itemId) {
-            PlacesUtils.bookmarks.removeItem(smartBookmark.itemId);
           }
 
-          // Create the new smart bookmark and store its updated itemId.
-          smartBookmark.itemId =
-            PlacesUtils.bookmarks.insertBookmark(smartBookmark.parent,
-                                                 smartBookmark.uri,
-                                                 smartBookmark.position,
-                                                 smartBookmark.title);
-          PlacesUtils.annotations.setItemAnnotation(smartBookmark.itemId,
-                                                    SMART_BOOKMARKS_ANNO,
-                                                    queryId, 0,
-                                                    PlacesUtils.annotations.EXPIRE_NEVER);
+          let bm = await PlacesUtils.bookmarks.fetch(smartBookmark.guid);
+          smartBookmark.parentGuid = bm.parentGuid;
+          smartBookmark.index = bm.index;
+        } else {
+          // We don't remove old Smart Bookmarks because user could still
+          // find them useful, or could have personalized them.
+          // Instead we remove the Smart Bookmark annotation.
+          PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
+        }
+      }
+
+      for (let queryId of Object.keys(smartBookmarks)) {
+        let smartBookmark = smartBookmarks[queryId];
+
+        // We update or create only changed or new smart bookmarks.
+        // Also we respect user choices, so we won't try to create a smart
+        // bookmark if it has been removed.
+        if (smartBookmarksCurrentVersion > 0 &&
+            smartBookmark.newInVersion <= smartBookmarksCurrentVersion &&
+            !smartBookmark.guid || !smartBookmark.url)
+          continue;
+
+        // Remove old version of the smart bookmark if it exists, since it
+        // will be replaced in place.
+        if (smartBookmark.guid) {
+          await PlacesUtils.bookmarks.remove(smartBookmark.guid);
         }
 
-        // If we are creating all Smart Bookmarks from ground up, add a
-        // separator below them in the bookmarks menu.
-        if (smartBookmarksCurrentVersion == 0 &&
-            smartBookmarkItemIds.length == 0) {
-          let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId,
-                                                        menuIndex);
-          // Don't add a separator if the menu was empty or there is one already.
-          if (id != -1 &&
-              PlacesUtils.bookmarks.getItemType(id) != PlacesUtils.bookmarks.TYPE_SEPARATOR) {
-            PlacesUtils.bookmarks.insertSeparator(PlacesUtils.bookmarksMenuFolderId,
-                                                  menuIndex);
-          }
+        // Create the new smart bookmark and store its updated guid.
+        if (!("index" in smartBookmark)) {
+          if (smartBookmark.parentGuid == PlacesUtils.bookmarks.toolbarGuid)
+            smartBookmark.index = toolbarIndex++;
+          else if (smartBookmark.parentGuid == PlacesUtils.bookmarks.menuGuid)
+            smartBookmark.index = menuIndex++;
+        }
+        smartBookmark = await PlacesUtils.bookmarks.insert(smartBookmark);
+        let itemId = await PlacesUtils.promiseItemId(smartBookmark.guid);
+        PlacesUtils.annotations.setItemAnnotation(itemId,
+                                                  SMART_BOOKMARKS_ANNO,
+                                                  queryId, 0,
+                                                  PlacesUtils.annotations.EXPIRE_NEVER);
+      }
+
+      // If we are creating all Smart Bookmarks from ground up, add a
+      // separator below them in the bookmarks menu.
+      if (smartBookmarksCurrentVersion == 0 &&
+          smartBookmarkItemIds.length == 0) {
+        let bm = await PlacesUtils.bookmarks.fetch({ parentGuid: PlacesUtils.bookmarks.menuGuid,
+                                                     index: menuIndex });
+        // Don't add a separator if the menu was empty or there is one already.
+        if (bm && bm.type != PlacesUtils.bookmarks.TYPE_SEPARATOR) {
+          await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_SEPARATOR,
+                                               parentGuid: PlacesUtils.bookmarks.menuGuid,
+                                               index: menuIndex });
         }
       }
-    };
-
-    try {
-      PlacesUtils.bookmarks.runInBatchMode(batch, null);
-    }
-    catch(ex) {
+    } catch (ex) {
       Cu.reportError(ex);
-    }
-    finally {
+    } finally {
       Services.prefs.setIntPref(SMART_BOOKMARKS_PREF, SMART_BOOKMARKS_VERSION);
       Services.prefs.savePrefFile(null);
     }
   },
 
   /**
    * Called as an observer when Sync's "display URI" notification is fired.
    */