Bug 319910 r=bryner Add bookmark function for redirect-aware bookmark status checking

browser/components/places/public/nsINavBookmarksService.idl

@@ -342,10 +342,23 @@ interface nsINavBookmarksService : nsISupports
   boolean getFolderReadonly(in PRInt64 folder);
 
   /**
-   * Returns true if the given URI is in any bookmark folder.
+   * Returns true if the given URI is in any bookmark folder. If you want the
+   * results to be redirect-aware, use getBookmarkedURIFor()
    */
   boolean isBookmarked(in nsIURI uri);
 
+  /**
+   * Used to see if the given URI is bookmarked, or any page that redirected to
+   * it is bookmarked. For example, if I bookmark "mozilla.org" by manually
+   * typing it in, and follow the bookmark, I will get redirected to
+   * "www.mozilla.org". Logically, this new page is also bookmarked. This
+   * function, if given "www.mozilla.org", will return the URI of the bookmark,
+   * in this case "mozilla.org".
+   *
+   * If there is no bookmarked page found, it will return NULL.
+   */
+  nsIURI getBookmarkedURIFor(in nsIURI uri);
+
   /**
    * Returns the list of folder ids that contain the given URI.
    */

browser/components/places/src/nsNavBookmarks.cpp

@@ -90,7 +90,6 @@ nsNavBookmarks::Init()
 
   nsNavHistory *history = History();
   NS_ENSURE_TRUE(history, NS_ERROR_UNEXPECTED);
-  history->AddObserver(this, PR_FALSE); // allows us to notify on title changes
   mozIStorageConnection *dbConn = DBConn();
   mozStorageTransaction transaction(dbConn, PR_FALSE);
 
@@ -255,6 +254,23 @@ nsNavBookmarks::Init()
                                getter_AddRefs(mDBGetChildAt));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // mDBGetRedirectDestinations
