Bug 599973 (part 1) - Don't use steps for async favicons.

r=sdwilsh a=blocking

browser/components/certerror/content/aboutCertError.xhtml

@@ -63,7 +63,7 @@
          If the location of the favicon is changed for both pages, the
          FAVICON_ERRORPAGE_URL symbol in toolkit/components/places/src/nsFaviconService.h
          should be updated. If this page starts using a different favicon
-         than neterrorm nsFaviconService->DoSetAndLoadFaviconForPage
+         than neterror.xhtml nsFaviconService->SetAndLoadFaviconForPage
          should be updated to ignore this one as well. -->
     <link rel="icon" type="image/png" id="favicon" href="chrome://global/skin/icons/warning-16.png"/>
 

toolkit/components/places/src/AsyncFaviconHelpers.h

@@ -36,28 +36,15 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
-/**
- * How to use this stepper:
- *
- * nsCOMPtr<AsyncFaviconStepper> stepper = new AsyncFaviconStepper(callback);
- * stepper->SetPageURI(aPageURI);
- * stepper->SetIconURI(aFaviconURI);
- * rv = stepper->AppendStep(new SomeStep());
- * NS_ENSURE_SUCCESS(rv, rv);
- * rv = stepper->AppendStep(new SomeOtherStep());
- * NS_ENSURE_SUCCESS(rv, rv);
- * rv = stepper->Start();
- * NS_ENSURE_SUCCESS(rv, rv);
- */
-
 #ifndef AsyncFaviconHelpers_h_
 #define AsyncFaviconHelpers_h_
 
 #include "nsCOMPtr.h"
 #include "nsCOMArray.h"
 #include "nsIURI.h"
+#include "nsThreadUtils.h"
 
-#include "nsIFaviconService.h"
+#include "nsFaviconService.h"
 #include "Helpers.h"
 
 #include "mozilla/storage.h"
@@ -66,34 +53,6 @@
 #include "nsIInterfaceRequestor.h"
 #include "nsIStreamListener.h"
 
-#include "nsCycleCollectionParticipant.h"
-
-// Avoid warnings about macro empty parameters.
-#define FAVICONSTEP_FAKE_EMPTYPARAM
-
-#define FAVICONSTEP_FAIL_IF_FALSE(_cond) \
-  FAVICONSTEP_FAIL_IF_FALSE_RV(_cond, FAVICONSTEP_FAKE_EMPTYPARAM)
-
-#define FAVICONSTEP_FAIL_IF_FALSE_RV(_cond, _rv) \
-  PR_BEGIN_MACRO \
-  if (!(_cond)) { \
-    NS_WARNING("AsyncFaviconStep failed!"); \
-    mStepper->Failure(); \
-    return _rv; \
-  } \
-  PR_END_MACRO
-
-#define FAVICONSTEP_CANCEL_IF_TRUE(_cond, _notify) \
-  FAVICONSTEP_CANCEL_IF_TRUE_RV(_cond, _notify, FAVICONSTEP_FAKE_EMPTYPARAM)
-
-#define FAVICONSTEP_CANCEL_IF_TRUE_RV(_cond, _notify, _rv) \
-  PR_BEGIN_MACRO \
-  if (_cond) { \
-    mStepper->Cancel(_notify); \
-    return _rv; \
-  } \
-  PR_END_MACRO
-
 #define ICON_STATUS_UNKNOWN 0
 #define ICON_STATUS_CHANGED 1 << 0
 #define ICON_STATUS_SAVED 1 << 1
