Bug 566738 - Add SetURITitle to IHistory. r=sdwilsh sr=bz

docshell/base/IHistory.h

@@ -43,6 +43,7 @@
 #include "nsISupports.h"
 
 class nsIURI;
+class nsString;
 
 namespace mozilla {
 
@@ -128,6 +129,18 @@ public:
         nsIURI *aLastVisitedURI,
         PRUint32 aFlags
     ) = 0;
+
+    /**
+     * Set the title of the URI.
+     *
+     * @pre aURI must not be null.
+     *
+     * @param aURI
+     *        The URI to set the title for.
+     * @param aTitle
+     *        The title string.
+     */
+    NS_IMETHOD SetURITitle(nsIURI* aURI, const nsString& aTitle) = 0;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(IHistory, IHISTORY_IID)
@@ -139,7 +152,8 @@ NS_DEFINE_STATIC_IID_ACCESSOR(IHistory, IHISTORY_IID)
                                          mozilla::dom::Link *aContent); \
     NS_IMETHOD VisitURI(nsIURI *aURI, \
                         nsIURI *aLastVisitedURI, \
-                        PRUint32 aFlags);
+                        PRUint32 aFlags); \
+    NS_IMETHOD SetURITitle(nsIURI* aURI, const nsString& aTitle);
 
 } // namespace mozilla
 

docshell/base/nsDocShell.cpp

@@ -4694,11 +4694,16 @@ nsDocShell::SetTitle(const PRUnichar * aTitle)
             treeOwnerAsWin->SetTitle(aTitle);
     }
 
-    if (mGlobalHistory && mCurrentURI && mLoadType != LOAD_ERROR_PAGE) {
-        mGlobalHistory->SetPageTitle(mCurrentURI, nsString(mTitle));
+    if (mCurrentURI && mLoadType != LOAD_ERROR_PAGE) {
+        nsCOMPtr<IHistory> history = services::GetHistoryService();
+        if (history) {
+            history->SetURITitle(mCurrentURI, nsString(mTitle));
+        }
+        else if (mGlobalHistory) {
+            mGlobalHistory->SetPageTitle(mCurrentURI, nsString(mTitle));
+        }
     }
 
-
     // Update SessionHistory with the document's title.
     if (mOSHE && mLoadType != LOAD_BYPASS_HISTORY &&
         mLoadType != LOAD_ERROR_PAGE) {

toolkit/components/places/src/History.cpp

@@ -305,7 +305,7 @@ public:
       if (!mData->hidden &&
           mData->transitionType != nsINavHistoryService::TRANSITION_EMBED &&
           mData->transitionType != nsINavHistoryService::TRANSITION_FRAMED_LINK) {
-        history->FireOnVisit(mData->uri, visitId, mData->dateTime,
+        history->NotifyOnVisit(mData->uri, visitId, mData->dateTime,
                              mData->sessionId, mData->lastVisitId,
                              mData->transitionType);
       }
@@ -696,6 +696,153 @@ NS_IMPL_ISUPPORTS1(
 , Step
 )
 
+////////////////////////////////////////////////////////////////////////////////
+//// Steps for SetURITitle
+
+struct SetTitleData : public FailSafeFinishTask
+{
+  nsCOMPtr<nsIURI> uri;
+  nsString title;
+};
+
+/**
+ * Step 3: Notify that title has been updated.
+ */
+class TitleNotifyStep: public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  TitleNotifyStep(nsAutoPtr<SetTitleData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+    history->NotifyTitleChange(mData->uri, mData->title);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<SetTitleData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  TitleNotifyStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 2: Set title.
+ */
+class SetTitleStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  SetTitleStep(nsAutoPtr<SetTitleData> aData)
+  : mData(aData)
+  {
+  }
+
+  NS_IMETHOD Callback(mozIStorageResultSet* aResultSet)
+  {
+    if (!aResultSet)
+      // URI record was not found.
+      return NS_OK;
+
+    nsCOMPtr<mozIStorageRow> row;
+    nsresult rv = aResultSet->GetNextRow(getter_AddRefs(row));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsAutoString title;
+    rv = row->GetString(2, title);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    // It is actually common to set the title to be the same thing it used to
+    // be. For example, going to any web page will always cause a title to be set,
+    // even though it will often be unchanged since the last visit. In these
+    // cases, we can avoid DB writing and observer overhead.
+    if (mData->title.Equals(title) || (mData->title.IsVoid() && title.IsVoid()))
+      return NS_OK;
+
+    nsNavHistory* history = nsNavHistory::GetHistoryService();
+
+    nsCOMPtr<mozIStorageStatement> stmt =
+      history->GetStatementById(DB_SET_PLACE_TITLE);
+    NS_ENSURE_STATE(stmt);
+
+    if (mData->title.IsVoid()) {
+      rv = stmt->BindNullByName(NS_LITERAL_CSTRING("page_title"));
+    }
+    else {
+      rv = stmt->BindStringByName(
+        NS_LITERAL_CSTRING("page_title"),
+        StringHead(mData->title, TITLE_LENGTH_MAX)
+      );
+    }
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), mData->uri);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    nsCOMPtr<Step> step = new TitleNotifyStep(mData);
+    rv = step->ExecuteAsync(stmt);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<SetTitleData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  SetTitleStep
+, mozIStorageStatementCallback
+)
+
+/**
+ * Step 1: See if there is an existing URI.
+ */
+class StartSetURITitleStep : public Step
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  StartSetURITitleStep(nsAutoPtr<SetTitleData> 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_URL_PAGE_INFO);
+    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 SetTitleStep(mData);
+    rv = step->ExecuteAsync(stmt);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    return NS_OK;
+  }
+
+protected:
+  nsAutoPtr<SetTitleData> mData;
+};
+NS_IMPL_ISUPPORTS1(
+  StartSetURITitleStep
+, Step
+)
+
 } // anonymous namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -981,6 +1128,36 @@ History::UnregisterVisitedCallback(nsIURI* aURI,
   return NS_OK;
 }
 