+  // input = page ID, time threshold; output = unique ID input has redirected to
+  rv = dbConn->CreateStatement(NS_LITERAL_CSTRING(
+      "SELECT dest_v.page_id "
+      "FROM moz_historyvisit source_v "
+      "LEFT JOIN moz_historyvisit dest_v ON dest_v.from_visit = source_v.visit_id "
+      "WHERE source_v.page_id = ?1 "
+      "AND source_v.visit_date >= ?2 "
+      "AND (dest_v.visit_type = 5 OR dest_v.visit_type = 6) "
+      "GROUP BY dest_v.page_id"),
+    getter_AddRefs(mDBGetRedirectDestinations));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  FillBookmarksHash();
+
+  // must be last: This may cause bookmarks to be imported, which will exercise
+  // most of the bookmark system
   // keywords
   rv = dbConn->CreateStatement(NS_LITERAL_CSTRING(
       "SELECT k.keyword FROM moz_history h "
@@ -272,7 +288,17 @@ nsNavBookmarks::Init()
   rv = InitRoots();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return transaction.Commit();
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // allows us to notify on title changes. MUST BE LAST so it is impossible
+  // to fail after this call, or the history service will have a reference to
+  // us and we won't go away.
+  history->AddObserver(this, PR_FALSE);
+
+  // DO NOT PUT STUFF HERE that can fail. See observer comment above.
+
+  return NS_OK;
 }
 
 struct RenumberItem {
@@ -420,6 +446,222 @@ nsNavBookmarks::CreateRoot(mozIStorageStatement* aGetRootStatement,
   return NS_OK;
 }
 
+
+// nsNavBookmarks::FillBookmarksHash
+//
+//    This initializes the bookmarks hashtable that tells us which bookmark
+//    a given URI redirects to. This hashtable includes all URIs that
+//    redirect to bookmarks.
+//
+//    This is called from the bookmark init function and so is wrapped
+//    in that transaction (for better performance).
+
+nsresult
+nsNavBookmarks::FillBookmarksHash()
+{
+  nsresult rv;
+  PRBool hasMore;
+
+  // first init the hashtable
+  NS_ENSURE_TRUE(mBookmarksHash.Init(1024), NS_ERROR_OUT_OF_MEMORY);
+
+  // first populate the table with all bookmarks
+  nsCOMPtr<mozIStorageStatement> statement;
+  rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
+      "SELECT h.id "
+      "FROM moz_bookmarks b "
+      "JOIN moz_history h ON b.item_child = h.id"),
+    getter_AddRefs(statement));
+  NS_ENSURE_SUCCESS(rv, rv);
+  while (NS_SUCCEEDED(statement->ExecuteStep(&hasMore)) && hasMore) {
+    PRInt64 pageID;
+    rv = statement->GetInt64(0, &pageID);
+    NS_ENSURE_TRUE(mBookmarksHash.Put(pageID, pageID), NS_ERROR_OUT_OF_MEMORY);
+  }
+
+  // Find all pages h2 that have been redirected to from a bookmarked URI:
+  //    bookmarked -> url (h1)         url (h2)
+  //                    |                 ^
+  //                    .                 |
+  //                 visit (v1) -> destination visit (v2)
+  // This should catch most redirects, which are only one level. More levels of
+  // redirection will be handled separately.
+  rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
+      "SELECT v1.page_id, v2.page_id "
+      "FROM moz_bookmarks b "
+      "LEFT JOIN moz_historyvisit v1 on b.item_child = v1.page_id "
+      "LEFT JOIN moz_historyvisit v2 on v2.from_visit = v1.visit_id "
+      "WHERE v2.visit_type = 5 OR v2.visit_type = 6 " // perm. or temp. RDRs
+      "GROUP BY v2.page_id"),
+    getter_AddRefs(statement));
+  NS_ENSURE_SUCCESS(rv, rv);
+  while (NS_SUCCEEDED(statement->ExecuteStep(&hasMore)) && hasMore) {
+    PRInt64 fromId, toId;
+    statement->GetInt64(0, &fromId);
+    statement->GetInt64(1, &toId);
+
+    NS_ENSURE_TRUE(mBookmarksHash.Put(toId, fromId), NS_ERROR_OUT_OF_MEMORY);
+
+    // handle redirects deeper than one level
+    rv = RecursiveAddBookmarkHash(fromId, toId, 0);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  return NS_OK;
+}
+
+
+// nsNavBookmarks::AddBookmarkToHash
+//
+//    Given a bookmark that was potentially added, this goes through all
+//    redirects that this page may have resulted in and adds them to our hash.
+//    Note that this takes the ID of the URL in the history system, which we
+//    generally have when calling this function and which makes it faster.
+//
+//    For better performance, this call should be in a DB transaction.
+//
+//    @see RecursiveAddBookmarkHash
+
+nsresult
+nsNavBookmarks::AddBookmarkToHash(PRInt64 aBookmarkId, PRTime aMinTime)
+{
+  // this function might be called before our hashtable is initialized (for
+  // example, on history import), just ignore these, we'll pick up the add when
+  // the hashtable is initialized later
+  if (! mBookmarksHash.IsInitialized())
+    return NS_OK;
+  if (! mBookmarksHash.Put(aBookmarkId, aBookmarkId))
+    return NS_ERROR_OUT_OF_MEMORY;
+  return RecursiveAddBookmarkHash(aBookmarkId, aBookmarkId, aMinTime);
+}
+
+
+// nsNavBookmkars::RecursiveAddBookmarkHash
+//
+//    Used to add a new level of redirect information to the bookmark hash.
+//    Given a source bookmark 'aBookmark' and 'aCurrentSouce' that has already
+//    been added to the hashtable, this will add all redirect destinations of
+//    'aCurrentSort'. Will call itself recursively to walk down the chain.
+//
+//    'aMinTime' is the minimum time to consider visits from. Visits previous
+//    to this will not be considered. This allows the search to be much more
+//    efficient if you know something happened recently. Use 0 for the min time
+//    to search all history for redirects.
+
+nsresult
+nsNavBookmarks::RecursiveAddBookmarkHash(PRInt64 aBookmarkID,
+                                         PRInt64 aCurrentSource,
+                                         PRTime aMinTime)
+{
+  nsresult rv;
+  nsTArray<PRInt64> found;
+
+  // scope for the DB statement. The statement must be reset by the time we
+  // recursively call ourselves again, because our recursive call will use the
+  // same statement.
+  {
+    mozStorageStatementScoper scoper(mDBGetRedirectDestinations);
+    rv = mDBGetRedirectDestinations->BindInt64Parameter(0, aCurrentSource);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mDBGetRedirectDestinations->BindInt64Parameter(1, aMinTime);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRBool hasMore;
+    while (NS_SUCCEEDED(mDBGetRedirectDestinations->ExecuteStep(&hasMore)) &&
+           hasMore) {
+
+      // add this newly found redirect destination to the hashtable
+      PRInt64 curID;
+      rv = mDBGetRedirectDestinations->GetInt64(0, &curID);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      // It is very important we ignore anything already in our hashtable. It
+      // is actually pretty common to get loops of redirects. For example,
+      // a restricted page will redirect you to a login page, which will
+      // redirect you to the restricted page again with the proper cookie.
+      PRInt64 alreadyExistingOne;
+      if (mBookmarksHash.Get(curID, &alreadyExistingOne))
+        continue;
+
+      if (! mBookmarksHash.Put(curID, aBookmarkID))
+        return NS_ERROR_OUT_OF_MEMORY;
+
+      // save for recursion later
+      found.AppendElement(curID);
+    }
+  }
+
+  // recurse on each found item now that we're done with the statement
+  for (PRUint32 i = 0; i < found.Length(); i ++) {
+    rv = RecursiveAddBookmarkHash(aBookmarkID, found[i], aMinTime);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  return NS_OK;
+}
+
+
+// nsNavBookmarks::UpdateBookmarkHashOnRemove
+//
+//    Call this when a bookmark is removed. It will see if the bookmark still
+//    exists anywhere in the system, and, if not, remove all references to it
+//    in the bookmark hashtable.
+//
+//    The callback takes a pointer to what bookmark is being removed (as
+//    an Int64 history page ID) as the userArg and removes all redirect
+//    destinations that reference it.
+
+PR_STATIC_CALLBACK(PLDHashOperator)
+RemoveBookmarkHashCallback(nsTrimInt64HashKey::KeyType aKey,
+                           PRInt64& aBookmark, void* aUserArg)
+{
+  const PRInt64* removeThisOne = NS_REINTERPRET_CAST(const PRInt64*, aUserArg);
+  if (aBookmark == *removeThisOne)
+    return PL_DHASH_REMOVE;
+  return PL_DHASH_NEXT;
+}
+nsresult
+nsNavBookmarks::UpdateBookmarkHashOnRemove(PRInt64 aBookmarkId)
+{
+  // note we have to use the DB version here since the hashtable may be
+  // out-of-date
+  PRBool inDB;
+  nsresult rv = IsBookmarkedInDatabase(aBookmarkId, &inDB);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (inDB)
+    return NS_OK; // bookmark still exists, don't need to update hashtable
+
+  // remove it
+  mBookmarksHash.Enumerate(RemoveBookmarkHashCallback,
+                           NS_REINTERPRET_CAST(void*, &aBookmarkId));
+  return NS_OK;
+}
+
+
+// nsNavBookmarks::IsBookmarkedInDatabase
+//
+//    This checks to see if the specified URI is actually bookmarked, bypassing
+//    our hashtable. Normal IsBookmarked checks just use the hashtable.
+
+nsresult
+nsNavBookmarks::IsBookmarkedInDatabase(PRInt64 aBookmarkID,
+                                       PRBool *aIsBookmarked)
+{
+  // we'll just select position since it's just an int32 and may be faster.
+  // We don't actually care about the data, just whether there is any.
+  nsCOMPtr<mozIStorageStatement> statement;
+  nsresult rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING(
+      "SELECT position FROM moz_bookmarks WHERE item_child = ?1"),
+    getter_AddRefs(statement));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = statement->BindInt64Parameter(0, aBookmarkID);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return statement->ExecuteStep(aIsBookmarked);
+}
+
+
 nsresult
 nsNavBookmarks::AdjustIndices(PRInt64 aFolder,
                               PRInt32 aStartIndex, PRInt32 aEndIndex,
@@ -592,6 +834,8 @@ nsNavBookmarks::InsertItem(PRInt64 aFolder, nsIURI *aItem, PRInt32 aIndex)
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  AddBookmarkToHash(childID, 0);
+
   ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
                       OnItemAdded(aItem, aFolder, index))
 