@@ -102,289 +61,229 @@
 namespace mozilla {
 namespace places {
 
-
-// Forward declarations.
-class AsyncFaviconStepperInternal;
-class AsyncFaviconStepper;
-
-
 /**
- * Executes a single async step on a favicon resource.
+ * Indicates when a icon should be fetched from network.
- * Once done, call backs to the stepper to proceed to the next step.
  */
-class AsyncFaviconStep : public AsyncStatementCallback
-{
-public:
-  AsyncFaviconStep() {}
-
-  /**
-   * Associate this step to a stepper.
-   *
-   * Automatically called by the stepper when the step is added to it.
-   * @see AsyncFaviconStepper::appendStep
-   */
-  void SetStepper(AsyncFaviconStepperInternal* aStepper) { mStepper = aStepper; }
-
-  /**
-   * Executes the step.  Virtual since it MUST be overridden.
-   */
-  virtual void Run() {};
-
-protected:
-  nsCOMPtr<AsyncFaviconStepperInternal> mStepper;
-};
-
-/**
- * Status definitions for the stepper.
- */
-enum AsyncFaviconStepperStatus {
-  STEPPER_INITING = 0
-, STEPPER_RUNNING = 1
-, STEPPER_FAILED = 2
-, STEPPER_COMPLETED = 3
-, STEPPER_CANCELED = 4
-};
-
-
-/**
- * This class provides public methods and properties to steps.
- * Any other code should use the wrapper (AsyncFaviconStepper) instead.
- *
- * @see AsyncFaviconStepper
- */
-class AsyncFaviconStepperInternal : public nsISupports
-{
-public:
-  NS_DECL_ISUPPORTS
-
-  /**
-   * Creates the stepper.
-   *
-   * @param aCallback
-   *        An nsIFaviconDataCallback to be called when done.
-   */
-  AsyncFaviconStepperInternal(nsIFaviconDataCallback* aCallback);
-
-  /**
-   * Proceed to next step.
-   */
-  nsresult Step();
-
-  /**
-   * Called by the steps when something goes wrong.
-   * Will unlink all steps and gently return.
-   */
-  void Failure();
-
-  /**
-   * Called by the steps when they require us to stop walking.
-   * This is not an error condition, sometimes we could want to bail out to
-   * avoid useless additional work.
-   * Will unlink all steps and gently return.
-   */
-  void Cancel(bool aNotify);
-
-  nsCOMPtr<nsIFaviconDataCallback> mCallback;
-  PRInt64 mPageId;
-  nsCOMPtr<nsIURI> mPageURI;
-  PRInt64 mIconId;
-  nsCOMPtr<nsIURI> mIconURI;
-  nsCString mData;
-  nsCString mMimeType;
-  PRTime mExpiration;
-  bool mIsRevisit;
-  PRUint16 mIconStatus; // This is a bitset, see ICON_STATUS_* defines above.
-
-private:
-  enum AsyncFaviconStepperStatus mStatus;
-  nsCOMArray<AsyncFaviconStep> mSteps;
-
-  friend class AsyncFaviconStepper;
-};
-
-
-/**
- * Walks through an ordered list of AsyncFaviconSteps.
- * Each step call backs the stepper that will proceed to the next one.
- * When all steps are complete it calls aCallback, if valid.
- *
- * This class is a wrapper around AsyncFaviconStepperInternal, where the actual
- * work is done.
- */
-class AsyncFaviconStepper : public nsISupports
-{
-public:
-  NS_DECL_ISUPPORTS
-
-  /**
-   * Creates the stepper.
-   *
-   * @param aCallback
-   *        An nsIFaviconDataCallback to call when done.
-   */
-  AsyncFaviconStepper(nsIFaviconDataCallback* aCallback);
-
-  /**
-   * Kick-off the first step.
-   */
-  nsresult Start();
-
-  /**
-   * Appends a new step to this stepper.
-   *
-   * @param aStep
-   *        An AsyncFaviconStep to append.
-   */
-  nsresult AppendStep(AsyncFaviconStep* aStep);
-
-  // Setters and getters.
-  // Some definitions are inline to try getting some love from the compiler.
-
-  void SetPageId(PRInt64 aPageId) { mStepper->mPageId = aPageId; }
-  PRInt64 GetPageId() { return mStepper->mPageId; }
-
-  void SetPageURI(nsIURI* aURI) { mStepper->mPageURI = aURI; }
-  already_AddRefed<nsIURI> GetPageURI() { return mStepper->mPageURI.forget(); }
-
-  void SetIconId(PRInt64 aIconId) { mStepper->mIconId = aIconId; }
-  PRInt64 GetIconId() { return mStepper->mIconId; }
-
-  void SetIconURI(nsIURI* aURI) { mStepper->mIconURI = aURI; }
-  already_AddRefed<nsIURI> GetIconURI() { return mStepper->mIconURI.forget(); }
-
-  nsresult SetIconData(const nsACString& aMimeType,
-                       const PRUint8* aData,
-                       PRUint32 aDataLen);
-  nsresult GetIconData(nsACString& aMimeType,
-                       const PRUint8** aData,
-                       PRUint32* aDataLen);
-
-  void SetExpiration(PRTime aExpiration) { mStepper->mExpiration = aExpiration; }
-  PRTime GetExpiration() { return mStepper->mExpiration; }
-
-private:
-  nsCOMPtr<AsyncFaviconStepperInternal> mStepper;
-};
-
-
-/**
- * Determines the real page URI that a favicon should be stored for.
- * Will ensure we can save this icon, and return the correct bookmark to
- * associate it with.
- */
-class GetEffectivePageStep : public AsyncFaviconStep
-{
-public:
-  NS_DECL_MOZISTORAGESTATEMENTCALLBACK
-
-  GetEffectivePageStep();
-  void Run();
-
-private:
-  void CheckPageAndProceed();
-
-  PRUint8 mSubStep;
-  bool mIsBookmarked;
-};
-
-
-/**
- * Fetch an existing icon and associated information from the database.
- */
-class FetchDatabaseIconStep : public AsyncFaviconStep
-{
-public:
-  NS_DECL_MOZISTORAGESTATEMENTCALLBACK
-
-  FetchDatabaseIconStep() {};
-  void Run();
-};
-
-
-/**
- * Fetch an existing icon and associated information from the database.
- * Requires mDBInsertIcon statement.
- */
-class EnsureDatabaseEntryStep : public AsyncFaviconStep
-{
-public:
-  NS_IMETHOD HandleCompletion(PRUint16 aReason);
-  NS_IMETHOD HandleError(mozIStorageError* aError);
-
-  EnsureDatabaseEntryStep() {};
-  void Run();
-};
-
 enum AsyncFaviconFetchMode {
   FETCH_NEVER = 0
-, FETCH_IF_MISSING = 1
+, FETCH_IF_MISSING
-, FETCH_ALWAYS = 2
+, FETCH_ALWAYS
 };
 
+/**
+ * Data cache for a icon entry.
+ */
+struct IconData
+{
+  IconData()
+  : id(0)
+  , expiration(0)
+  , fetchMode(FETCH_NEVER)
+  , status(ICON_STATUS_UNKNOWN)
+  {
+  }
+
+  PRInt64 id;
+  nsCString spec;
+  nsCString data;
+  nsCString mimeType;
+  PRTime expiration;
+  enum AsyncFaviconFetchMode fetchMode;
+  PRUint16 status; // This is a bitset, see ICON_STATUS_* defines above.
+};
 
 /**
- * Fetch an icon and associated information from the network.
+ * Data cache for a page entry.
- * Requires mDBGetIconInfoWithPage statement.
  */
-class FetchNetworkIconStep : public AsyncFaviconStep
+struct PageData
-                           , public nsIStreamListener
+{
-                           , public nsIInterfaceRequestor
+  PageData()
-                           , public nsIChannelEventSink
+  : id(0)
+  , canAddToHistory(true)
+  , iconId(0)
+  {
+  }
+
+  PRInt64 id;
+  nsCString spec;
+  nsCString bookmarkedSpec;
+  nsString revHost;
+  bool canAddToHistory; // False for disabled history and unsupported schemas.
+  PRInt64 iconId;
+};
+
+/**
+ * Async fetches icon from database or network, associates it with the required
+ * page and finally notifies the change.
+ */
+class AsyncFetchAndSetIconForPage : public nsRunnable
+{
+public:
+  NS_DECL_NSIRUNNABLE
+
+  /**
+   * Creates the event and dispatches it to the async thread.
+   *
+   * @param aFaviconURI
+   *        URI of the icon to be fetched and associated.
+   * @param aPageURI
+   *        URI of the page to which associate the icon.
+   * @param aFetchMode
+   *        Specifies whether a icon should be fetched from network if not found
+   *        in the database.
+   * @param aDBConn
+   *        Database connection to use.
+   * @param aCallback
+   *        Function to be called when the fetch and associate process finishes.
+   */
+  static nsresult start(nsIURI* aFaviconURI,
+                        nsIURI* aPageURI,
+                        enum AsyncFaviconFetchMode aFetchMode,
+                        nsCOMPtr<mozIStorageConnection>& aDBConn,
+                        nsIFaviconDataCallback* aCallback);
+
+  /**
+   * Constructor.
+   *
+   * @param aIcon
+   *        Icon to be fetched and associated.
+   * @param aPage
+   *        Page to which associate the icon.
+   * @param aDBConn
+   *        Database connection to use.
+   * @param aCallback
+   *        Function to be called when the fetch and associate process finishes.
+   */
+  AsyncFetchAndSetIconForPage(IconData& aIcon,
+                              PageData& aPage,
+                              nsCOMPtr<mozIStorageConnection>& aDBConn,
+                              nsRefPtr<nsFaviconService>& aFaviconSvc,
+                              nsCOMPtr<nsIFaviconDataCallback>& aCallback);
+
+  virtual ~AsyncFetchAndSetIconForPage();
+
+protected:
+  IconData mIcon;
+  PageData mPage;
+  nsCOMPtr<mozIStorageConnection>& mDBConn;
+  // Strong reference since we don't want it to disappear out from under us.
+  nsRefPtr<nsFaviconService> mFaviconSvc;
+  // Strong reference since we are responsible for its existence.
+  nsCOMPtr<nsIFaviconDataCallback> mCallback;
+
+};
+
+/**
+ * If needed will asynchronously fetch the icon from the network.  It will
+ * finally dispatch an event to the async thread to associate the icon with
+ * the required page.
+ */
+class AsyncFetchAndSetIconFromNetwork : public nsRunnable
+                                      , public nsIStreamListener
+                                      , public nsIInterfaceRequestor
+                                      , public nsIChannelEventSink
 {
 public:
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(FetchNetworkIconStep, AsyncFaviconStep)
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIREQUESTOBSERVER
+  NS_DECL_NSIRUNNABLE
+  NS_DECL_ISUPPORTS_INHERITED
 
-  FetchNetworkIconStep(enum AsyncFaviconFetchMode aFetchMode);
+  /**
-  void Run();
+   * Constructor.
+   *
+   * @param aIcon
+   *        Icon to be fetched and associated.
+   * @param aPage
+   *        Page to which associate the icon.
+   * @param aDBConn
+   *        Database connection to use.
+   * @param aCallback
+   *        Function to be called when the fetch and associate process finishes.
+   */
+  AsyncFetchAndSetIconFromNetwork(IconData& aIcon,
+                                  PageData& aPage,
+                                  nsCOMPtr<mozIStorageConnection>& aDBConn,
+                                  nsRefPtr<nsFaviconService>& aFaviconSvc,
+                                  nsCOMPtr<nsIFaviconDataCallback>& aCallback);
 
-private:
+  virtual ~AsyncFetchAndSetIconFromNetwork();
-  enum AsyncFaviconFetchMode mFetchMode;
+
+protected:
+  IconData mIcon;
+  PageData mPage;
+  nsCOMPtr<mozIStorageConnection>& mDBConn;
+  // Strong reference since we don't want it to disappear out from under us.
+  nsRefPtr<nsFaviconService> mFaviconSvc;
+  // Strong reference since we are responsible for its existence.
+  nsCOMPtr<nsIFaviconDataCallback> mCallback;
   nsCOMPtr<nsIChannel> mChannel;
-  nsCString mData;
 };
 
-
 /**
- * Saves icon data in the database if it has changed.
+ * Associates the icon to the required page, finally dispatches an event to the
+ * main thread to notify the change to observers.
  */
-class SetFaviconDataStep : public AsyncFaviconStep
+class AsyncAssociateIconToPage : public nsRunnable
 {
 public:
-  NS_IMETHOD HandleCompletion(PRUint16 aReason);
+  NS_DECL_NSIRUNNABLE
-  NS_IMETHOD HandleError(mozIStorageError* aError);
 
-  SetFaviconDataStep() {};
+  /**
-  void Run();
+   * Constructor.
+   *
+   * @param aIcon
+   *        Icon to be associated.
+   * @param aPage
+   *        Page to which associate the icon.
+   * @param aDBConn
+   *        Database connection to use.
+   * @param aCallback
+   *        Function to be called when the fetch and associate process finishes.
+   */
+  AsyncAssociateIconToPage(IconData& aIcon,
+                           PageData& aPage,
+                           nsCOMPtr<mozIStorageConnection>& aDBConn,
+                           nsRefPtr<nsFaviconService>& aFaviconSvc,
+                           nsCOMPtr<nsIFaviconDataCallback>& aCallback);
+
+  virtual ~AsyncAssociateIconToPage();
+
+protected:
+  IconData mIcon;
+  PageData mPage;
+  nsCOMPtr<mozIStorageConnection>& mDBConn;
+  // Strong reference since we don't want it to disappear out from under us.
+  nsRefPtr<nsFaviconService> mFaviconSvc;
+  // Strong reference since we are responsible for its existence.
+  nsCOMPtr<nsIFaviconDataCallback> mCallback;
 };
 
-
 /**
- * Associate icon with page.
+ * Notifies the icon change to favicon observers.
  */
-class AssociateIconWithPageStep : public AsyncFaviconStep
+class NotifyIconObservers : public nsRunnable
 {
 public:
-  NS_IMETHOD HandleCompletion(PRUint16 aReason);
+  NS_DECL_NSIRUNNABLE
-  NS_IMETHOD HandleError(mozIStorageError* aError);
 
-  AssociateIconWithPageStep() {};
+  NotifyIconObservers(IconData& aIcon,
-  void Run();
+                      PageData& aPage,
-};
+                      nsCOMPtr<mozIStorageConnection>& aDBConn,
+                      nsRefPtr<nsFaviconService>& aFaviconSvc,
+                      nsCOMPtr<nsIFaviconDataCallback>& aCallback);
+  virtual ~NotifyIconObservers();
 
-
+protected:
-/**
+  IconData mIcon;
- * Notify favicon changes.
+  PageData mPage;
- */
+  nsCOMPtr<mozIStorageConnection>& mDBConn;
-class NotifyStep : public AsyncFaviconStep
+  // Strong reference since we don't want it to disappear out from under us.
-{
+  nsRefPtr<nsFaviconService> mFaviconSvc;
-public:
+  // Strong reference since we are responsible for its existence.
-  NotifyStep() {};
+  nsCOMPtr<nsIFaviconDataCallback> mCallback;
-  void Run();
 };
 
 } // namespace places

toolkit/components/places/src/nsFaviconService.cpp

@@ -165,18 +165,6 @@ nsFaviconService::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
     "SELECT id, length(data), expiration FROM moz_favicons "
     "WHERE url = :icon_url"));
 
