Bug 556400 - Implement asyncable VisitURI. r=sdwilsh, sr=bz

docshell/base/IHistory.h

@@ -51,7 +51,7 @@ namespace mozilla {
     }
 
 #define IHISTORY_IID \
-  {0xaf27265d, 0x5672, 0x4d23, {0xa0, 0x75, 0x34, 0x8e, 0xb9, 0x73, 0x5a, 0x9a}}
+  {0x6f736049, 0x6370, 0x4376, {0xb7, 0x17, 0xfa, 0xfc, 0x0b, 0x4f, 0xd0, 0xf1}}
 
 class IHistory : public nsISupports
 {
@@ -96,6 +96,38 @@ public:
      */
     NS_IMETHOD UnregisterVisitedCallback(nsIURI *aURI, dom::Link *aLink) = 0;
 
+    enum VisitFlags {
+        /**
+         * Indicates whether the URI was loaded in a top-level window.
+         */
+        TOP_LEVEL = 1 << 0,
+        /**
+         * Indicates whether the URI was loaded as part of a permanent redirect.
+         */
+        REDIRECT_PERMANENT = 1 << 1,
+        /**
+         * Indicates whether the URI was loaded as part of a temporary redirect.
+         */
+        REDIRECT_TEMPORARY = 1 << 2
+    };
+
+    /**
+     * Adds a history visit for the URI.
+     *
+     * @pre aURI must not be null.
+     *
+     * @param aURI
+     *        The URI of the page being visited.
+     * @param aLastVisitedURI
+     *        The URI of the last visit in the chain.
+     * @param aFlags
+     *        The VisitFlags describing this visit.
+     */
+    NS_IMETHOD VisitURI(
+        nsIURI *aURI,
+        nsIURI *aLastVisitedURI,
+        PRUint32 aFlags
+    ) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(IHistory, IHISTORY_IID)
@@ -104,7 +136,10 @@ NS_DEFINE_STATIC_IID_ACCESSOR(IHistory, IHISTORY_IID)
     NS_IMETHOD RegisterVisitedCallback(nsIURI *aURI, \
                                        mozilla::dom::Link *aContent); \
     NS_IMETHOD UnregisterVisitedCallback(nsIURI *aURI, \
-                                         mozilla::dom::Link *aContent);
+                                         mozilla::dom::Link *aContent); \
+    NS_IMETHOD VisitURI(nsIURI *aURI, \
+                        nsIURI *aLastVisitedURI, \
+                        PRUint32 aFlags);
 
 } // namespace mozilla
 

docshell/base/nsDocShell.cpp

@@ -112,6 +112,8 @@
 #include "nsIOfflineCacheUpdate.h"
 #include "nsCPrefetchService.h"
 #include "nsJSON.h"
+#include "IHistory.h"
+#include "mozilla/Services.h"
 
 // we want to explore making the document own the load group
 // so we can associate the document URI with the load group.