@@ -643,6 +887,9 @@ nsNavBookmarks::RemoveItem(PRInt64 aFolder, nsIURI *aItem)
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
+  rv = UpdateBookmarkHashOnRemove(childID);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
                       OnItemRemoved(aItem, aFolder, childIndex))
 
@@ -1440,16 +1687,74 @@ nsNavBookmarks::FolderCount(PRInt64 aFolder)
 NS_IMETHODIMP
 nsNavBookmarks::IsBookmarked(nsIURI *aURI, PRBool *aBookmarked)
 {
-  *aBookmarked = PR_FALSE;
+  nsNavHistory* history = History();
+  NS_ENSURE_TRUE(history, NS_ERROR_UNEXPECTED);
 
-  mozStorageStatementScoper scope(mDBFindURIBookmarks);
+  // convert the URL to an ID
-
+  PRInt64 urlID;
-  nsresult rv = BindStatementURI(mDBFindURIBookmarks, 0, aURI);
+  nsresult rv = history->GetUrlIdFor(aURI, &urlID, PR_FALSE);
   NS_ENSURE_SUCCESS(rv, rv);
+  if (! urlID) {
+    // never seen this before, not even in history
+    *aBookmarked = PR_FALSE;
+    return NS_OK;
+  }
 
-  rv = mDBFindURIBookmarks->ExecuteStep(aBookmarked);
+  PRInt64 bookmarkedID;
+  PRBool foundOne = mBookmarksHash.Get(urlID, &bookmarkedID);
+
+  // IsBookmarked only tests if this exact URI is bookmarked, so we need to
+  // check that the destination matches
+  if (foundOne)
+    *aBookmarked = (urlID == bookmarkedID);
+  else
+    *aBookmarked = PR_FALSE;
+
+#ifdef DEBUG
+  // sanity check for the bookmark hashtable
+  PRBool realBookmarked;
+  rv = IsBookmarkedInDatabase(urlID, &realBookmarked);
+  NS_ASSERTION(realBookmarked == *aBookmarked,
+               "Bookmark hash table out-of-sync with the database");
+#endif
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsNavBookmarks::GetBookmarkedURIFor(nsIURI* aURI, nsIURI** _retval)
+{
+  *_retval = nsnull;
+
+  nsNavHistory* history = History();
+  NS_ENSURE_TRUE(history, NS_ERROR_UNEXPECTED);
+
+  // convert the URL to an ID
+  PRInt64 urlID;
+  nsresult rv = history->GetUrlIdFor(aURI, &urlID, PR_FALSE);
   NS_ENSURE_SUCCESS(rv, rv);
+  if (! urlID) {
+    // never seen this before, not even in history, leave result NULL
+    return NS_OK;
+  }
 
+  PRInt64 bookmarkID;
+  if (mBookmarksHash.Get(urlID, &bookmarkID)) {
+    // found one, convert ID back to URL. This statement is NOT refcounted
+    mozIStorageStatement* statement = history->DBGetIdPageInfo();
+    NS_ENSURE_TRUE(statement, NS_ERROR_UNEXPECTED);
+    mozStorageStatementScoper scoper(statement);
+
+    rv = statement->BindInt64Parameter(0, bookmarkID);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRBool hasMore;
+    if (NS_SUCCEEDED(statement->ExecuteStep(&hasMore)) && hasMore) {
+      nsCAutoString spec;
+      statement->GetUTF8String(nsNavHistory::kGetInfoIndex_URL, spec);
+      return NS_NewURI(_retval, spec);
+    }
+  }
   return NS_OK;
 }
 

browser/components/places/src/nsNavBookmarks.h

@@ -42,6 +42,7 @@
 #include "nsINavBookmarksService.h"
 #include "nsIStringBundle.h"
 #include "nsNavHistory.h"
+#include "nsNavHistoryResult.h" // need for Int64 hashtable
 #include "nsBrowserCompsCID.h"
 
 class nsNavBookmarks : public nsINavBookmarksService,
@@ -65,6 +66,8 @@ public:
     return sInstance;
   }
 