-  // If the page does not exist url = NULL will return NULL instead of 0,
-  // since (1 = NULL) is NULL.  Thus the need for the IFNULL.
-  RETURN_IF_STMT(mDBGetIconInfoWithPage, NS_LITERAL_CSTRING(
-    "SELECT id, length(data), expiration, data, mime_type, "
-    "IFNULL(url = (SELECT f.url "
-                  "FROM moz_places h "
-                  "JOIN moz_favicons f ON h.favicon_id = f.id "
-                  "WHERE h.url = :page_url "
-                  "LIMIT 1), "
-           "0) "
-    "FROM moz_favicons WHERE url = :icon_url"));
-
   RETURN_IF_STMT(mDBGetURL, NS_LITERAL_CSTRING(
     "SELECT f.id, f.url, length(f.data), f.expiration "
     "FROM moz_places h "
@@ -199,11 +187,6 @@ nsFaviconService::GetStatement(const nsCOMPtr<mozIStorageStatement>& aStmt)
   RETURN_IF_STMT(mDBSetPageFavicon, NS_LITERAL_CSTRING(
     "UPDATE moz_places SET favicon_id = :icon_id WHERE id = :page_id"));
 
-  RETURN_IF_STMT(mDBAssociateFaviconURIToPageURI, NS_LITERAL_CSTRING(
-    "UPDATE moz_places "
-    "SET favicon_id = (SELECT id FROM moz_favicons WHERE url = :icon_url) "
-    "WHERE url = :page_url"));
-
   RETURN_IF_STMT(mDBRemoveOnDiskReferences, NS_LITERAL_CSTRING(
     "UPDATE moz_places "
     "SET favicon_id = NULL "
@@ -391,54 +374,6 @@ nsFaviconService::SetFaviconUrlForPageInternal(nsIURI* aPageURI,
 }
 
 
-// nsFaviconService::UpdateBookmarkRedirectFavicon
-//
-//    It is not uncommon to have a bookmark (usually manually entered or
-//    modified) that redirects to some other page. For example, "mozilla.org"
-//    redirects to "www.mozilla.org". We want that bookmark's favicon to get
-//    updated. So, we see if this URI has a bookmark redirect and set the
-//    favicon there as well.
-//
-//    This should be called only when we know there is data for the favicon
-//    already loaded. We will always send out notifications for the bookmarked
-//    page.
-
-nsresult
-nsFaviconService::UpdateBookmarkRedirectFavicon(nsIURI* aPageURI,
-                                                nsIURI* aFaviconURI)
-{
-  NS_ENSURE_ARG_POINTER(aPageURI);
-  NS_ENSURE_ARG_POINTER(aFaviconURI);
-
-  nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
-  NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY);
-
-  nsCOMPtr<nsIURI> bookmarkURI;
-  nsresult rv = bookmarks->GetBookmarkedURIFor(aPageURI,
-                                               getter_AddRefs(bookmarkURI));
-  NS_ENSURE_SUCCESS(rv, rv);
-  if (! bookmarkURI)
-    return NS_OK; // no bookmark redirect
-
-  PRBool sameAsBookmark;
-  if (NS_SUCCEEDED(bookmarkURI->Equals(aPageURI, &sameAsBookmark)) &&
-      sameAsBookmark)
-    return NS_OK; // bookmarked directly, not through a redirect
-
-  PRBool hasData = PR_FALSE;
-  rv = SetFaviconUrlForPageInternal(bookmarkURI, aFaviconURI, &hasData);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  if (hasData) {
-    // send notifications
-    SendFaviconNotifications(bookmarkURI, aFaviconURI);
-  } else {
-    NS_WARNING("Calling UpdateBookmarkRedirectFavicon when you don't have data for the favicon yet.");
-  }
-  return NS_OK;
-}
-
-
 // nsFaviconService::SendFaviconNotifications
 //
 //    Call to send out favicon changed notifications. Should only be called
@@ -470,25 +405,7 @@ nsFaviconService::SetAndLoadFaviconForPage(nsIURI* aPageURI,
   if (mFaviconsExpirationRunning)
     return NS_OK;
 
-  nsresult rv = DoSetAndLoadFaviconForPage(aPageURI, aFaviconURI, aForceReload,
+  // If a favicon is in the failed cache, only load it during a forced reload.
-                                           aCallback);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
-
-nsresult
-nsFaviconService::DoSetAndLoadFaviconForPage(nsIURI* aPageURI,
-                                             nsIURI* aFaviconURI,
-                                             PRBool aForceReload,
-                                             nsIFaviconDataCallback* aCallback)
-{
-  if (mFaviconsExpirationRunning)
-    return NS_OK;
-
-  // If a favicon is in the failed cache, we'll only load it if we are forcing
-  // a reload.
   PRBool previouslyFailed;
   nsresult rv = IsFailedFavicon(aFaviconURI, &previouslyFailed);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -499,25 +416,12 @@ nsFaviconService::DoSetAndLoadFaviconForPage(nsIURI* aPageURI,
       return NS_OK;
   }
 
-  nsCOMPtr<AsyncFaviconStepper> stepper = new AsyncFaviconStepper(aCallback);
+  // Check if the icon already exists and fetch it from the network, if needed.
-  stepper->SetPageURI(aPageURI);
+  // Finally associate the icon to the requested page if not yet associated.
-  stepper->SetIconURI(aFaviconURI);
+  rv = AsyncFetchAndSetIconForPage::start(
-  rv = stepper->AppendStep(new GetEffectivePageStep());
+    aFaviconURI, aPageURI, aForceReload ? FETCH_ALWAYS : FETCH_IF_MISSING,
-  NS_ENSURE_SUCCESS(rv, rv);
+    mDBConn, aCallback
-  rv = stepper->AppendStep(new FetchDatabaseIconStep());
+  );
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stepper->AppendStep(new EnsureDatabaseEntryStep());
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stepper->AppendStep(new FetchNetworkIconStep(
-    aForceReload ? FETCH_ALWAYS : FETCH_IF_MISSING));
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stepper->AppendStep(new SetFaviconDataStep());
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stepper->AppendStep(new AssociateIconWithPageStep());
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stepper->AppendStep(new NotifyStep());
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = stepper->Start();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // DB will be updated and observers notified when data has finished loading.
@@ -1023,13 +927,11 @@ nsFaviconService::FinalizeStatements() {
     mDBGetURL,
     mDBGetData,
     mDBGetIconInfo,
-    mDBGetIconInfoWithPage,
     mDBInsertIcon,
     mDBUpdateIcon,
     mDBSetPageFavicon,
     mDBRemoveOnDiskReferences,
     mDBRemoveAllFavicons,
-    mDBAssociateFaviconURIToPageURI,
   };
 
   for (PRUint32 i = 0; i < NS_ARRAY_LENGTH(stmts); i++) {

toolkit/components/places/src/nsFaviconService.h

@@ -36,6 +36,9 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+#ifndef nsFaviconService_h_
+#define nsFaviconService_h_
+
 #include "nsCOMPtr.h"
 #include "nsDataHashtable.h"
 #include "nsIFaviconService.h"
@@ -51,18 +54,6 @@
 // This still allows us to accept a favicon even if we cannot optimize it.
 #define MAX_FAVICON_SIZE 10240
 
-namespace mozilla {
-namespace places {
-
-  enum FaviconStatementId {
-    DB_GET_ICON_INFO_WITH_PAGE = 0
-  , DB_INSERT_ICON = 1
-  , DB_ASSOCIATE_ICONURI_TO_PAGEURI = 2
-  };
-
-} // namespace places
-} // namespace mozilla
-
 // Most icons will be smaller than this rough estimate of the size of an
 // uncompressed 16x16 RGBA image of the same dimensions.
 #define MAX_ICON_FILESIZE(s) ((PRUint32) s*s*4)
@@ -149,24 +140,6 @@ public:
    */
   nsresult FinalizeStatements();
 
-  mozIStorageStatement* GetStatementById(
-    enum mozilla::places::FaviconStatementId aStatementId
-  )
-  {
-    using namespace mozilla::places;
-    switch(aStatementId) {
-      case DB_GET_ICON_INFO_WITH_PAGE:
-        return GetStatement(mDBGetIconInfoWithPage);
-      case DB_INSERT_ICON:
-        return GetStatement(mDBInsertIcon);
-      case DB_ASSOCIATE_ICONURI_TO_PAGEURI:
-        return GetStatement(mDBAssociateFaviconURIToPageURI);
-    }
-    return nsnull;
-  }
-
-  nsresult UpdateBookmarkRedirectFavicon(nsIURI* aPage, nsIURI* aFavicon);
-
   void SendFaviconNotifications(nsIURI* aPage, nsIURI* aFaviconURI);
 
   NS_DECL_ISUPPORTS
@@ -184,11 +157,9 @@ private:
   nsCOMPtr<mozIStorageStatement> mDBGetURL; // returns URL, data len given page
   nsCOMPtr<mozIStorageStatement> mDBGetData; // returns actual data given URL
   nsCOMPtr<mozIStorageStatement> mDBGetIconInfo;
-  nsCOMPtr<mozIStorageStatement> mDBGetIconInfoWithPage;
   nsCOMPtr<mozIStorageStatement> mDBInsertIcon;
   nsCOMPtr<mozIStorageStatement> mDBUpdateIcon;
   nsCOMPtr<mozIStorageStatement> mDBSetPageFavicon;
-  nsCOMPtr<mozIStorageStatement> mDBAssociateFaviconURIToPageURI;
   nsCOMPtr<mozIStorageStatement> mDBRemoveOnDiskReferences;
   nsCOMPtr<mozIStorageStatement> mDBRemoveAllFavicons;
 
@@ -232,3 +203,5 @@ private:
 };
 
 #define FAVICON_ANNOTATION_NAME "favicon"
+
+#endif // nsFaviconService_h_