+NS_IMETHODIMP
+History::SetURITitle(nsIURI* aURI, const nsString& aTitle)
+{
+  NS_PRECONDITION(aURI, "Must pass a non-null URI!");
+
+  nsNavHistory* history = nsNavHistory::GetHistoryService();
+  PRBool canAdd;
+  nsresult rv = history->CanAddURI(aURI, &canAdd);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (!canAdd) {
+    return NS_OK;
+  }
+
+  nsAutoPtr<SetTitleData> data(new SetTitleData());
+  data->uri = aURI;
+
+  if (aTitle.IsEmpty()) {
+    data->title.SetIsVoid(PR_TRUE);
+  }
+  else {
+    data->title.Assign(aTitle);
+  }
+
+  nsCOMPtr<Step> task(new StartSetURITitleStep(data));
+  AppendTask(task);
+
+  return NS_OK;
+}
+ 
+ 
 ////////////////////////////////////////////////////////////////////////////////
 //// nsISupports
 

toolkit/components/places/src/History.h

@@ -52,7 +52,7 @@ namespace mozilla {
 namespace places {
 
 #define NS_HISTORYSERVICE_CID \
-  {0x9fc91e65, 0x1475, 0x4353, {0x9b, 0x9a, 0x93, 0xd7, 0x6f, 0x5b, 0xd9, 0xb7}}
+  {0x0937a705, 0x91a6, 0x417a, {0x82, 0x92, 0xb2, 0x2e, 0xb1, 0x0d, 0xa8, 0x6c}}
 
 class History : public IHistory
 {

toolkit/components/places/src/nsNavHistory.cpp

@@ -2173,12 +2173,12 @@ nsNavHistory::GetNewSessionID()
 
 
 void
-nsNavHistory::FireOnVisit(nsIURI* aURI,
-                          PRInt64 aVisitID,
-                          PRTime aTime,
-                          PRInt64 aSessionID,
-                          PRInt64 referringVisitID,
-                          PRInt32 aTransitionType)
+nsNavHistory::NotifyOnVisit(nsIURI* aURI,
+                            PRInt64 aVisitID,
+                            PRTime aTime,
+                            PRInt64 aSessionID,
+                            PRInt64 referringVisitID,
+                            PRInt32 aTransitionType)
 {
   PRUint32 added = 0;
   NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
@@ -2187,6 +2187,13 @@ nsNavHistory::FireOnVisit(nsIURI* aURI,
                            referringVisitID, aTransitionType, &added));
 }
 
+void
+nsNavHistory::NotifyTitleChange(nsIURI* aURI, const nsString& aTitle)
+{
+  NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                   nsINavHistoryObserver, OnTitleChanged(aURI, aTitle));
+}
+
 
 PRInt32
 nsNavHistory::GetDaysOfHistory() {
@@ -7358,7 +7365,6 @@ nsNavHistory::SetPageTitleInternal(nsIURI* aURI, const nsAString& aTitle)
   rv = mDBSetPlaceTitle->Execute();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // observers (have to check first if it's bookmarked)
   NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                    nsINavHistoryObserver, OnTitleChanged(aURI, aTitle));
 

toolkit/components/places/src/nsNavHistory.h

@@ -121,6 +121,8 @@ namespace places {
   , DB_GET_PAGE_VISIT_STATS = 5
   , DB_UPDATE_PAGE_VISIT_STATS = 6
   , DB_ADD_NEW_PAGE = 7
+  , DB_GET_URL_PAGE_INFO = 8
+  , DB_SET_PLACE_TITLE = 9
   };
 
 } // namespace places