+  nsresult AddBookmarkToHash(PRInt64 aBookmarkId, PRTime aMinTime);
+
   nsresult ResultNodeForFolder(PRInt64 aID, nsNavHistoryQueryOptions *aOptions,
                                nsNavHistoryResultNode **aNode);
 
@@ -118,6 +121,15 @@ private:
   // be committed when our batch level reaches 0 again.
   PRBool mBatchHasTransaction;
 
+  // This stores a mapping from all pages reachable by redirects from bookmarked
+  // pages to the bookmarked page. Used by GetBookmarkedURIFor.
+  nsDataHashtable<nsTrimInt64HashKey, PRInt64> mBookmarksHash;
+  nsresult FillBookmarksHash();
+  nsresult RecursiveAddBookmarkHash(PRInt64 aBookmarkId, PRInt64 aCurrentSource,
+                                    PRTime aMinTime);
+  nsresult UpdateBookmarkHashOnRemove(PRInt64 aBookmarkId);
+  nsresult IsBookmarkedInDatabase(PRInt64 aBookmarkID, PRBool* aIsBookmarked);
+
   nsCOMPtr<mozIStorageStatement> mDBGetFolderInfo;    // kGetFolderInfoIndex_* results
 
   nsCOMPtr<mozIStorageStatement> mDBGetChildren;       // kGetInfoIndex_* results + kGetChildrenIndex_* results