@@ -5662,32 +5664,53 @@ nsDocShell::OnRedirectStateChange(nsIChannel* aOldChannel,
     if (!(aStateFlags & STATE_IS_DOCUMENT))
         return; // not a toplevel document
 
-    nsCOMPtr<nsIGlobalHistory3> history3(do_QueryInterface(mGlobalHistory));
+    nsCOMPtr<nsIURI> oldURI, newURI;
-    nsresult result = NS_ERROR_NOT_IMPLEMENTED;
+    aOldChannel->GetURI(getter_AddRefs(oldURI));
-    if (history3) {
+    aNewChannel->GetURI(getter_AddRefs(newURI));
-        // notify global history of this redirect
+    if (!oldURI || !newURI) {
-        result = history3->AddDocumentRedirect(aOldChannel, aNewChannel,
+        return;
-                                               aRedirectFlags, !IsFrame());
     }
 
-    if (result == NS_ERROR_NOT_IMPLEMENTED) {
+    // Below a URI visit is saved (see AddURIVisit method doc).
-        // when there is no GlobalHistory3, or it doesn't implement
+    // The visit chain looks something like:
-        // AddToplevelRedirect, we fall back to GlobalHistory2.  Just notify
+    //   ...
-        // that the redirecting page was a rePdirect so it will be link colored
+    //   Site N - 1
-        // but not visible.
+    //                =>  Site N
-        nsCOMPtr<nsIURI> oldURI;
+    //   (redirect to =>) Site N + 1 (we are here!)
-        aOldChannel->GetURI(getter_AddRefs(oldURI));
+
-        if (! oldURI)
+    // Get N - 1 and transition type
-            return; // nothing to tell anybody about
+    nsCOMPtr<nsIURI> previousURI;
-        AddToGlobalHistory(oldURI, PR_TRUE, aOldChannel);
+    PRUint32 previousFlags = 0;
+    ExtractLastVisit(aOldChannel, getter_AddRefs(previousURI), &previousFlags);
+
+    if (aRedirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL ||
+        ChannelIsPost(aOldChannel)) {
+        // 1. Internal redirects are ignored because they are specific to the
+        //    channel implementation.
+        // 2. POSTs are not saved by global history.
+        //
+        // Regardless, we need to propagate the previous visit to the new
+        // channel.
+        SaveLastVisit(aNewChannel, previousURI, previousFlags);
+    }
+    else {
+        nsCOMPtr<nsIURI> referrer;
+        // Treat referrer as null if there is an error getting it.
+        (void)NS_GetReferrerFromChannel(aOldChannel,
+                                        getter_AddRefs(referrer));
+
+        // Add visit N -1 => N
+        AddURIVisit(oldURI, referrer, previousURI, previousFlags);
+
+        // Since N + 1 could be the final destination, we will not save N => N + 1
+        // here.  OnNewURI will do that, so we will cache it.
+        SaveLastVisit(aNewChannel, oldURI, aRedirectFlags);
     }
 
     // check if the new load should go through the application cache.
     nsCOMPtr<nsIApplicationCacheChannel> appCacheChannel =
         do_QueryInterface(aNewChannel);
     if (appCacheChannel) {
-        nsCOMPtr<nsIURI> newURI;
-        aNewChannel->GetURI(getter_AddRefs(newURI));
         appCacheChannel->SetChooseApplicationCache(ShouldCheckAppCache(newURI));
     }
 
@@ -8927,7 +8950,7 @@ nsDocShell::OnNewURI(nsIURI * aURI, nsIChannel * aChannel, nsISupports* aOwner,
     nsCOMPtr<nsIInputStream> inputStream;
     if (aChannel) {
         nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aChannel));
-        
+
         // Check if the HTTPChannel is hiding under a multiPartChannel
         if (!httpChannel)  {
             GetHttpChannel(aChannel, getter_AddRefs(httpChannel));
@@ -9017,8 +9040,8 @@ nsDocShell::OnNewURI(nsIURI * aURI, nsIChannel * aChannel, nsISupports* aOwner,
         
         nsCOMPtr<nsICachingChannel> cacheChannel(do_QueryInterface(aChannel));
         nsCOMPtr<nsISupports>  cacheKey;
-        // Get the Cache Key  and store it in SH.         
+        // Get the Cache Key and store it in SH.
-        if (cacheChannel) 
+        if (cacheChannel)
             cacheChannel->GetCacheKey(getter_AddRefs(cacheKey));
         // If we already have a loading history entry, store the new cache key
         // in it.  Otherwise, since we're doing a reload and won't be updating
@@ -9040,10 +9063,22 @@ nsDocShell::OnNewURI(nsIURI * aURI, nsIChannel * aChannel, nsISupports* aOwner,
                                        getter_AddRefs(mLSHE));
         }
 
-        // Update Global history
         if (aAddToGlobalHistory) {
-            // Get the referrer uri from the channel
+            // If this is a POST request, we do not want to include this in global
-            AddToGlobalHistory(aURI, PR_FALSE, aChannel);
+            // history.
+            if (!ChannelIsPost(aChannel)) {
+                nsCOMPtr<nsIURI> previousURI;
+                PRUint32 previousFlags = 0;
+                ExtractLastVisit(aChannel, getter_AddRefs(previousURI),
+                                 &previousFlags);
+
+                nsCOMPtr<nsIURI> referrer;
+                // Treat referrer as null if there is an error getting it.
+                (void)NS_GetReferrerFromChannel(aChannel,
+                                                getter_AddRefs(referrer));
+
+                AddURIVisit(aURI, referrer, previousURI, previousFlags);
+            }
         }
     }
 
@@ -9362,7 +9397,7 @@ nsDocShell::AddState(nsIVariant *aData, const nsAString& aTitle,
         SetCurrentURI(newURI, nsnull, PR_TRUE);
         document->SetDocumentURI(newURI);
 
-        AddToGlobalHistory(newURI, PR_FALSE, oldURI);
+        AddURIVisit(newURI, oldURI, oldURI, 0);
     }
     else {
         FireOnLocationChange(this, nsnull, mCurrentURI);
@@ -10063,53 +10098,109 @@ NS_IMETHODIMP nsDocShell::MakeEditable(PRBool inWaitForUriLoad)
   return mEditorData->MakeEditable(inWaitForUriLoad);
 }
 
-nsresult
+bool
-nsDocShell::AddToGlobalHistory(nsIURI * aURI, PRBool aRedirect,
+nsDocShell::ChannelIsPost(nsIChannel* aChannel)
-                               nsIChannel * aChannel)
 {
-    // If this is a POST request, we do not want to include this in global
+    nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aChannel));
-    // history, so return early.
+    if (!httpChannel) {
-    nsCOMPtr<nsIHttpChannel> hchan(do_QueryInterface(aChannel));
+        return false;
-    if (hchan) {
-        nsCAutoString type;
-        nsresult rv = hchan->GetRequestMethod(type);
-        if (NS_SUCCEEDED(rv) && type.EqualsLiteral("POST"))
-            return NS_OK;
     }
 
-    nsCOMPtr<nsIURI> referrer;
+    nsCAutoString method;
-    if (aChannel)
+    httpChannel->GetRequestMethod(method);
-        NS_GetReferrerFromChannel(aChannel, getter_AddRefs(referrer));
+    return method.Equals("POST");
-
-    return AddToGlobalHistory(aURI, aRedirect, referrer);
 }
 
-nsresult
+void
-nsDocShell::AddToGlobalHistory(nsIURI * aURI, PRBool aRedirect,
+nsDocShell::ExtractLastVisit(nsIChannel* aChannel,
-                               nsIURI * aReferrer)
+                             nsIURI** aURI,
+                             PRUint32* aChannelRedirectFlags)
 {
-    if (mItemType != typeContent || !mGlobalHistory)
+    nsCOMPtr<nsIPropertyBag2> props(do_QueryInterface(aChannel));
-        return NS_OK;
+    if (!props) {
-
+        return;
-    PRBool visited;
-    nsresult rv = mGlobalHistory->IsVisited(aURI, &visited);
-    if (NS_FAILED(rv))
-        return rv;
-
-    rv = mGlobalHistory->AddURI(aURI, aRedirect, !IsFrame(), aReferrer);
-    if (NS_FAILED(rv))
-        return rv;
-
-    if (!visited) {
-        nsCOMPtr<nsIObserverService> obsService =
-            mozilla::services::GetObserverService();
-        if (obsService) {
-            obsService->NotifyObservers(aURI, NS_LINK_VISITED_EVENT_TOPIC, nsnull);
-        }
     }
 
-    return NS_OK;
+    nsresult rv = props->GetPropertyAsInterface(
+        NS_LITERAL_STRING("docshell.previousURI"),
+        NS_GET_IID(nsIURI),
+        reinterpret_cast<void**>(aURI)
+    );
 
+    if (NS_FAILED(rv)) {
+        // There is no last visit for this channel, so this must be the first
+        // link.  Link the visit to the referrer of this request, if any.
+        // Treat referrer as null if there is an error getting it.
+        (void)NS_GetReferrerFromChannel(aChannel, aURI);
+    }
+    else {
+      rv = props->GetPropertyAsUint32(
+          NS_LITERAL_STRING("docshell.previousFlags"),
+          aChannelRedirectFlags
+      );
+
+      NS_WARN_IF_FALSE(
+          NS_FAILED(rv),
+          "Could not fetch previous flags, URI will be treated like referrer"
+      );
+    }
+}
+
+void
+nsDocShell::SaveLastVisit(nsIChannel* aChannel,
+                          nsIURI* aURI,
+                          PRUint32 aChannelRedirectFlags)
+{
+    nsCOMPtr<nsIWritablePropertyBag2> props(do_QueryInterface(aChannel));
+    if (!props || !aURI) {
+        return;
+    }
+
+    props->SetPropertyAsInterface(NS_LITERAL_STRING("docshell.previousURI"),
+                                  aURI);
+    props->SetPropertyAsUint32(NS_LITERAL_STRING("docshell.previousFlags"),
+                               aChannelRedirectFlags);
+}
+
+void
+nsDocShell::AddURIVisit(nsIURI* aURI,
+                        nsIURI* aReferrerURI,
+                        nsIURI* aPreviousURI,
+                        PRUint32 aChannelRedirectFlags)
+{
+    NS_ASSERTION(aURI, "Visited URI is null!");
+
+    // Only content-type docshells save URI visits.
+    if (mItemType != typeContent) {
+        return;
+    }
+
+    nsCOMPtr<IHistory> history = services::GetHistoryService();
+
+    if (history) {
+        PRUint32 visitURIFlags = 0;
+
+        if (!IsFrame()) {
+            visitURIFlags |= IHistory::TOP_LEVEL;
+        }
+
+        if (aChannelRedirectFlags & nsIChannelEventSink::REDIRECT_TEMPORARY) {
+            visitURIFlags |= IHistory::REDIRECT_TEMPORARY;
+        }
+        else if (aChannelRedirectFlags &
+                 nsIChannelEventSink::REDIRECT_PERMANENT) {
+            visitURIFlags |= IHistory::REDIRECT_PERMANENT;
+        }
+
+        (void)history->VisitURI(aURI, aPreviousURI, visitURIFlags);
+    }
+    else if (mGlobalHistory) {
+        // Falls back to sync global history interface.
+        (void)mGlobalHistory->AddURI(aURI,
+                                     !!aChannelRedirectFlags,
+                                     !IsFrame(),
+                                     aReferrerURI);
+    }
 }
 
 //*****************************************************************************

docshell/base/nsDocShell.h

@@ -433,12 +433,77 @@ protected:
                                        PRUint32 aRedirectFlags,
                                        PRUint32 aStateFlags);
 
-    // Global History
+    /**
+     * Helper function that determines if channel is an HTTP POST.
+     *
+     * @param aChannel
+     *        The channel to test
+     *
+     * @return True iff channel is an HTTP post.
+     */
+    bool ChannelIsPost(nsIChannel* aChannel);
 
-    nsresult AddToGlobalHistory(nsIURI * aURI, PRBool aRedirect,
+    /**
-                                nsIChannel * aChannel);
+     * Helper function that finds the last URI and its transition flags for a
-    nsresult AddToGlobalHistory(nsIURI * aURI, PRBool aRedirect,
+     * channel.
-                                nsIURI * aReferrer);
+     *
+     * This method first checks the channel's property bag to see if previous
+     * info has been saved.  If not, it gives back the referrer of the channel.
+     *
+     * @param aChannel
+     *        The channel we are transitioning to
+     * @param aURI
+     *        Output parameter with the previous URI, not addref'd
+     * @param aChannelRedirectFlags
+     *        If a redirect, output parameter with the previous redirect flags
+     *        from nsIChannelEventSink
+     */
+    void ExtractLastVisit(nsIChannel* aChannel,
+                          nsIURI** aURI,
+                          PRUint32* aChannelRedirectFlags);
+
+    /**
+     * Helper function that caches a URI and a transition for saving later.
+     *
+     * @param aChannel
+     *        Channel that will have these properties saved
+     * @param aURI
+     *        The URI to save for later
+     * @param aChannelRedirectFlags
+     *        The nsIChannelEventSink redirect flags to save for later
+     */
+    void SaveLastVisit(nsIChannel* aChannel,
+                       nsIURI* aURI,
+                       PRUint32 aChannelRedirectFlags);
+
+    /**
+     * Helper function for adding a URI visit using IHistory.  If IHistory is
+     * not available, the method tries nsIGlobalHistory2.
+     *
+     * The IHistory API maintains chains of visits, tracking both HTTP referrers
+     * and redirects for a user session. VisitURI requires the current URI and
+     * the previous URI in the chain.
+     *
+     * Visits can be saved either during a redirect or when the request has
+     * reached its final destination.  The previous URI in the visit may be
+     * from another redirect or it may be the referrer.
+     *
+     * @pre aURI is not null.
+     *
+     * @param aURI
+     *        The URI that was just visited
+     * @param aReferrerURI
+     *        The referrer URI of this request
+     * @param aPreviousURI
+     *        The previous URI of this visit (may be the same as aReferrerURI)
+     * @param aChannelRedirectFlags
+     *        For redirects, the redirect flags from nsIChannelEventSink
+     *        (0 otherwise)
+     */
+    void AddURIVisit(nsIURI* aURI,
+                     nsIURI* aReferrerURI,
+                     nsIURI* aPreviousURI,
+                     PRUint32 aChannelRedirectFlags);
 
     // Helper Routines
     nsresult   ConfirmRepost(PRBool * aRepost);
@@ -700,6 +765,9 @@ protected:
 
     PRInt32                    mMarginWidth;
     PRInt32                    mMarginHeight;
+
+    // This can either be a content docshell or a chrome docshell.  After
+    // Create() is called, the type is not expected to change.
     PRInt32                    mItemType;
 
     // Index into the SHTransaction list, indicating the previous and current

toolkit/components/places/src/Helpers.cpp

@@ -60,7 +60,7 @@ AsyncStatementCallback::HandleError(mozIStorageError *aError)
 
   nsCAutoString warnMsg;
   warnMsg.Append("An error occurred while executing an async statement: ");
-  warnMsg.Append(result);
+  warnMsg.AppendInt(result);
   warnMsg.Append(" ");
   warnMsg.Append(message);
   NS_WARNING(warnMsg.get());
@@ -180,6 +180,33 @@ URIBinder::Bind(mozIStorageBindingParams* aParams,
 
 #undef URI_TO_URLCSTRING
 
+nsresult
+GetReversedHostname(nsIURI* aURI, nsString& aRevHost)
+{
+  nsCAutoString forward8;
+  nsresult rv = aURI->GetHost(forward8);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // can't do reversing in UTF8, better use 16-bit chars
+  GetReversedHostname(NS_ConvertUTF8toUTF16(forward8), aRevHost);
+  return NS_OK;
+}
+
+void
+GetReversedHostname(const nsString& aForward, nsString& aRevHost)
+{
+  ReverseString(aForward, aRevHost);
+  aRevHost.Append(PRUnichar('.'));
+}
+
+void
+ReverseString(const nsString& aInput, nsString& aReversed)
+{
+  aReversed.Truncate(0);
+  for (PRInt32 i = aInput.Length() - 1; i >= 0; i--) {
+    aReversed.Append(aInput[i]);
+  }
+}
 
 } // namespace places
 } // namespace mozilla

toolkit/components/places/src/Helpers.h

@@ -136,6 +136,44 @@ public:
                        const nsACString& aURLString);
 };
 
