From 0203e71bdfc2fc899dd3f1c342d9242b4769c143 Mon Sep 17 00:00:00 2001 From: Benjamin Stover Date: Fri, 25 Jun 2010 22:53:41 -0700 Subject: [PATCH] Bug 566738 - Add SetURITitle to IHistory. r=sdwilsh sr=bz --- docshell/base/IHistory.h | 16 +- docshell/base/nsDocShell.cpp | 11 +- toolkit/components/places/src/History.cpp | 179 +++++++++++++++++- toolkit/components/places/src/History.h | 2 +- .../components/places/src/nsNavHistory.cpp | 20 +- toolkit/components/places/src/nsNavHistory.h | 13 +- .../places/tests/browser/Makefile.in | 3 + .../places/tests/browser/browser_settitle.js | 122 ++++++++++++ .../places/tests/browser/settitle/title1.html | 12 ++ .../places/tests/browser/settitle/title2.html | 14 ++ 10 files changed, 378 insertions(+), 14 deletions(-) create mode 100644 toolkit/components/places/tests/browser/browser_settitle.js create mode 100644 toolkit/components/places/tests/browser/settitle/title1.html create mode 100644 toolkit/components/places/tests/browser/settitle/title2.html diff --git a/docshell/base/IHistory.h b/docshell/base/IHistory.h index a5f9d9b39beb..6d65e9bb4ba4 100644 --- a/docshell/base/IHistory.h +++ b/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 diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index cc9c2251d0de..9e3c282fe3fe 100644 --- a/docshell/base/nsDocShell.cpp +++ b/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 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) { diff --git a/toolkit/components/places/src/History.cpp b/toolkit/components/places/src/History.cpp index d156ce15294c..0d7f00adc2d3 100644 --- a/toolkit/components/places/src/History.cpp +++ b/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 uri; + nsString title; +}; + +/** + * Step 3: Notify that title has been updated. + */ +class TitleNotifyStep: public Step +{ +public: + NS_DECL_ISUPPORTS + + TitleNotifyStep(nsAutoPtr aData) + : mData(aData) + { + } + + NS_IMETHOD Callback(mozIStorageResultSet* aResultSet) + { + nsNavHistory* history = nsNavHistory::GetHistoryService(); + history->NotifyTitleChange(mData->uri, mData->title); + + return NS_OK; + } + +protected: + nsAutoPtr mData; +}; +NS_IMPL_ISUPPORTS1( + TitleNotifyStep +, mozIStorageStatementCallback +) + +/** + * Step 2: Set title. + */ +class SetTitleStep : public Step +{ +public: + NS_DECL_ISUPPORTS + + SetTitleStep(nsAutoPtr aData) + : mData(aData) + { + } + + NS_IMETHOD Callback(mozIStorageResultSet* aResultSet) + { + if (!aResultSet) + // URI record was not found. + return NS_OK; + + nsCOMPtr 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 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 = new TitleNotifyStep(mData); + rv = step->ExecuteAsync(stmt); + NS_ENSURE_SUCCESS(rv, rv); + + return NS_OK; + } + +protected: + nsAutoPtr 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 aData) + : mData(aData) + { + } + + NS_IMETHOD Callback(mozIStorageResultSet* aResultSet) + { + nsNavHistory* history = nsNavHistory::GetHistoryService(); + + // Find existing entry in moz_places table, if any. + nsCOMPtr 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 = new SetTitleStep(mData); + rv = step->ExecuteAsync(stmt); + NS_ENSURE_SUCCESS(rv, rv); + + return NS_OK; + } + +protected: + nsAutoPtr 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 data(new SetTitleData()); + data->uri = aURI; + + if (aTitle.IsEmpty()) { + data->title.SetIsVoid(PR_TRUE); + } + else { + data->title.Assign(aTitle); + } + + nsCOMPtr task(new StartSetURITitleStep(data)); + AppendTask(task); + + return NS_OK; +} + + //////////////////////////////////////////////////////////////////////////////// //// nsISupports diff --git a/toolkit/components/places/src/History.h b/toolkit/components/places/src/History.h index 4432befba26d..f927e4dc2a18 100644 --- a/toolkit/components/places/src/History.h +++ b/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 { diff --git a/toolkit/components/places/src/nsNavHistory.cpp b/toolkit/components/places/src/nsNavHistory.cpp index ae1b696ae614..b68cbf5ff86e 100644 --- a/toolkit/components/places/src/nsNavHistory.cpp +++ b/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)); diff --git a/toolkit/components/places/src/nsNavHistory.h b/toolkit/components/places/src/nsNavHistory.h index 5b126c6d6881..f95daf86a557 100644 --- a/toolkit/components/places/src/nsNavHistory.h +++ b/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(); diff --git a/toolkit/components/places/tests/browser/Makefile.in b/toolkit/components/places/tests/browser/Makefile.in index a155bd2b12d2..e102872b80ed 100644 --- a/toolkit/components/places/tests/browser/Makefile.in +++ b/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) diff --git a/toolkit/components/places/tests/browser/browser_settitle.js b/toolkit/components/places/tests/browser/browser_settitle.js new file mode 100644 index 000000000000..bf7da355cccc --- /dev/null +++ b/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(); + } +} diff --git a/toolkit/components/places/tests/browser/settitle/title1.html b/toolkit/components/places/tests/browser/settitle/title1.html new file mode 100644 index 000000000000..3c98d693eca1 --- /dev/null +++ b/toolkit/components/places/tests/browser/settitle/title1.html @@ -0,0 +1,12 @@ + + + + + + + title1.html + + diff --git a/toolkit/components/places/tests/browser/settitle/title2.html b/toolkit/components/places/tests/browser/settitle/title2.html new file mode 100644 index 000000000000..28a6b69b59c6 --- /dev/null +++ b/toolkit/components/places/tests/browser/settitle/title2.html @@ -0,0 +1,14 @@ + + + + + Some title + + + title2.html + + +