@@ -438,6 +440,10 @@ public:
         return mDBUpdatePageVisitStats;
       case DB_ADD_NEW_PAGE:
         return mDBAddNewPage;
+      case DB_GET_URL_PAGE_INFO:
+        return mDBGetURLPageInfo;
+      case DB_SET_PLACE_TITLE:
+        return mDBSetPlaceTitle;
     }
     return nsnull;
   }
@@ -447,13 +453,18 @@ public:
   /**
    * Fires onVisit event to nsINavHistoryService observers
    */
-  void FireOnVisit(nsIURI* aURI,
+  void NotifyOnVisit(nsIURI* aURI,
                    PRInt64 aVisitID,
                    PRTime aTime,
                    PRInt64 aSessionID,
                    PRInt64 referringVisitID,
                    PRInt32 aTransitionType);
 
+  /**
+   * Fires onTitleChanged event to nsINavHistoryService observers
+   */
+  void NotifyTitleChange(nsIURI* aURI, const nsString& title);
+
 private:
   ~nsNavHistory();
 

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

@@ -48,6 +48,7 @@ include $(topsrcdir)/config/rules.mk
 _BROWSER_FILES = \
 	browser_bug399606.js \
 	browser_visituri.js \
+	browser_settitle.js \
 	$(NULL)
 
 # These are files that need to be loaded via the HTTP proxy server
@@ -63,6 +64,8 @@ _HTTP_FILES = \
 	visituri/redirect_twice.sjs \
 	visituri/redirect_once.sjs \
 	visituri/final.html \
+	settitle/title1.html \
+	settitle/title2.html \
 	$(NULL)
 
 libs:: $(_BROWSER_FILES)

toolkit/components/places/tests/browser/browser_settitle.js

@@ -0,0 +1,122 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+gBrowser.selectedTab = gBrowser.addTab();
+
+function finishAndCleanUp()
+{
+  gBrowser.removeCurrentTab();
+  waitForClearHistory(finish());
+}
+
+/**
+ * One-time DOMContentLoaded callback.
+ */
+function load(href, callback)
+{
+  content.location.href = href;
+  gBrowser.addEventListener("load", function() {
+    if (content.location.href == href) {
+      gBrowser.removeEventListener("load", arguments.callee, true);
+      if (callback)
+        callback();
+    }
+  }, true);
+}
+
+var conn = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase).DBConnection;
+
+/**
+ * Gets a single column value from either the places or historyvisits table.
+ */
+function getColumn(table, column, fromColumnName, fromColumnValue)
+{
+  var stmt = conn.createStatement(
+    "SELECT " + column + " FROM " + table + "_temp WHERE " + fromColumnName + "=:val " +
+    "UNION ALL " +
+    "SELECT " + column + " FROM " + table + " WHERE " + fromColumnName + "=:val " +
+    "LIMIT 1");
+  try {
+    stmt.params.val = fromColumnValue;
+    stmt.executeStep();
+    return stmt.row[column];
+  }
+  finally {
+    stmt.finalize();
+  }
+}
+
+/**
+ * Clears history invoking callback when done.
+ */
+function waitForClearHistory(aCallback) {
+  const TOPIC_EXPIRATION_FINISHED = "places-expiration-finished";
+  let observer = {
+    observe: function(aSubject, aTopic, aData) {
+      Services.obs.removeObserver(this, TOPIC_EXPIRATION_FINISHED);
+      aCallback();
+    }
+  };
+  Services.obs.addObserver(observer, TOPIC_EXPIRATION_FINISHED, false);
+
+  let hs = Cc["@mozilla.org/browser/nav-history-service;1"].
+           getService(Ci.nsINavHistoryService);
+  hs.QueryInterface(Ci.nsIBrowserHistory).removeAllPages();
+}
+
+function test()
+{
+  // Make sure titles are correctly saved for a URI with the proper
+  // notifications.
+
+  waitForExplicitFinish();
+
+  // Create and add history observer.
+  var historyObserver = {
+    data: [],
+    onBeginUpdateBatch: function() {},
+    onEndUpdateBatch: function() {},
+    onVisit: function(aURI, aVisitID, aTime, aSessionID, aReferringID,
+                      aTransitionType) {
+    },
+    onTitleChanged: function(aURI, aPageTitle) {
+      this.data.push({ uri: aURI, title: aPageTitle });
+
+      // We only expect one title change.
+      //
+      // Although we are loading two different pages, the first page does not
+      // have a title.  Since the title starts out as empty and then is set
+      // to empty, there is no title change notification.
+
+      PlacesUtils.history.removeObserver(this);
+      confirmResults(this.data);
+    },
+    onBeforeDeleteURI: function() {},
+    onDeleteURI: function() {},
+    onClearHistory: function() {},
+    onPageChanged: function() {},
+    onDeleteVisits: function() {},
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
+  };
+  PlacesUtils.history.addObserver(historyObserver, false);
+
+  load("http://example.com/tests/toolkit/components/places/tests/browser/title1.html", function() {
+    load("http://example.com/tests/toolkit/components/places/tests/browser/title2.html");
+  });
+
+  function confirmResults(data) {
+    is(data[0].uri.spec, "http://example.com/tests/toolkit/components/places/tests/browser/title2.html");
+    is(data[0].title, "Some title");
+
+    data.forEach(function(item) {
+      var title = getColumn("moz_places", "title", "url", data[0].uri.spec);
+      is(title, item.title);
+    });
+
+    finishAndCleanUp();
+  }
+}

toolkit/components/places/tests/browser/settitle/title1.html

@@ -0,0 +1,12 @@
+<!--
+ Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/
+-->
+
+<html>
+  <head>
+  </head>
+  <body>
+    title1.html
+  </body>
+</html>

toolkit/components/places/tests/browser/settitle/title2.html

@@ -0,0 +1,14 @@
+<!--
+ Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/
+-->
+
+<html>
+  <head>
+    <title>Some title</title>
+  </head>
+  <body>
+    title2.html
+  </body>
+</html>
+