Bug 392984 - Feed that fails to load results in many messages in livemark, r=mano, a=blocker
authorphilringnalda@gmail.com
Tue, 04 Dec 2007 21:02:06 -0800
changeset 8737 0430866f569a36992d15835aef749bdf8691db2b
parent 8736 df5e0ebb3a588ce2288130b67b01f5df648220a6
child 8738 118b6ff0e26c1a2ce79e3d1980f2a7d39e403807
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherderautoland@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmano, blocker
bugs392984
milestone1.9b2pre
Bug 392984 - Feed that fails to load results in many messages in livemark, r=mano, a=blocker
toolkit/components/places/src/nsLivemarkService.js
--- a/toolkit/components/places/src/nsLivemarkService.js
+++ b/toolkit/components/places/src/nsLivemarkService.js
@@ -61,16 +61,17 @@ const LIVEMARK_TIMEOUT = 15000; // fire 
 const LIVEMARK_ICON_URI = "chrome://browser/skin/places/livemarkItem.png";
 const PLACES_BUNDLE_URI = 
   "chrome://browser/locale/places/places.properties";
 const DEFAULT_LOAD_MSG = "Live Bookmark loading...";
 const DEFAULT_FAIL_MSG = "Live Bookmark feed failed to load.";
 const LMANNO_FEEDURI = "livemark/feedURI";
 const LMANNO_SITEURI = "livemark/siteURI";
 const LMANNO_EXPIRATION = "livemark/expiration";
+const LMANNO_LOADFAILED = "livemark/loadfailed";
 
 const PS_CONTRACTID = "@mozilla.org/preferences-service;1";
 const NH_CONTRACTID = "@mozilla.org/browser/nav-history-service;1";
 const AS_CONTRACTID = "@mozilla.org/browser/annotation-service;1";
 const OS_CONTRACTID = "@mozilla.org/observer-service;1";
 const SB_CONTRACTID = "@mozilla.org/intl/stringbundle;1";
 const IO_CONTRACTID = "@mozilla.org/network/io-service;1";
 const BMS_CONTRACTID = "@mozilla.org/browser/nav-bookmarks-service;1";
@@ -105,16 +106,30 @@ function GetString(name)
       return gStringBundle.GetStringFromName(name);
   } catch (ex) {
     LOG("Exception loading string bundle: " + ex.message);
   }
 
   return null;
 }
 