@@ -138,6 +150,8 @@ private:
   nsCOMPtr<mozIStorageStatement> mDBIndexOfFolder;
   nsCOMPtr<mozIStorageStatement> mDBGetChildAt;
 
+  nsCOMPtr<mozIStorageStatement> mDBGetRedirectDestinations;
+
   // keywords
   nsCOMPtr<mozIStorageStatement> mDBGetKeywordForURI;
   nsCOMPtr<mozIStorageStatement> mDBGetURIForKeyword;

browser/components/places/src/nsNavHistory.cpp

@@ -82,6 +82,10 @@
 // This is 15 minutes           m    s/m  us/s
 #define RECENT_EVENT_THRESHOLD (15 * 60 * 1000000)
 
+// Microseconds ago to look for redirects when updating bookmarks. Used to
+// compute the threshold for nsNavBookmarks::AddBookmarkToHash
+#define BOOKMARK_REDIRECT_TIME_THRESHOLD (2 * 60 * 100000)
+
 // 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
@@ -2360,11 +2364,30 @@ nsNavHistory::AddURI(nsIURI *aURI, PRBool aRedirect,
 {
   mozStorageTransaction transaction(mDBConn, PR_FALSE);
 
+  PRInt64 redirectBookmark = 0;
   PRInt64 visitID, sessionID;
   nsresult rv = AddVisitChain(aURI, aToplevel, aRedirect, aReferrer,
-                              &visitID, &sessionID);
+                              &visitID, &sessionID, &redirectBookmark);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // The bookmark cache of redirects may be out-of-date with this addition, so
+  // we need to update it. The issue here is if they bookmark "mozilla.org" by
+  // typing it in without ever having visited "www.mozilla.org". They will then
+  // get redirected to the latter, and we need to add mozilla.org ->
+  // www.mozilla.org to the bookmark hashtable.
+  //
+  // AddVisitChain will put the spec of a bookmarked URI if it encounters one
+  // into bookmarkURI. If this is non-empty, we know that something has happened
+  // with a bookmark and we should probably go update it.
+  if (redirectBookmark) {
+    nsNavBookmarks* bookmarkService = nsNavBookmarks::GetBookmarksService();
+    if (bookmarkService) {
+      PRTime now = GetNow();
+      bookmarkService->AddBookmarkToHash(redirectBookmark,
+                                         now - BOOKMARK_REDIRECT_TIME_THRESHOLD);
+    }
+  }
+
   transaction.Commit();
   return NS_OK;
 }