+/**
+ * This extracts the hostname from the URI and reverses it in the
+ * form that we use (always ending with a "."). So
+ * "http://microsoft.com/" becomes "moc.tfosorcim."
+ * 
+ * The idea behind this is that we can create an index over the items in
+ * the reversed host name column, and then query for as much or as little
+ * of the host name as we feel like.
+ * 
+ * For example, the query "host >= 'gro.allizom.' AND host < 'gro.allizom/'
+ * Matches all host names ending in '.mozilla.org', including
+ * 'developer.mozilla.org' and just 'mozilla.org' (since we define all
+ * reversed host names to end in a period, even 'mozilla.org' matches).
+ * The important thing is that this operation uses the index. Any substring
+ * calls in a select statement (even if it's for the beginning of a string)
+ * will bypass any indices and will be slow).
+ *
+ * @param aURI
+ *        URI that contains spec to reverse
+ * @param aRevHost
+ *        Out parameter
+ */
+nsresult GetReversedHostname(nsIURI* aURI, nsString& aRevHost);
+
+/**
+ * Similar method to GetReversedHostName but for strings
+ */
+void GetReversedHostname(const nsString& aForward, nsString& aRevHost);
+
+/**
+ * Reverses a string.
+ *
+ * @param aInput
+ *        The string to be reversed
+ * @param aReversed
+ *        Ouput parameter will contain the reversed string
+ */
+void ReverseString(const nsString& aInput, nsString& aReversed);
 
 } // namespace places
 } // namespace mozilla

toolkit/components/places/src/History.cpp

@@ -39,6 +39,7 @@
 
 #include "History.h"
 #include "nsNavHistory.h"
+#include "nsNavBookmarks.h"
 #include "Helpers.h"
 
 #include "mozilla/storage.h"
