Bug 885205 - Extremely slow reflect bookmarks menu items after restoring bookmarks. And disk access abnormal. And Firefox process does not disappear from task manager on exit. r=mak, a=bajaj
authorRaymond Lee <raymond@raysquare.com>
Fri, 05 Jul 2013 10:22:05 +0800
changeset 147875 d2db7b4b2b9eecadceb619c33e4936c633ec3ce7
parent 147874 9d8307a0266b4bc0afbf1632aa97d95bcf99cfb7
child 147876 ba6c735171209d7823e256c4f9e9729ab8ee558c
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, bajaj
bugs885205
milestone24.0a2
Bug 885205 - Extremely slow reflect bookmarks menu items after restoring bookmarks. And disk access abnormal. And Firefox process does not disappear from task manager on exit. r=mak, a=bajaj
toolkit/components/places/BookmarkJSONUtils.jsm
--- a/toolkit/components/places/BookmarkJSONUtils.jsm
+++ b/toolkit/components/places/BookmarkJSONUtils.jsm
@@ -80,17 +80,19 @@ this.BookmarkJSONUtils = Object.freeze({
    * @resolves an array containing of maps of old folder ids to new folder ids,
    *           and an array of saved search ids that need to be fixed up.
    *           eg: [[[oldFolder1, newFolder1]], [search1]]
    * @rejects JavaScript exception.
    */
   importJSONNode: function BJU_importJSONNode(aData, aContainer, aIndex,
                                               aGrandParentId) {
     let importer = new BookmarkImporter();
-    return importer.importJSONNode(aData, aContainer, aIndex, aGrandParentId);
+    // Can't make importJSONNode a task until we have made the
+    // runInBatchMode in BI_importFromJSON to run asynchronously. Bug 890203
+    return Promise.resolve(importer.importJSONNode(aData, aContainer, aIndex, aGrandParentId));
   },
 
   /**
    * Serializes the given node (and all its descendents) as JSON
    * and writes the serialization to the given output stream.
    *
    * @param   aNode
    *          An nsINavHistoryResultNode
@@ -263,66 +265,64 @@ BookmarkImporter.prototype = {
                 PlacesUtils.bookmarks.removeItem(rootItemId);
               }
             }
           }
 
           let searchIds = [];
           let folderIdMap = [];
 
-          Task.spawn(function() {
-            for (let node of batch.nodes) {
-              if (!node.children || node.children.length == 0)
-                continue; // Nothing to restore for this root
+          for (let node of batch.nodes) {
+            if (!node.children || node.children.length == 0)
+              continue; // Nothing to restore for this root
 
-              if (node.root) {
-                let container = PlacesUtils.placesRootId; // Default to places root
-                switch (node.root) {
-                  case "bookmarksMenuFolder":
-                    container = PlacesUtils.bookmarksMenuFolderId;
-                    break;
-                  case "tagsFolder":
-                    container = PlacesUtils.tagsFolderId;
-                    break;
-                  case "unfiledBookmarksFolder":
-                    container = PlacesUtils.unfiledBookmarksFolderId;
-                    break;
-                  case "toolbarFolder":
-                    container = PlacesUtils.toolbarFolderId;
-                    break;
-                }
+            if (node.root) {
+              let container = PlacesUtils.placesRootId; // Default to places root
+              switch (node.root) {
+                case "bookmarksMenuFolder":
+                  container = PlacesUtils.bookmarksMenuFolderId;
+                  break;
+                case "tagsFolder":
+                  container = PlacesUtils.tagsFolderId;
+                  break;
+                case "unfiledBookmarksFolder":
+                  container = PlacesUtils.unfiledBookmarksFolderId;
+                  break;
+                case "toolbarFolder":
+                  container = PlacesUtils.toolbarFolderId;
+                  break;
+              }
 
-                // Insert the data into the db
-                for (let child of node.children) {
-                  let index = child.index;
-                  let [folders, searches] =
-                    yield this.importJSONNode(child, container, index, 0);
-                  for (let i = 0; i < folders.length; i++) {
-                    if (folders[i])
-                      folderIdMap[i] = folders[i];
-                  }
-                  searchIds = searchIds.concat(searches);
+              // Insert the data into the db
+              for (let child of node.children) {
+                let index = child.index;
+                let [folders, searches] =
+                  this.importJSONNode(child, container, index, 0);
+                for (let i = 0; i < folders.length; i++) {
+                  if (folders[i])
+                    folderIdMap[i] = folders[i];
                 }
-              } else {
-                yield this.importJSONNode(
-                  node, PlacesUtils.placesRootId, node.index, 0);
+                searchIds = searchIds.concat(searches);
               }
+            } else {
+              this.importJSONNode(
+                node, PlacesUtils.placesRootId, node.index, 0);
             }
+          }
 
-            // Fixup imported place: uris that contain folders
-            searchIds.forEach(function(aId) {
-              let oldURI = PlacesUtils.bookmarks.getBookmarkURI(aId);
-              let uri = fixupQuery(oldURI, folderIdMap);
-              if (!uri.equals(oldURI)) {
-                PlacesUtils.bookmarks.changeBookmarkURI(aId, uri);
-              }
-            });
+          // Fixup imported place: uris that contain folders
+          searchIds.forEach(function(aId) {
+            let oldURI = PlacesUtils.bookmarks.getBookmarkURI(aId);
+            let uri = fixupQuery(oldURI, folderIdMap);
+            if (!uri.equals(oldURI)) {
+              PlacesUtils.bookmarks.changeBookmarkURI(aId, uri);
+            }
+          });
 
-            deferred.resolve();
-          }.bind(this));
+          deferred.resolve();
         }.bind(this)
       };
 
       PlacesUtils.bookmarks.runInBatchMode(batch, null);
     }
     return deferred.promise;
   },
 
@@ -336,148 +336,147 @@ BookmarkImporter.prototype = {
    * @param aIndex
    *        The index within the container the item was dropped or pasted at
    * @return an array containing of maps of old folder ids to new folder ids,
    *         and an array of saved search ids that need to be fixed up.
    *         eg: [[[oldFolder1, newFolder1]], [search1]]
    */
   importJSONNode: function BI_importJSONNode(aData, aContainer, aIndex,
                                              aGrandParentId) {
-    return Task.spawn(function() {
-      let folderIdMap = [];
-      let searchIds = [];
-      let id = -1;
-      switch (aData.type) {
-        case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER:
-          if (aContainer == PlacesUtils.tagsFolderId) {
-            // Node is a tag
-            if (aData.children) {
-              aData.children.forEach(function(aChild) {
-                try {
-                  PlacesUtils.tagging.tagURI(
-                    NetUtil.newURI(aChild.uri), [aData.title]);
-                } catch (ex) {
-                  // Invalid tag child, skip it
-                }
-              });
-              throw new Task.Result([folderIdMap, searchIds]);
+    let folderIdMap = [];
+    let searchIds = [];
+    let id = -1;
+    switch (aData.type) {
+      case PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER:
+        if (aContainer == PlacesUtils.tagsFolderId) {
+          // Node is a tag
+          if (aData.children) {
+            aData.children.forEach(function(aChild) {
+              try {
+                PlacesUtils.tagging.tagURI(
+                  NetUtil.newURI(aChild.uri), [aData.title]);
+              } catch (ex) {
+                // Invalid tag child, skip it
+              }
+            });
+            return [folderIdMap, searchIds];
+          }
+        } else if (aData.livemark && aData.annos) {
+          // Node is a livemark
+          let feedURI = null;
+          let siteURI = null;
+          aData.annos = aData.annos.filter(function(aAnno) {
+            switch (aAnno.name) {
+              case PlacesUtils.LMANNO_FEEDURI:
+                feedURI = NetUtil.newURI(aAnno.value);
+                return false;
+              case PlacesUtils.LMANNO_SITEURI:
+                siteURI = NetUtil.newURI(aAnno.value);
+                return false;
+              default:
+                return true;
             }
-          } else if (aData.livemark && aData.annos) {
-            // Node is a livemark
-            let feedURI = null;
-            let siteURI = null;
-            aData.annos = aData.annos.filter(function(aAnno) {
-              switch (aAnno.name) {
-                case PlacesUtils.LMANNO_FEEDURI:
-                  feedURI = NetUtil.newURI(aAnno.value);
-                  return false;
-                case PlacesUtils.LMANNO_SITEURI:
-                  siteURI = NetUtil.newURI(aAnno.value);
-                  return false;
-                default:
-                  return true;
+          });
+
+          if (feedURI) {
+            PlacesUtils.livemarks.addLivemark({
+              title: aData.title,
+              feedURI: feedURI,
+              parentId: aContainer,
+              index: aIndex,
+              lastModified: aData.lastModified,
+              siteURI: siteURI
+            }, function(aStatus, aLivemark) {
+              if (Components.isSuccessCode(aStatus)) {
+                let id = aLivemark.id;
+                if (aData.dateAdded)
+                  PlacesUtils.bookmarks.setItemDateAdded(id, aData.dateAdded);
+                if (aData.annos && aData.annos.length)
+                  PlacesUtils.setAnnotationsForItem(id, aData.annos);
               }
             });
-
-            if (feedURI) {
-              PlacesUtils.livemarks.addLivemark({
-                title: aData.title,
-                feedURI: feedURI,
-                parentId: aContainer,
-                index: aIndex,
-                lastModified: aData.lastModified,
-                siteURI: siteURI
-              }, function(aStatus, aLivemark) {
-                if (Components.isSuccessCode(aStatus)) {
-                  let id = aLivemark.id;
-                  if (aData.dateAdded)
-                    PlacesUtils.bookmarks.setItemDateAdded(id, aData.dateAdded);
-                  if (aData.annos && aData.annos.length)
-                    PlacesUtils.setAnnotationsForItem(id, aData.annos);
-                }
-              });
-            }
-          } else {
-            id = PlacesUtils.bookmarks.createFolder(
-                   aContainer, aData.title, aIndex);
-            folderIdMap[aData.id] = id;
-            // Process children
-            if (aData.children) {
-              for (let i = 0; i < aData.children.length; i++) {
-                let child = aData.children[i];
-                let [folders, searches] =
-                  yield this.importJSONNode(child, id, i, aContainer);
-                for (let j = 0; j < folders.length; j++) {
-                  if (folders[j])
-                    folderIdMap[j] = folders[j];
-                }
-                searchIds = searchIds.concat(searches);
+          }
+        } else {
+          id = PlacesUtils.bookmarks.createFolder(
+                 aContainer, aData.title, aIndex);
+          folderIdMap[aData.id] = id;
+          // Process children
+          if (aData.children) {
+            for (let i = 0; i < aData.children.length; i++) {
+              let child = aData.children[i];
+              let [folders, searches] =
+                this.importJSONNode(child, id, i, aContainer);
+              for (let j = 0; j < folders.length; j++) {
+                if (folders[j])
+                  folderIdMap[j] = folders[j];
               }
+              searchIds = searchIds.concat(searches);
             }
           }
-          break;
-        case PlacesUtils.TYPE_X_MOZ_PLACE:
-          id = PlacesUtils.bookmarks.insertBookmark(
-                 aContainer, NetUtil.newURI(aData.uri), aIndex, aData.title);
-          if (aData.keyword)
-            PlacesUtils.bookmarks.setKeywordForBookmark(id, aData.keyword);
-          if (aData.tags) {
-            let tags = aData.tags.split(", ");
-            if (tags.length)
-              PlacesUtils.tagging.tagURI(NetUtil.newURI(aData.uri), tags);
-          }
-          if (aData.charset) {
-            yield PlacesUtils.setCharsetForURI(
-              NetUtil.newURI(aData.uri), aData.charset);
-          }
-          if (aData.uri.substr(0, 6) == "place:")
-            searchIds.push(id);
-          if (aData.icon) {
-            try {
-              // Create a fake faviconURI to use (FIXME: bug 523932)
-              let faviconURI = NetUtil.newURI("fake-favicon-uri:" + aData.uri);
-              PlacesUtils.favicons.replaceFaviconDataFromDataURL(
-                faviconURI, aData.icon, 0);
-              PlacesUtils.favicons.setAndFetchFaviconForPage(
-                NetUtil.newURI(aData.uri), faviconURI, false,
-                PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE);
-            } catch (ex) {
-              Components.utils.reportError("Failed to import favicon data:" + ex);
-            }
+        }
+        break;
+      case PlacesUtils.TYPE_X_MOZ_PLACE:
+        id = PlacesUtils.bookmarks.insertBookmark(
+               aContainer, NetUtil.newURI(aData.uri), aIndex, aData.title);
+        if (aData.keyword)
+          PlacesUtils.bookmarks.setKeywordForBookmark(id, aData.keyword);
+        if (aData.tags) {
+          let tags = aData.tags.split(", ");
+          if (tags.length)
+            PlacesUtils.tagging.tagURI(NetUtil.newURI(aData.uri), tags);
+        }
+        if (aData.charset) {
+          PlacesUtils.annotations.setPageAnnotation(
+            NetUtil.newURI(aData.uri), PlacesUtils.CHARSET_ANNO, aData.charset,
+            0, Ci.nsIAnnotationService.EXPIRE_NEVER);
+        }
+        if (aData.uri.substr(0, 6) == "place:")
+          searchIds.push(id);
+        if (aData.icon) {
+          try {
+            // Create a fake faviconURI to use (FIXME: bug 523932)
+            let faviconURI = NetUtil.newURI("fake-favicon-uri:" + aData.uri);
+            PlacesUtils.favicons.replaceFaviconDataFromDataURL(
+              faviconURI, aData.icon, 0);
+            PlacesUtils.favicons.setAndFetchFaviconForPage(
+              NetUtil.newURI(aData.uri), faviconURI, false,
+              PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE);
+          } catch (ex) {
+            Components.utils.reportError("Failed to import favicon data:" + ex);
           }
-          if (aData.iconUri) {
-            try {
-              PlacesUtils.favicons.setAndFetchFaviconForPage(
-                NetUtil.newURI(aData.uri), NetUtil.newURI(aData.iconUri), false,
-                PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE);
-            } catch (ex) {
-              Components.utils.reportError("Failed to import favicon URI:" + ex);
-            }
+        }
+        if (aData.iconUri) {
+          try {
+            PlacesUtils.favicons.setAndFetchFaviconForPage(
+              NetUtil.newURI(aData.uri), NetUtil.newURI(aData.iconUri), false,
+              PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE);
+          } catch (ex) {
+            Components.utils.reportError("Failed to import favicon URI:" + ex);
           }
-          break;
-        case PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR:
-          id = PlacesUtils.bookmarks.insertSeparator(aContainer, aIndex);
-          break;
-        default:
-          // Unknown node type
-      }
+        }
+        break;
+      case PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR:
+        id = PlacesUtils.bookmarks.insertSeparator(aContainer, aIndex);
+        break;
+      default:
+        // Unknown node type
+    }
 
-      // Set generic properties, valid for all nodes
-      if (id != -1 && aContainer != PlacesUtils.tagsFolderId &&
-          aGrandParentId != PlacesUtils.tagsFolderId) {
-        if (aData.dateAdded)
-          PlacesUtils.bookmarks.setItemDateAdded(id, aData.dateAdded);
-        if (aData.lastModified)
-          PlacesUtils.bookmarks.setItemLastModified(id, aData.lastModified);
-        if (aData.annos && aData.annos.length)
-          PlacesUtils.setAnnotationsForItem(id, aData.annos);
-      }
+    // Set generic properties, valid for all nodes
+    if (id != -1 && aContainer != PlacesUtils.tagsFolderId &&
+        aGrandParentId != PlacesUtils.tagsFolderId) {
+      if (aData.dateAdded)
+        PlacesUtils.bookmarks.setItemDateAdded(id, aData.dateAdded);
+      if (aData.lastModified)
+        PlacesUtils.bookmarks.setItemLastModified(id, aData.lastModified);
+      if (aData.annos && aData.annos.length)
+        PlacesUtils.setAnnotationsForItem(id, aData.annos);
+    }
 
-      throw new Task.Result([folderIdMap, searchIds]);
-    }.bind(this));
+    return [folderIdMap, searchIds];
   }
 }
 
 function notifyObservers(topic) {
   Services.obs.notifyObservers(null, topic, "json");
 }
 
 /**