@@ -2383,11 +2406,17 @@ nsNavHistory::AddURI(nsIURI *aURI, PRBool aRedirect,
 //    save them in mRecentRedirects. This function will add all of them for a
 //    given destination page when that page is actually visited.)
 //    See GetRedirectFor for more information about how redirects work.
+//
+//    aRedirectBookmark should be empty when this function is first called. If
+//    there are any redirects that are bookmarks the specs will be placed in
+//    this buffer. The caller can then determine if any bookmarked items were
+//    visited so it knows whether to update the bookmark service's redirect
+//    hashtable.
 
 nsresult
 nsNavHistory::AddVisitChain(nsIURI* aURI, PRBool aToplevel, PRBool aIsRedirect,
                             nsIURI* aReferrer, PRInt64* aVisitID,
-                            PRInt64* aSessionID)
+                            PRInt64* aSessionID, PRInt64* aRedirectBookmark)
 {
   PRUint32 transitionType = 0;
   PRInt64 referringVisit = 0;
@@ -2400,13 +2429,22 @@ nsNavHistory::AddVisitChain(nsIURI* aURI, PRBool aToplevel, PRBool aIsRedirect,
   nsCAutoString redirectSource;
   if (GetRedirectFor(spec, redirectSource, &visitTime, &transitionType)) {
     // this was a redirect: See GetRedirectFor for info on how this works
-
-    // Find the visit for the source
     nsCOMPtr<nsIURI> redirectURI;
     rv = NS_NewURI(getter_AddRefs(redirectURI), redirectSource);
     NS_ENSURE_SUCCESS(rv, rv);
+
+    // remember if any redirect sources were bookmarked
+    nsNavBookmarks* bookmarkService = nsNavBookmarks::GetBookmarksService();
+    PRBool isBookmarked;
+    if (bookmarkService &&
+        NS_SUCCEEDED(bookmarkService->IsBookmarked(redirectURI, &isBookmarked))
+        && isBookmarked) {
+      GetUrlIdFor(redirectURI, aRedirectBookmark, PR_FALSE);
+    }
+
+    // Find the visit for the source
     rv = AddVisitChain(redirectURI, aToplevel, PR_TRUE, aReferrer,
-                       &referringVisit, aSessionID);
+                       &referringVisit, aSessionID, aRedirectBookmark);
     NS_ENSURE_SUCCESS(rv, rv);
 
   } else if (aReferrer) {

browser/components/places/src/nsNavHistory.h

@@ -333,7 +333,7 @@ protected:
 
   nsresult AddVisitChain(nsIURI* aURI, PRBool aToplevel, PRBool aRedirect,
                          nsIURI* aReferrer, PRInt64* aVisitID,
-                         PRInt64* aSessionID);
+                         PRInt64* aSessionID, PRInt64* aRedirectBookmark);
   nsresult InternalAddNewPage(nsIURI* aURI, PRBool aHidden, PRBool aTyped,
                               PRInt32 aVisitCount, PRInt64* aPageID);
   nsresult InternalAddVisit(PRInt64 aPageID, PRInt64 aReferringVisit,