@@ -58,6 +59,87 @@ namespace places {
 #define URI_VISITED "visited"
 #define URI_NOT_VISITED "not visited"
 #define URI_VISITED_RESOLUTION_TOPIC "visited-status-resolution"
+// Observer event fired after a visit has been registered in the DB.
+#define URI_VISIT_SAVED "uri-visit-saved"
+
+////////////////////////////////////////////////////////////////////////////////
+//// Step
+
+class Step : public AsyncStatementCallback
+{
+public:
+  /**
+   * Executes statement asynchronously using this as a callback.
+   * 
+   * @param aStmt
+   *        Statement to execute asynchronously
+   */
+  NS_IMETHOD ExecuteAsync(mozIStorageStatement* aStmt);
+
+  /**
+   * Called once after query is completed.  If your query has more than one
+   * result set to process, you will want to override HandleResult to process
+   * each one.
+   *
+   * @param aResultSet
+   *        Results from ExecuteAsync
+   *        Unlike HandleResult, this *can be NULL* if there were no results.
+   */
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet);
+
+  /**
+   * By default, stores the last result set received in mResultSet.
+   * For queries with only one result set, you don't need to override.
+   *
+   * @param aResultSet
+   *        Results from ExecuteAsync
+   */
+  NS_IMETHOD HandleResult(mozIStorageResultSet* aResultSet);
+
+  /**
+   * By default, this calls Callback with any saved results from HandleResult.
+   * For queries with only one result set, you don't need to override.
+   *
+   * @param aReason
+   *        SQL status code
+   */
+  NS_IMETHOD HandleCompletion(PRUint16 aReason);
+
+private:
+  // Used by HandleResult to cache results until HandleCompletion is called.
+  nsCOMPtr<mozIStorageResultSet> mResultSet;
+};
+
+NS_IMETHODIMP
+Step::ExecuteAsync(mozIStorageStatement* aStmt)
+{
+  nsCOMPtr<mozIStoragePendingStatement> handle;
+  nsresult rv = aStmt->ExecuteAsync(this, getter_AddRefs(handle));
+  NS_ENSURE_SUCCESS(rv, rv);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+Step::Callback(mozIStorageResultSet* aResultSet)
+{
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+Step::HandleResult(mozIStorageResultSet* aResultSet)
+{
+  mResultSet = aResultSet;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+Step::HandleCompletion(PRUint16 aReason)
+{
+  nsCOMPtr<mozIStorageResultSet> resultSet = mResultSet;
+  mResultSet = NULL;
+  Callback(resultSet);
+  return NS_OK;
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Anonymous Helpers
@@ -71,7 +153,7 @@ public:
 
   static nsresult Start(nsIURI* aURI)
   {
-    NS_ASSERTION(aURI, "Don't pass a null URI!");
+    NS_PRECONDITION(aURI, "Null URI");
 
     nsNavHistory* navHist = nsNavHistory::GetHistoryService();
     NS_ENSURE_TRUE(navHist, NS_ERROR_FAILURE);
@@ -144,6 +226,478 @@ NS_IMPL_ISUPPORTS1(
   mozIStorageStatementCallback
 )
 
+/**
+ * Fail-safe mechanism for ensuring that your task completes, no matter what.
+ * Pass this around as an nsAutoPtr in your steps to guarantee that when all
+ * your steps are finished, your task is finished.
+ */
+class FailSafeFinishTask
+{
+public:
+  ~FailSafeFinishTask() {
+    History::GetService()->CurrentTaskFinished();
+  }
+};
+
+////////////////////////////////////////////////////////////////////////////////
+//// Steps for VisitURI
+
+struct VisitURIData : public FailSafeFinishTask
+{
+  PRInt64 placeId;
+  PRInt32 hidden;
+  PRInt32 typed;
+  nsCOMPtr<nsIURI> uri;
+
+  // Url of last added visit in chain.
+  nsCString lastSpec;
+  PRInt64 lastVisitId;
+  PRInt32 transitionType;
+  PRInt64 sessionId;
+  PRTime dateTime;
+};
+
+/**
+ * Step 6: Update frecency of URI and notify observers.
+ */
+class UpdateFrecencyAndNotifyStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  UpdateFrecencyAndNotifyStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    // Result set contains new visit created in earlier step
+    NS_ENSURE_STATE(aResultSet);
+
+    nsCOMPtr<mozIStorageRow> row;
+    nsresult rv = aResultSet->GetNextRow(getter_AddRefs(row));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRInt64 visitId;
+    rv = row->GetInt64(0, &visitId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    // TODO need to figure out story for not synchronous frecency updating
+    // (bug 556631)
+
+    // Swallow errors here, since if we've gotten this far, it's more
+    // important to notify the observers below.
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+    NS_WARN_IF_FALSE(history, "Could not get history service");
+    nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+    NS_WARN_IF_FALSE(bookmarks, "Could not get bookmarks service");
+    if (history && bookmarks) {
+      // Update frecency *after* the visit info is in the db
+      nsresult rv = history->UpdateFrecency(
+        mData->placeId,
+        bookmarks->IsRealBookmark(mData->placeId)
+      );
+      NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not update frecency");
+
+      // Notify nsNavHistory observers of visit, but only for certain types of
+      // visits to maintain consistency with nsNavHistory::GetQueryResults.
+      if (!mData->hidden &&
+          mData->transitionType != nsINavHistoryService::TRANSITION_EMBED &&
+          mData->transitionType != nsINavHistoryService::TRANSITION_FRAMED_LINK) {
+        history->FireOnVisit(mData->uri, visitId, mData->dateTime,
+                             mData->sessionId, mData->lastVisitId,
+                             mData->transitionType);
+      }
+    }
+
+    nsCOMPtr<nsIObserverService> obsService =
+      mozilla::services::GetObserverService();
+    if (obsService) {
+      nsresult rv = obsService->NotifyObservers(mData->uri, URI_VISIT_SAVED, nsnull);
+      NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not notify observers");
+    }
+
+    History::GetService()->NotifyVisited(mData->uri);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  UpdateFrecencyAndNotifyStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 5: Get newly created visit ID from moz_history_visits table.
+ */
+class GetVisitIDStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  GetVisitIDStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    // Find visit ID, needed for notifying observers in next step.
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+    NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
+    nsCOMPtr<mozIStorageStatement> stmt =
+      history->GetStatementById(DB_RECENT_VISIT_OF_URL);
+    NS_ENSURE_STATE(stmt);
+
+    nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mData->uri);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCOMPtr<Step> step = new UpdateFrecencyAndNotifyStep(mData);
+    rv = step->ExecuteAsync(stmt);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  GetVisitIDStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 4: Add visit to moz_history_visits table.
+ */
+class AddVisitStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  AddVisitStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    nsresult rv;
+
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+    NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
+
+    // TODO need to figure out story for new session IDs that isn't synchronous
+    // (bug 561450)
+
+    if (aResultSet) {
+      // Result set contains last visit information for this session
+      nsCOMPtr<mozIStorageRow> row;
+      rv = aResultSet->GetNextRow(getter_AddRefs(row));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      PRInt64 possibleSessionId;
+      PRTime lastVisitOfSession;
+
+      rv = row->GetInt64(0, &mData->lastVisitId);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = row->GetInt64(1, &possibleSessionId);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = row->GetInt64(2, &lastVisitOfSession);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      if (mData->dateTime - lastVisitOfSession <= RECENT_EVENT_THRESHOLD) {
+        mData->sessionId = possibleSessionId;
+      }
+      else {
+        // Session is too old. Start a new one.
+        mData->sessionId = history->GetNewSessionID();
+        mData->lastVisitId = 0;
+      }
+    }
+    else {
+      // No previous saved visit entry could be found, so start a new session.
+      mData->sessionId = history->GetNewSessionID();
+      mData->lastVisitId = 0;
+    }
+
+    nsCOMPtr<mozIStorageStatement> stmt =
+      history->GetStatementById(DB_INSERT_VISIT);
+    NS_ENSURE_STATE(stmt);
+
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("from_visit"),
+                               mData->lastVisitId);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"),
+                               mData->placeId);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("visit_date"),
+                               mData->dateTime);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("visit_type"),
+                               mData->transitionType);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("session"),
+                               mData->sessionId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCOMPtr<Step> step = new GetVisitIDStep(mData);
+    rv = step->ExecuteAsync(stmt);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  AddVisitStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 3: Callback for inserting or updating a moz_places entry.
+ *         This step checks database for the last visit in session.
+ */
+class CheckLastVisitStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  CheckLastVisitStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    nsresult rv;
+
+    if (aResultSet) {
+      // Last step inserted a new URL. This query contains the id.
+      nsCOMPtr<mozIStorageRow> row;
+      rv = aResultSet->GetNextRow(getter_AddRefs(row));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      rv = row->GetInt64(0, &mData->placeId);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    if (!mData->lastSpec.IsEmpty()) {
+      // Find last visit ID and session ID using lastSpec so we can add them
+      // to a browsing session if the visit was recent.
+      nsNavHistory* history = nsNavHistory::GetHistoryService();
+      NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
+      nsCOMPtr<mozIStorageStatement> stmt =
+        history->GetStatementById(DB_RECENT_VISIT_OF_URL);
+      NS_ENSURE_STATE(stmt);
+
+      rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mData->lastSpec);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      nsCOMPtr<Step> step = new AddVisitStep(mData);
+      rv = step->ExecuteAsync(stmt);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+    else {
+      // Empty lastSpec.
+      // Not part of a session.  Just run next step's callback with no results.
+      nsCOMPtr<Step> step = new AddVisitStep(mData);
+      rv = step->Callback(NULL);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  CheckLastVisitStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 2a: Called only when a new entry is put into moz_places.
+ *          Finds the ID of a recently inserted place.
+ */
+class FindNewIdStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  FindNewIdStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+    NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
+    nsCOMPtr<mozIStorageStatement> stmt =
+      history->GetStatementById(DB_GET_PAGE_VISIT_STATS);
+    NS_ENSURE_STATE(stmt);
+
+    nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mData->uri);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCOMPtr<Step> step = new CheckLastVisitStep(mData);
+    rv = step->ExecuteAsync(stmt);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  FindNewIdStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 2: Callback for checking for an existing URI in moz_places.
+ *         This step inserts or updates the URI accordingly.
+ */
+class CheckExistingStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  CheckExistingStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    nsresult rv;
+    nsCOMPtr<mozIStorageStatement> stmt;
+
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+    NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
+
+    if (aResultSet) {
+      nsCOMPtr<mozIStorageRow> row;
+      rv = aResultSet->GetNextRow(getter_AddRefs(row));
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      rv = row->GetInt64(0, &mData->placeId);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      if (!mData->typed) {
+        // If this transition wasn't typed, others might have been. If database
+        // has location as typed, reflect that in our data structure.
+        rv = row->GetInt32(2, &mData->typed);
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+      if (mData->hidden) {
+        // If this transition was hidden, it is possible that others were not.
+        // Any one visible transition makes this location visible. If database
+        // has location as visible, reflect that in our data structure.
+        rv = row->GetInt32(3, &mData->hidden);
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Note: trigger will update visit_count.
+      stmt = history->GetStatementById(DB_UPDATE_PAGE_VISIT_STATS);
+      NS_ENSURE_STATE(stmt);
+
+      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("typed"), mData->typed);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("hidden"), mData->hidden);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), mData->placeId);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      nsCOMPtr<Step> step = new CheckLastVisitStep(mData);
+      rv = step->ExecuteAsync(stmt);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+    else {
+      // No entry exists, so create one.
+      stmt = history->GetStatementById(DB_ADD_NEW_PAGE);
+      NS_ENSURE_STATE(stmt);
+
+      nsAutoString revHost;
+      rv = GetReversedHostname(mData->uri, revHost);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mData->uri);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->BindStringByName(NS_LITERAL_CSTRING("rev_host"), revHost);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("typed"), mData->typed);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("hidden"), mData->hidden);
+      NS_ENSURE_SUCCESS(rv, rv);
+      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("frecency"), -1);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      nsCOMPtr<Step> step = new FindNewIdStep(mData);
+      rv = step->ExecuteAsync(stmt);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  CheckExistingStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 1: See if there is an existing URI.
+ */
+class StartVisitURIStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  StartVisitURIStep(nsAutoPtr<VisitURIData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+
+    // Find existing entry in moz_places table, if any.
+    nsCOMPtr<mozIStorageStatement> stmt =
+      history->GetStatementById(DB_GET_PAGE_VISIT_STATS);
+    NS_ENSURE_STATE(stmt);
+
+    nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mData->uri);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCOMPtr<Step> step = new CheckExistingStep(mData);
+    rv = step->ExecuteAsync(stmt);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<VisitURIData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  StartVisitURIStep
+, Step
+)
+
 } // anonymous namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -160,6 +714,7 @@ History::History()
 History::~History()
 {
   gService = NULL;
+
 #ifdef DEBUG
   if (mObservers.IsInitialized()) {
     NS_ASSERTION(mObservers.Count() == 0,
@@ -168,6 +723,28 @@ History::~History()
 #endif
 }
 
+void
+History::AppendTask(Step* aTask)
+{
+  NS_PRECONDITION(aTask, "Got NULL task.");
+
+  NS_ADDREF(aTask);
+  mPendingVisits.Push(aTask);
+
+  if (mPendingVisits.GetSize() == 1) {
+    // There are no other pending tasks.
+    StartNextTask();
+  }
+}
+
+void
+History::CurrentTaskFinished()
+{
+  nsCOMPtr<Step> deadTaskWalking =
+    dont_AddRef(static_cast<Step*>(mPendingVisits.PopFront()));
+  StartNextTask();
+}
+
 void
 History::NotifyVisited(nsIURI* aURI)
 {
@@ -228,9 +805,107 @@ History::GetSingleton()
   return gService;
 }
 
+void
+History::StartNextTask()
+{
+  nsCOMPtr<Step> nextTask =
+    static_cast<Step*>(mPendingVisits.PeekFront());
+  if (!nextTask) {
+    // No more pending visits left to process.
+    return;
+  }
+  nsresult rv = nextTask->Callback(NULL);
+  NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Beginning a task failed.");
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// IHistory
 
+NS_IMETHODIMP
+History::VisitURI(nsIURI* aURI,
+                  nsIURI* aLastVisitedURI,
+                  PRUint32 aFlags)
+{
+  NS_PRECONDITION(aURI, "URI should not be NULL.");
+
+  nsNavHistory* history = nsNavHistory::GetHistoryService();
+  NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
+
+  // Silently return if URI is something we shouldn't add to DB.
+  PRBool canAdd;
+  nsresult rv = history->CanAddURI(aURI, &canAdd);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (!canAdd) {
+    return NS_OK;
+  }
+
+  // Populate data structure that will be used in our async SQL steps.
+  nsAutoPtr<VisitURIData> data(new VisitURIData());
+
+  nsCAutoString spec;
+  rv = aURI->GetSpec(spec);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (aLastVisitedURI) {
+    rv = aLastVisitedURI->GetSpec(data->lastSpec);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  if (spec.Equals(data->lastSpec)) {
+    // Do not save refresh-page visits.
+    return NS_OK;
+  }
+
+  // Assigns a type to the edge in the visit linked list. Each type will be
+  // considered differently when weighting the frecency of a location.
+  PRUint32 recentFlags = history->GetRecentFlags(aURI);
+  bool redirected = false;
+  if (aFlags & IHistory::REDIRECT_TEMPORARY) {
+    data->transitionType = nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY;
+    redirected = true;
+  }
+  else if (aFlags & IHistory::REDIRECT_PERMANENT) {
+    data->transitionType = nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT;
+    redirected = true;
+  }
+  else if (recentFlags & nsNavHistory::RECENT_TYPED) {
+    data->transitionType = nsINavHistoryService::TRANSITION_TYPED;
+  }
+  else if (recentFlags & nsNavHistory::RECENT_BOOKMARKED) {
+    data->transitionType = nsINavHistoryService::TRANSITION_BOOKMARK;
+  }
+  else if (aFlags & IHistory::TOP_LEVEL) {
+    // User was redirected or link was clicked in the main window.
+    data->transitionType = nsINavHistoryService::TRANSITION_LINK;
+  }
+  else if (recentFlags & nsNavHistory::RECENT_ACTIVATED) {
+    // User activated a link in a frame.
+    data->transitionType = nsINavHistoryService::TRANSITION_FRAMED_LINK;
+  }
+  else {
+    // A frame redirected to a new site without user interaction.
+    data->transitionType = nsINavHistoryService::TRANSITION_EMBED;
+  }
+
+  data->typed = (data->transitionType == nsINavHistoryService::TRANSITION_TYPED) ? 1 : 0;
+  data->hidden = 
+    (data->transitionType == nsINavHistoryService::TRANSITION_FRAMED_LINK ||
+     data->transitionType == nsINavHistoryService::TRANSITION_EMBED ||
+     redirected) ? 1 : 0;
+  data->dateTime = PR_Now();
+  data->uri = aURI;
+
+  nsCOMPtr<Step> task(new StartVisitURIStep(data));
+  AppendTask(task);
+
+  nsCOMPtr<nsIObserverService> obsService =
+    mozilla::services::GetObserverService();
+  if (obsService) {
+    obsService->NotifyObservers(aURI, NS_LINK_VISITED_EVENT_TOPIC, nsnull);
+  }
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 History::RegisterVisitedCallback(nsIURI* aURI,
                                  Link* aLink)
@@ -312,8 +987,8 @@ History::UnregisterVisitedCallback(nsIURI* aURI,
 //// nsISupports
 
 NS_IMPL_ISUPPORTS1(
-  History,
+  History
-  IHistory
+, IHistory
 )
 
 } // namespace places

toolkit/components/places/src/History.h

@@ -46,6 +46,7 @@
 #include "nsString.h"
 #include "nsURIHashKey.h"
 #include "nsTArray.h"
+#include "nsDeque.h"
 
 namespace mozilla {
 namespace places {
@@ -69,6 +70,26 @@ public:
    */
   void NotifyVisited(nsIURI *aURI);
 
+  /**
+   * Append a task to the queue for SQL queries that need to happen
+   * atomically.
+   *
+   * @pre aTask is not null
+   *
+   * @param aTask
+   *        Task that needs to be completed atomically
+   */
+  void AppendTask(class Step* aTask);
+
+  /**
+   * Call when all steps of the current running task are finished.  Each task
+   * should be responsible for calling this when it is finished (even if there
+   * are errors).
+   *
+   * Do not call this twice for the same visit.
+   */
+  void CurrentTaskFinished();
+
   /**
    * Obtains a pointer to this service.
    */
@@ -83,6 +104,26 @@ public:
 private:
   ~History();
 
+  /**
+   * Since visits rapidly fire at once, it's very likely to have race
+   * conditions for SQL queries.  We often need to see if a row exists
+   * or peek at values, and by the time we have retrieved them they could
+   * be different.
+   *
+   * We guarantee an ordering of our SQL statements so that a set of
+   * callbacks for one visit are guaranteed to be atomic.  Each visit consists
+   * of a data structure that sits in this queue.
+   *
+   * The front of the queue always has the current visit we are processing.
+   */
+  nsDeque mPendingVisits;
+
+  /**
+   * Begins next task at the front of the queue.  The task remains in the queue
+   * until it is done and calls CurrentTaskFinished.
+   */
+  void StartNextTask();
+
   static History *gService;
 
   typedef nsTArray<mozilla::dom::Link *> ObserverArray;

toolkit/components/places/src/nsNavHistory.cpp

@@ -88,11 +88,6 @@
 
 using namespace mozilla::places;
 
-// Microsecond timeout for "recent" events such as typed and bookmark following.
-// If you typed it more than this time ago, it's not recent.
-// This is 15 minutes           m    s/m  us/s
-#define RECENT_EVENT_THRESHOLD PRTime((PRInt64)15 * 60 * PR_USEC_PER_SEC)
-
 // The maximum number of things that we will store in the recent events list
 // before calling ExpireNonrecentEvents. This number should be big enough so it
 // is very difficult to get that many unconsumed events (for example, typed but
@@ -237,21 +232,12 @@ NS_IMPL_CI_INTERFACE_GETTER5(
 
 namespace {
 
-static nsresult GetReversedHostname(nsIURI* aURI, nsAString& host);
-static void GetReversedHostname(const nsString& aForward, nsAString& aReversed);
 static PRInt64 GetSimpleBookmarksQueryFolder(
     const nsCOMArray<nsNavHistoryQuery>& aQueries,
     nsNavHistoryQueryOptions* aOptions);
 static void ParseSearchTermsFromQueries(const nsCOMArray<nsNavHistoryQuery>& aQueries,
                                         nsTArray<nsTArray<nsString>*>* aTerms);
 
-inline void ReverseString(const nsString& aInput, nsAString& aReversed)
-{
-  aReversed.Truncate(0);
-  for (PRInt32 i = aInput.Length() - 1; i >= 0; i --)
-    aReversed.Append(aInput[i]);
-}
-
 } // anonymous namespace
 
 namespace mozilla {
@@ -909,6 +895,27 @@ nsNavHistory::UpdateSchemaVersion()
 }
 
 
+PRUint32
+nsNavHistory::GetRecentFlags(nsIURI *aURI)
+{
+  PRUint32 result = 0;
+  nsCAutoString spec;
+  nsresult rv = aURI->GetSpec(spec);
+  NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Unable to get aURI's spec");
+
+  if (NS_SUCCEEDED(rv)) {
+    if (CheckIsRecentEvent(&mRecentTyped, spec))
+      result |= RECENT_TYPED;
+    if (CheckIsRecentEvent(&mRecentLink, spec))
+      result |= RECENT_ACTIVATED;
+    if (CheckIsRecentEvent(&mRecentBookmark, spec))
+      result |= RECENT_BOOKMARKED;
+  }
+
+  return result;
+}
+
+
 /**
  * Called after InitDB, this creates our own functions
  */
@@ -1865,6 +1872,9 @@ nsNavHistory::GetUrlIdFor(nsIURI* aURI, PRInt64* aEntryID,
 //    THIS SHOULD BE THE ONLY PLACE NEW moz_places ROWS ARE
 //    CREATED. This allows us to maintain better consistency.
 //
+//    XXX this functionality is being moved to History.cpp, so
+//    in fact there *are* two places where new pages are added.
+//
 //    If non-null, the new page ID will be placed into aPageID.
 
 nsresult
@@ -2162,6 +2172,22 @@ nsNavHistory::GetNewSessionID()
 }
 
 
+void
+nsNavHistory::FireOnVisit(nsIURI* aURI,
+                          PRInt64 aVisitID,
+                          PRTime aTime,
+                          PRInt64 aSessionID,
+                          PRInt64 referringVisitID,
+                          PRInt32 aTransitionType)
+{
+  PRUint32 added = 0;
+  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                   nsINavHistoryObserver,
+                   OnVisit(aURI, aVisitID, aTime, aSessionID,
+                           referringVisitID, aTransitionType, &added));
+}
+
+
 PRInt32
 nsNavHistory::GetDaysOfHistory() {
   PRInt32 daysOfHistory = 0;
@@ -2866,12 +2892,9 @@ nsNavHistory::AddVisit(nsIURI* aURI, PRTime aTime, nsIURI* aReferringURI,
   // Notify observers: The hidden detection code must match that in
   // GetQueryResults to maintain consistency.
   // FIXME bug 325241: make a way to observe hidden URLs
-  PRUint32 added = 0;
   if (!hidden) {
-    NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+    FireOnVisit(aURI, *aVisitID, aTime, aSessionID, referringVisitID,
-                     nsINavHistoryObserver,
+                aTransitionType);
-                     OnVisit(aURI, *aVisitID, aTime, aSessionID,
-                             referringVisitID, aTransitionType, &added));
   }
 
   // Normally docshell sends the link visited observer notification for us (this
@@ -7547,52 +7570,6 @@ nsNavHistory::RemoveDuplicateURIs()
 
 namespace {
 
-// GetReversedHostname
-//
-//    This extracts the hostname from the URI and reverses it in the
-//    form that we use (always ending with a "."). So
-//    "http://microsoft.com/" becomes "moc.tfosorcim."
-//
-//    The idea behind this is that we can create an index over the items in
-//    the reversed host name column, and then query for as much or as little
-//    of the host name as we feel like.
-//
-//    For example, the query "host >= 'gro.allizom.' AND host < 'gro.allizom/'
-//    Matches all host names ending in '.mozilla.org', including
-//    'developer.mozilla.org' and just 'mozilla.org' (since we define all
-//    reversed host names to end in a period, even 'mozilla.org' matches).
-//    The important thing is that this operation uses the index. Any substring
-//    calls in a select statement (even if it's for the beginning of a string)
-//    will bypass any indices and will be slow).
-
-nsresult
-GetReversedHostname(nsIURI* aURI, nsAString& aRevHost)
-{
-  nsCString forward8;
-  nsresult rv = aURI->GetHost(forward8);
-  if (NS_FAILED(rv)) {
-    return rv;
-  }
-
-  // can't do reversing in UTF8, better use 16-bit chars
-  NS_ConvertUTF8toUTF16 forward(forward8);
-  GetReversedHostname(forward, aRevHost);
-  return NS_OK;
-}
-
-
-// GetReversedHostname
-//
-//    Same as previous but for strings
-
-void
-GetReversedHostname(const nsString& aForward, nsAString& aRevHost)
-{
-  ReverseString(aForward, aRevHost);
-  aRevHost.Append(PRUnichar('.'));
-}
-
-
 // GetSimpleBookmarksQueryFolder
 //
 //    Determines if this set of queries is a simple bookmarks query for a

toolkit/components/places/src/nsNavHistory.h

@@ -88,6 +88,10 @@
 #define URI_LENGTH_MAX 65536
 #define TITLE_LENGTH_MAX 4096
 
+// Microsecond timeout for "recent" events such as typed and bookmark following.
+// If you typed it more than this time ago, it's not recent.
+#define RECENT_EVENT_THRESHOLD PRTime((PRInt64)15 * 60 * PR_USEC_PER_SEC)
+
 #ifdef MOZ_XUL
 // Fired after autocomplete feedback has been updated.
 #define TOPIC_AUTOCOMPLETE_FEEDBACK_UPDATED "places-autocomplete-feedback-updated"
@@ -112,6 +116,11 @@ namespace places {
     DB_GET_PAGE_INFO_BY_URL = 0
   , DB_GET_TAGS = 1
   , DB_IS_PAGE_VISITED = 2
+  , DB_INSERT_VISIT = 3
+  , DB_RECENT_VISIT_OF_URL = 4
+  , DB_GET_PAGE_VISIT_STATS = 5
+  , DB_UPDATE_PAGE_VISIT_STATS = 6
+  , DB_ADD_NEW_PAGE = 7
   };
 
 } // namespace places
@@ -394,6 +403,19 @@ public:
    */
   bool canNotify() { return mCanNotify; }
 
+  enum RecentEventFlags {
+    RECENT_TYPED      = 1 << 0,    // User typed in URL recently
+    RECENT_ACTIVATED  = 1 << 1,    // User tapped URL link recently
+    RECENT_BOOKMARKED = 1 << 2     // User bookmarked URL recently
+  };
+
+  /**
+   * Returns any recent activity done with a URL.
+   * @return Any recent events associated with this URI.  Each bit is set
+   *         according to RecentEventFlags enum values.
+   */
+  PRUint32 GetRecentFlags(nsIURI *aURI);
+
   mozIStorageStatement* GetStatementById(
     enum mozilla::places::HistoryStatementId aStatementId
   )
@@ -406,10 +428,32 @@ public:
         return mDBGetTags;
       case DB_IS_PAGE_VISITED:
         return mDBIsPageVisited;
+      case DB_INSERT_VISIT:
+        return mDBInsertVisit;
+      case DB_RECENT_VISIT_OF_URL:
+        return mDBRecentVisitOfURL;
+      case DB_GET_PAGE_VISIT_STATS:
+        return mDBGetPageVisitStats;
+      case DB_UPDATE_PAGE_VISIT_STATS:
+        return mDBUpdatePageVisitStats;
+      case DB_ADD_NEW_PAGE:
+        return mDBAddNewPage;
     }
     return nsnull;
   }
 
+  PRInt64 GetNewSessionID();
+
+  /**
+   * Fires onVisit event to nsINavHistoryService observers
+   */
+  void FireOnVisit(nsIURI* aURI,
+                   PRInt64 aVisitID,
+                   PRTime aTime,
+                   PRInt64 aSessionID,
+                   PRInt64 referringVisitID,
+                   PRInt32 aTransitionType);
+
 private:
   ~nsNavHistory();
 
@@ -687,7 +731,6 @@ protected:
 
   // Sessions tracking.
   PRInt64 mLastSessionID;
-  PRInt64 GetNewSessionID();
 
 #ifdef MOZ_XUL
   // AutoComplete stuff

toolkit/components/places/tests/browser/Makefile.in

@@ -47,6 +47,7 @@ include $(topsrcdir)/config/rules.mk
 
 _BROWSER_FILES = \
 	browser_bug399606.js \
+	browser_visituri.js \
 	$(NULL)
 
 # These are files that need to be loaded via the HTTP proxy server
@@ -58,6 +59,10 @@ _HTTP_FILES = \
 	bug_399606/399606-window.location.href.html \
 	bug_399606/399606-window.location.html \
 	bug_399606/399606-history.go-0.html \
+	visituri/begin.html \
+	visituri/redirect_twice.sjs \
+	visituri/redirect_once.sjs \
+	visituri/final.html \
 	$(NULL)
 
 libs:: $(_BROWSER_FILES)

toolkit/components/places/tests/cpp/places_test_harness.h

@@ -47,6 +47,9 @@
 #include "nsINavHistoryService.h"
 #include "nsIObserverService.h"
 #include "mozilla/IHistory.h"
+#include "mozIStorageConnection.h"
+#include "mozIStorageStatement.h"
+#include "nsPIPlacesDatabase.h"
 
 using namespace mozilla;
 
@@ -117,6 +120,21 @@ addURI(nsIURI* aURI)
   do_check_success(rv);
 }
 
+struct PlaceRecord
+{
+  PRInt64 id;
+  PRInt32 hidden;
+  PRInt32 typed;
+  PRInt32 visitCount;
+};
+
+struct VisitRecord
+{
+  PRInt64 id;
+  PRInt64 lastVisitId;
+  PRInt32 transitionType;
+};
+
 already_AddRefed<IHistory>
 do_get_IHistory()
 {
@@ -124,3 +142,107 @@ do_get_IHistory()
   do_check_true(history);
   return history.forget();
 }
+
+already_AddRefed<nsINavHistoryService>
+do_get_NavHistory()
+{
+  nsCOMPtr<nsINavHistoryService> serv =
+    do_GetService(NS_NAVHISTORYSERVICE_CONTRACTID);
+  do_check_true(serv);
+  return serv.forget();
+}
+
+already_AddRefed<mozIStorageConnection>
+do_get_db()
+{
+  nsCOMPtr<nsINavHistoryService> history = do_get_NavHistory();
+  nsCOMPtr<nsPIPlacesDatabase> database = do_QueryInterface(history);
+  do_check_true(database);
+
+  mozIStorageConnection* dbConn;
+  nsresult rv = database->GetDBConnection(&dbConn);
+  do_check_success(rv);
+  return dbConn;
+}
+
+/**
+ * Get the place record from the database.
+ *
+ * @param aURI The unique URI of the place we are looking up
+ * @param result Out parameter where the result is stored
+ */
+void
+do_get_place(nsIURI* aURI, PlaceRecord& result)
+{
+  nsCOMPtr<mozIStorageConnection> dbConn = do_get_db();
+  nsCOMPtr<mozIStorageStatement> stmt;
+
+  nsCString spec;
+  nsresult rv = aURI->GetSpec(spec);
+  do_check_success(rv);
+
+  rv = dbConn->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT id, hidden, typed, visit_count FROM moz_places_temp "
+    "WHERE url=?1 "
+    "UNION ALL "
+    "SELECT id, hidden, typed, visit_count FROM moz_places "
+    "WHERE url=?1 "
+    "LIMIT 1"
+  ), getter_AddRefs(stmt));
+  do_check_success(rv);
+
+  rv = stmt->BindUTF8StringParameter(0, spec);
+  do_check_success(rv);
+
+  PRBool hasResults;
+  rv = stmt->ExecuteStep(&hasResults);
+  do_check_true(hasResults);
+  do_check_success(rv);
+
+  rv = stmt->GetInt64(0, &result.id);
+  do_check_success(rv);
+  rv = stmt->GetInt32(1, &result.hidden);
+  do_check_success(rv);
+  rv = stmt->GetInt32(2, &result.typed);
+  do_check_success(rv);
+  rv = stmt->GetInt32(3, &result.visitCount);
+  do_check_success(rv);
+}
+
+/**
+ * Gets the most recent visit to a place.
+ *
+ * @param placeID ID from the moz_places table
+ * @param result Out parameter where visit is stored
+ */
+void
+do_get_lastVisit(PRInt64 placeId, VisitRecord& result)
+{
+  nsCOMPtr<mozIStorageConnection> dbConn = do_get_db();
+  nsCOMPtr<mozIStorageStatement> stmt;
+
+  nsresult rv = dbConn->CreateStatement(NS_LITERAL_CSTRING(
+    "SELECT id, from_visit, visit_type FROM moz_historyvisits_temp "
+    "WHERE place_id=?1 "
+    "UNION ALL "
+    "SELECT id, from_visit, visit_type FROM moz_historyvisits "
+    "WHERE place_id=?1 "
+    "LIMIT 1"
+  ), getter_AddRefs(stmt));
+  do_check_success(rv);
+
+  rv = stmt->BindInt64Parameter(0, placeId);
+  do_check_success(rv);
+
+  PRBool hasResults;
+  rv = stmt->ExecuteStep(&hasResults);
+  do_check_true(hasResults);
+  do_check_success(rv);
+
+  rv = stmt->GetInt64(0, &result.id);
+  do_check_success(rv);
+  rv = stmt->GetInt64(1, &result.lastVisitId);
+  do_check_success(rv);
+  rv = stmt->GetInt32(2, &result.transitionType);
+  do_check_success(rv);
+}

toolkit/components/places/tests/cpp/test_IHistory.cpp

@@ -38,6 +38,7 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "places_test_harness.h"
+#include "nsIBrowserHistory.h"
 
 #include "mock_Link.h"
 using namespace mozilla::dom;
@@ -76,6 +77,53 @@ new_test_uri()
   return testURI.forget();
 }
 
+class VisitURIObserver : public nsIObserver
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  VisitURIObserver(int aExpectedVisits = 1) :
+    mVisits(0),
+    mExpectedVisits(aExpectedVisits)
+  {
+    nsCOMPtr<nsIObserverService> observerService =
+      do_GetService(NS_OBSERVERSERVICE_CONTRACTID);
+    do_check_true(observerService);
+    (void)observerService->AddObserver(this,
+                                       "uri-visit-saved",
+                                       PR_FALSE);
+  }
+
+  void WaitForNotification()
+  {
+    while (mVisits < mExpectedVisits) {
+      (void)NS_ProcessNextEvent();
+    }
+  }
+
+  NS_IMETHOD Observe(nsISupports* aSubject,
+                     const char* aTopic,
+                     const PRUnichar* aData)
+  {
+    mVisits++;
+
+    if (mVisits == mExpectedVisits) {
+      nsCOMPtr<nsIObserverService> observerService =
+        do_GetService(NS_OBSERVERSERVICE_CONTRACTID);
+      (void)observerService->RemoveObserver(this, "uri-visit-saved");
+    }
+
+    return NS_OK;
+  }
+private:
+  int mVisits;
+  int mExpectedVisits;
+};
+NS_IMPL_ISUPPORTS1(
+  VisitURIObserver,
+  nsIObserver
+)
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Functions
 
@@ -363,6 +411,145 @@ test_observer_topic_dispatched()
   run_next_test();
 }
 