+function MarkLivemarkLoadFailed(aFolderId) {
+  // if it failed before, too, nothing more to do
+  var ans = Cc[AS_CONTRACTID].getService(Ci.nsIAnnotationService);
+  if (ans.itemHasAnnotation(aFolderId, LMANNO_LOADFAILED))
+    return;
+
+  var failedMsg = GetString("bookmarksLivemarkFailed") || DEFAULT_FAIL_MSG;
+  var failedURI = gIoService.newURI("about:livemark-failed", null, null);
+  var bms = Cc[BMS_CONTRACTID].getService(Ci.nsINavBookmarksService);
+  bms.insertBookmark(aFolderId, failedURI, 0, failedMsg);
+  ans.setItemAnnotation(aFolderId, LMANNO_LOADFAILED, true, 0,
+                        ans.EXPIRE_NEVER);
+} 
+
 var gLivemarkService;
 function LivemarkService() {
 
   try {
     var prefs = Cc[PS_CONTRACTID].getService(Ci.nsIPrefBranch);
     var livemarkRefresh = 
       prefs.getIntPref("browser.bookmarks.livemark_refresh_seconds");
     // Reset global expiration variable to reflect hidden pref (in ms)
@@ -207,17 +222,17 @@ LivemarkService.prototype = {
       livemark.loadingId = bms.insertBookmark(livemark.folderId, loadingURI,
                                               0, this._loading);
   },
 
   _updateLivemarkChildren:
   function LS__updateLivemarkChildren(index, forceUpdate) {
     if (this._livemarks[index].locked)
       return;
-    
+
     var livemark = this._livemarks[index];
     livemark.locked = true;
     try {
       // Check the TTL/expiration on this.  If there isn't one,
       // then we assume it's never been loaded.  We perform this
       // check even when the update is being forced, in case the
       // livemark has somehow never been loaded.
       var exprTime = this._ans.getPageAnnotation(livemark.feedURI,
@@ -238,40 +253,43 @@ LivemarkService.prototype = {
       if (idleTime > IDLE_TIMELIMIT)
       {
         livemark.locked = false;
         return;
       }
     }
     catch (ex) {
       // This livemark has never been loaded, since it has no expire time.
-      this.insertLivemarkLoadingItem(this._bms, livemark);
     }
 
     var loadgroup;
     try {
       // Create a load group for the request.  This will allow us to
       // automatically keep track of redirects, so we can always
       // cancel the channel.
       loadgroup = Cc[LG_CONTRACTID].createInstance(Ci.nsILoadGroup);
       var uriChannel = gIoService.newChannel(livemark.feedURI.spec, null, null);
       uriChannel.loadGroup = loadgroup;
       uriChannel.loadFlags |= Ci.nsIRequest.LOAD_BACKGROUND | 
                               Ci.nsIRequest.VALIDATE_ALWAYS;
       var httpChannel = uriChannel.QueryInterface(Ci.nsIHttpChannel);
       httpChannel.requestMethod = "GET";
       httpChannel.setRequestHeader("X-Moz", "livebookmarks", false);
 
-      this.insertLivemarkLoadingItem(this._bms, livemark);
-
       // Stream the result to the feed parser with this listener
       var listener = new LivemarkLoadListener(livemark);
+      this.insertLivemarkLoadingItem(this._bms, livemark);
       httpChannel.asyncOpen(listener, null);
     }
     catch (ex) {
+      if (livemark.loadingId != -1) {
+        this._bms.removeItem(livemark.loadingId);
+        livemark.loadingId = -1;
+      }
+      MarkLivemarkLoadFailed(livemark.folderId);      
       livemark.locked = false;
       LOG("exception: " + ex);
       throw ex;
     }
     livemark.loadGroup = loadgroup;
   },
 
   createLivemark: function LS_createLivemark(folder, name, siteURI,
@@ -455,20 +473,16 @@ function LivemarkLoadListener(livemark) 
   this._processor = null;
   this._isAborted = false;
   this._ttl = gExpiration;
   this._ans = Cc[AS_CONTRACTID].getService(Ci.nsIAnnotationService);
 }
 
 LivemarkLoadListener.prototype = {
 
-  get _failed() {
-    return GetString("bookmarksLivemarkFailed") || DEFAULT_FAIL_MSG;
-  },
-
   abort: function LLL_abort() {
     this._isAborted = true;
   },
 
   get _bms() {
     if (!this.__bms)
       this.__bms = Cc[BMS_CONTRACTID].getService(Ci.nsINavBookmarksService);
     return this.__bms;
@@ -492,24 +506,25 @@ LivemarkLoadListener.prototype = {
     var lmService = Cc[LS_CONTRACTID].getService(Ci.nsILivemarkService);
 
     // Enforce well-formedness because the existing code does
     if (!result || !result.doc || result.bozo) {
       if (this._livemark.loadingId != -1) {
         this._bms.removeItem(this._livemark.loadingId);
         this._livemark.loadingId = -1;
       }
-
-      this.insertLivemarkFailedItem(this._livemark.folderId);
+      MarkLivemarkLoadFailed(this._livemark.folderId);
       this._ttl = gExpiration;
       throw Cr.NS_ERROR_FAILURE;
     }
 
     this.deleteLivemarkChildren(this._livemark.folderId);
     this._livemark.loadingId = -1;
+    // removeItemAnnotation can safely be used even when the anno isn't set
+    this._ans.removeItemAnnotation(this._livemark.folderId, LMANNO_LOADFAILED);
     var feed = result.doc.QueryInterface(Ci.nsIFeed);
     // Loop through and check for a link and a title
     // as the old code did
     for (var i = 0; i < feed.items.length; ++i) {
       let entry = feed.items.queryElementAt(i, Ci.nsIFeedEntry);
       let href = entry.link;
       if (!href)
         continue;
@@ -530,37 +545,37 @@ LivemarkLoadListener.prototype = {
     }
   },
 
   /**
    * See nsIFeedResultListener.idl
    */
   handleResult: function LLL_handleResult(result) {
     if (this._isAborted) {
+      if (this._livemark.loadingId != -1) {
+        this._bms.removeItem(this._livemark.loadingId);
+        this._livemark.loadingId = -1;
+      }
+      MarkLivemarkLoadFailed(this._livemark.folderId);
       this._livemark.locked = false;
       return;
     }
     try {
       // The actual work is done in runBatched, see above.
       this._bms.runInBatchMode(this, result);
     }
     finally {
       this._processor.listener = null;
       this._processor = null;
       this._livemark.locked = false;
     }
   },
   
   deleteLivemarkChildren: LivemarkService.prototype.deleteLivemarkChildren,
 
-  insertLivemarkFailedItem: function LS_insertLivemarkFailed(folderId) {
-    var failedURI = gIoService.newURI("about:livemark-failed", null, null);
-    var id = this._bms.insertBookmark(folderId, failedURI, 0, this._failed);
-  },
-
   insertLivemarkChild:
   function LS_insertLivemarkChild(folderId, uri, title) {
     var id = this._bms.insertBookmark(folderId, uri, this._bms.DEFAULT_INDEX,
                                       title);
   },
 
   /**
    * See nsIStreamListener.idl
@@ -591,16 +606,22 @@ LivemarkLoadListener.prototype = {
   /**
    * See nsIRequestObserver.idl
    */
   onStopRequest: function LLL_onStopRequest(request, context, status) {
     if (!Components.isSuccessCode(status)) {
       // Something went wrong; try to load again in a bit
       this._setResourceTTL(ERROR_EXPIRATION);
       this._isAborted = true;
+      if (this._livemark.loadingId != -1) {
+        this._bms.removeItem(this._livemark.loadingId);
+        this._livemark.loadingId = -1;
+      }
+      MarkLivemarkLoadFailed(this._livemark.folderId);
+      this._livemark.locked = false;
       return;
     }
     // Set an expiration on the livemark, for reloading the data
     try { 
       this._processor.onStopRequest(request, context, status);
 
       // Calculate a new ttl
       var channel = request.QueryInterface(Ci.nsICachingChannel);