Bug 636917 - isLivemark may return the wrong result to annotations observers; r=dietrich
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 22 Mar 2011 10:57:01 -0400
changeset 63632 e4a1143ddbdf9d587d396e6e38640d8c6ea1d934
parent 63631 8a61e8610d6684005e3db4ec97bf9c9e9779d82e
child 63633 0c6c92d27f04ed21a5ad64c44cc6057f8a4a5051
push id19248
push usereakhgari@mozilla.com
push dateWed, 23 Mar 2011 23:19:35 +0000
treeherdermozilla-central@ab95ab9e389b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdietrich
bugs636917
milestone2.0b13pre
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 636917 - isLivemark may return the wrong result to annotations observers; r=dietrich
toolkit/components/places/src/nsLivemarkService.js
toolkit/components/places/tests/unit/test_bug636917_isLivemark.js
--- a/toolkit/components/places/src/nsLivemarkService.js
+++ b/toolkit/components/places/src/nsLivemarkService.js
@@ -366,16 +366,20 @@ LivemarkService.prototype = {
     return folderId;
   },
 
   _createFolder:
   function LS__createFolder(aParentId, aName, aSiteURI, aFeedURI, aIndex) {
     var folderId = bms.createFolder(aParentId, aName, aIndex);
     bms.setFolderReadonly(folderId, true);
 
+    // Annotate this folder as being the last created livemark.  This is needed
+    // by isLivemark since it is not aware of this livemark till _pushLivemark
+    // is called, later during the addition path.
+    this._lastCreatedLivemarkFolderId = folderId;
     // Add an annotation to map the folder id to the livemark feed URI
     ans.setItemAnnotation(folderId, LMANNO_FEEDURI, aFeedURI.spec,
                           0, ans.EXPIRE_NEVER);
 
     if (aSiteURI) {
       // Add an annotation to map the folder URI to the livemark site URI
       this._setSiteURISecure(folderId, aFeedURI, aSiteURI);
     }
@@ -386,16 +390,21 @@ LivemarkService.prototype = {
   isLivemark: function LS_isLivemark(aFolderId) {
     if (aFolderId < 1)
       throw Cr.NS_ERROR_INVALID_ARG;
     try {
       this._getLivemarkIndex(aFolderId);
       return true;
     }
     catch (ex) {}
+    // There is an edge case here, if a AnnotationChanged notification asks for
+    // isLivemark and the livemark is currently being added, it is not yet in
+    // the _livemarks array.  In such a case go the slow path.
+    if (this._lastCreatedLivemarkFolderId === aFolderId)
+      return ans.itemHasAnnotation(aFolderId, LMANNO_FEEDURI);
     return false;
   },
 
   getLivemarkIdForFeedURI: function LS_getLivemarkIdForFeedURI(aFeedURI) {
     if (!(aFeedURI instanceof Ci.nsIURI))
       throw Cr.NS_ERROR_INVALID_ARG;
 
     for (var i = 0; i < this._livemarks.length; ++i) {
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/unit/test_bug636917_isLivemark.js
@@ -0,0 +1,40 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that asking for isLivemark in a annotationChanged notification
+// correctly returns true.
+function run_test()
+{
+  do_test_pending();
+
+  let annoObserver = {
+    onItemAnnotationSet:
+    function AO_onItemAnnotationSet(aItemId, aAnnotationName)
+    {
+      if (aAnnotationName == PlacesUtils.LMANNO_FEEDURI) {
+        PlacesUtils.annotations.removeObserver(this);
+        do_check_true(PlacesUtils.livemarks.isLivemark(aItemId));
+        do_execute_soon(function () {
+          PlacesUtils.bookmarks.removeItem(aItemId);
+          do_check_false(PlacesUtils.livemarks.isLivemark(aItemId));
+          do_test_finished();
+        });
+      }
+    },
+  
+    onItemAnnotationRemoved: function () {},
+    onPageAnnotationSet: function() {},
+    onPageAnnotationRemoved: function() {},
+    QueryInterface: XPCOMUtils.generateQI([
+      Ci.nsIAnnotationObserver
+    ]),
+  }
+  PlacesUtils.annotations.addObserver(annoObserver, false);
+  PlacesUtils.livemarks.createLivemarkFolderOnly(
+    PlacesUtils.unfiledBookmarksFolderId,
+    "livemark title",
+    uri("http://example.com/"),
+    uri("http://example.com/rdf"),
+    PlacesUtils.bookmarks.DEFAULT_INDEX
+  );
+}