+void
+test_visituri_inserts()
+{
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsCOMPtr<nsIURI> lastURI(new_test_uri());
+  nsCOMPtr<nsIURI> visitedURI(new_test_uri());
+
+  history->VisitURI(visitedURI, lastURI, mozilla::IHistory::TOP_LEVEL);
+
+  nsCOMPtr<VisitURIObserver> finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  PlaceRecord place;
+  do_get_place(visitedURI, place);
+
+  do_check_true(place.id > 0);
+  do_check_false(place.hidden);
+  do_check_false(place.typed);
+  do_check_true(place.visitCount == 1);
+
+  run_next_test();
+}
+
+void
+test_visituri_updates()
+{
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsCOMPtr<nsIURI> lastURI(new_test_uri());
+  nsCOMPtr<nsIURI> visitedURI(new_test_uri());
+  nsCOMPtr<VisitURIObserver> finisher;
+
+  history->VisitURI(visitedURI, lastURI, mozilla::IHistory::TOP_LEVEL);
+  finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  history->VisitURI(visitedURI, lastURI, mozilla::IHistory::TOP_LEVEL);
+  finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  PlaceRecord place;
+  do_get_place(visitedURI, place);
+
+  do_check_true(place.visitCount == 2);
+
+  run_next_test();
+}
+
+void
+test_visituri_preserves_shown_and_typed()
+{
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsCOMPtr<nsIURI> lastURI(new_test_uri());
+  nsCOMPtr<nsIURI> visitedURI(new_test_uri());
+
+  history->VisitURI(visitedURI, lastURI, mozilla::IHistory::TOP_LEVEL);
+  // this simulates the uri visit happening in a frame.  Normally frame
+  // transitions would be hidden unless it was previously loaded top-level
+  history->VisitURI(visitedURI, lastURI, 0);
+
+  nsCOMPtr<VisitURIObserver> finisher = new VisitURIObserver(2);
+  finisher->WaitForNotification();
+
+  PlaceRecord place;
+  do_get_place(visitedURI, place);
+  do_check_false(place.hidden);
+
+  run_next_test();
+}
+
+void
+test_visituri_creates_visit()
+{
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsCOMPtr<nsIURI> lastURI(new_test_uri());
+  nsCOMPtr<nsIURI> visitedURI(new_test_uri());
+
+  history->VisitURI(visitedURI, lastURI, mozilla::IHistory::TOP_LEVEL);
+  nsCOMPtr<VisitURIObserver> finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  PlaceRecord place;
+  VisitRecord visit;
+  do_get_place(visitedURI, place);
+  do_get_lastVisit(place.id, visit);
+
+  do_check_true(visit.id > 0);
+  do_check_true(visit.lastVisitId == 0);
+  do_check_true(visit.transitionType == nsINavHistoryService::TRANSITION_LINK);
+
+  run_next_test();
+}
+
+void
+test_visituri_transition_typed()
+{
+  nsCOMPtr<nsINavHistoryService> navHistory = do_get_NavHistory();
+  nsCOMPtr<nsIBrowserHistory> browserHistory = do_QueryInterface(navHistory);
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsCOMPtr<nsIURI> lastURI(new_test_uri());
+  nsCOMPtr<nsIURI> visitedURI(new_test_uri());
+
+  browserHistory->MarkPageAsTyped(visitedURI);
+  history->VisitURI(visitedURI, lastURI, mozilla::IHistory::TOP_LEVEL);
+  nsCOMPtr<VisitURIObserver> finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  PlaceRecord place;
+  VisitRecord visit;
+  do_get_place(visitedURI, place);
+  do_get_lastVisit(place.id, visit);
+
+  do_check_true(visit.transitionType == nsINavHistoryService::TRANSITION_TYPED);
+
+  run_next_test();
+}
+
+void
+test_visituri_transition_embed()
+{
+  nsCOMPtr<nsINavHistoryService> navHistory = do_get_NavHistory();
+  nsCOMPtr<nsIBrowserHistory> browserHistory = do_QueryInterface(navHistory);
+  nsCOMPtr<IHistory> history(do_get_IHistory());
+  nsCOMPtr<nsIURI> lastURI(new_test_uri());
+  nsCOMPtr<nsIURI> visitedURI(new_test_uri());
+
+  history->VisitURI(visitedURI, lastURI, 0);
+  nsCOMPtr<VisitURIObserver> finisher = new VisitURIObserver();
+  finisher->WaitForNotification();
+
+  PlaceRecord place;
+  VisitRecord visit;
+  do_get_place(visitedURI, place);
+  do_get_lastVisit(place.id, visit);
+
+  do_check_true(visit.transitionType == nsINavHistoryService::TRANSITION_EMBED);
+
+  run_next_test();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Harness
 
@@ -378,6 +565,12 @@ Test gTests[] = {
   TEST(test_new_visit_notifies_waiting_Link),
   TEST(test_RegisterVisitedCallback_returns_before_notifying),
   TEST(test_observer_topic_dispatched),
+  TEST(test_visituri_inserts),
+  TEST(test_visituri_updates),
+  TEST(test_visituri_preserves_shown_and_typed),
+  TEST(test_visituri_creates_visit),
+  TEST(test_visituri_transition_typed),
+  TEST(test_visituri_transition_embed),
 };
 
 const char* file = __FILE__;

xpcom/build/Makefile.in

@@ -113,6 +113,7 @@ LOCAL_INCLUDES	= \
 		-I$(srcdir)/../threads/_xpidlgen \
 		-I$(srcdir)/../proxy/src \
 		-I$(srcdir)/../reflect/xptinfo/src \
+		-I$(srcdir)/../../docshell/base \
 		$(NULL)
 
 EXPORTS_NAMESPACES = mozilla

xpcom/build/ServiceList.h

@@ -5,3 +5,14 @@ MOZ_SERVICE(XULOverlayProviderService, nsIXULOverlayProvider, "@mozilla.org/chro
 MOZ_SERVICE(IOService, nsIIOService, "@mozilla.org/network/io-service;1")
 MOZ_SERVICE(ObserverService, nsIObserverService, "@mozilla.org/observer-service;1")
 MOZ_SERVICE(StringBundleService, nsIStringBundleService, "@mozilla.org/intl/stringbundle;1")
+
+#ifdef MOZ_USE_NAMESPACE
+namespace mozilla
+{
+#endif
+
+MOZ_SERVICE(HistoryService, IHistory, "@mozilla.org/browser/history;1")
+
+#ifdef MOZ_USE_NAMESPACE
+}
+#endif

xpcom/build/Services.cpp

@@ -48,7 +48,9 @@
 #include "nsIStringBundle.h"
 #include "nsIToolkitChromeRegistry.h"
 #include "nsIXULOverlayProvider.h"
+#include "IHistory.h"
 
+using namespace mozilla;
 using namespace mozilla::services;
 
 /*

xpcom/build/Services.h

@@ -42,9 +42,12 @@
 #include "nscore.h"
 #include "nsCOMPtr.h"
 
+#define MOZ_USE_NAMESPACE
 #define MOZ_SERVICE(NAME, TYPE, SERVICE_CID) class TYPE;
+
 #include "ServiceList.h"
 #undef MOZ_SERVICE
+#undef MOZ_USE_NAMESPACE
 
 namespace mozilla {
 namespace services {