Bug 737133 - getFaviconURLForPage and getFaviconDataForPage should invoke nsIFaviconDataCallback even if the favicon is not available. r=mak sr=gavin

--HG--
extra : rebase_source : 70f0139a7b87b4a3f4de785a7c29f3f36e2055a9
This commit is contained in:
Paolo Amadini 2012-04-12 12:27:14 +02:00
parent 36dd9561ce
commit f59a485dfb
15 changed files with 253 additions and 124 deletions

View File

@ -3604,12 +3604,12 @@ function FillHistoryMenu(aParent) {
item.setAttribute("index", j);
if (j != index) {
function FHM_getFaviconURLCallback(aURI) {
let iconURL = PlacesUtils.favicons.getFaviconLinkForIcon(aURI).spec;
item.style.listStyleImage = "url(" + iconURL + ")";
}
PlacesUtils.favicons.getFaviconURLForPage(entry.URI,
FHM_getFaviconURLCallback);
PlacesUtils.favicons.getFaviconURLForPage(entry.URI, function (aURI) {
if (aURI) {
let iconURL = PlacesUtils.favicons.getFaviconLinkForIcon(aURI).spec;
item.style.listStyleImage = "url(" + iconURL + ")";
}
});
}
if (j < index) {

View File

@ -92,28 +92,28 @@ Site.prototype = {
* A callback function that takes a favicon image URL as a parameter.
*/
getFavicon: function Site_getFavicon(aCallback) {
let callbackExecuted = false;
function faviconDataCallback(aURI, aDataLen, aData, aMimeType) {
// We don't need a second callback, so we can ignore it to avoid making
// a second database query for the favicon data.
if (callbackExecuted) {
return;
}
function invokeCallback(aFaviconURI) {
try {
// Use getFaviconLinkForIcon to get image data from the database instead
// of using the favicon URI to fetch image data over the network.
aCallback(gFaviconService.getFaviconLinkForIcon(aURI).spec);
callbackExecuted = true;
aCallback(gFaviconService.getFaviconLinkForIcon(aFaviconURI).spec);
} catch (e) {
Cu.reportError("AboutPermissions: " + e);
}
}
// Try to find favicion for both URIs. Callback will only be called if a
// favicon URI is found. We'll ignore the second callback if it is called,
// so this means we'll always prefer the https favicon.
gFaviconService.getFaviconURLForPage(this.httpsURI, faviconDataCallback);
gFaviconService.getFaviconURLForPage(this.httpURI, faviconDataCallback);
// Try to find favicon for both URIs, but always prefer the https favicon.
gFaviconService.getFaviconURLForPage(this.httpsURI, function (aURI) {
if (aURI) {
invokeCallback(aURI);
} else {
gFaviconService.getFaviconURLForPage(this.httpURI, function (aURI) {
if (aURI) {
invokeCallback(aURI);
}
});
}
}.bind(this));
},
/**

View File

@ -7894,8 +7894,7 @@ namespace
{
// Callback used by CopyFavicon to inform the favicon service that one URI
// (mNewURI) has the same favicon URI (OnFaviconDataAvailable's aFaviconURI) as
// another.
// (mNewURI) has the same favicon URI (OnComplete's aFaviconURI) as another.
class nsCopyFaviconCallback : public nsIFaviconDataCallback
{
public:
@ -7907,9 +7906,14 @@ public:
}
NS_IMETHODIMP
OnFaviconDataAvailable(nsIURI *aFaviconURI, PRUint32 aDataLen,
const PRUint8 *aData, const nsACString &aMimeType)
OnComplete(nsIURI *aFaviconURI, PRUint32 aDataLen,
const PRUint8 *aData, const nsACString &aMimeType)
{
// Continue only if there is an associated favicon.
if (!aFaviconURI) {
return NS_OK;
}
NS_ASSERTION(aDataLen == 0,
"We weren't expecting the callback to deliver data.");
nsCOMPtr<mozIAsyncFavicons> favSvc =

View File

@ -880,13 +880,9 @@ AsyncGetFaviconURLForPage::Run()
nsCAutoString iconSpec;
nsresult rv = FetchIconURL(mDB, mPageSpec, iconSpec);
NS_ENSURE_SUCCESS(rv, rv);
MOZ_ASSERT(NS_SUCCEEDED(rv));
// No icon was found.
if (iconSpec.IsEmpty())
return NS_OK;
// Now notify our callback of the icon spec we retrieved.
// Now notify our callback of the icon spec we retrieved, even if empty.
IconData iconData;
iconData.spec.Assign(iconSpec);
@ -949,11 +945,7 @@ AsyncGetFaviconDataForPage::Run()
nsCAutoString iconSpec;
nsresult rv = FetchIconURL(mDB, mPageSpec, iconSpec);
NS_ENSURE_SUCCESS(rv, rv);
if (!iconSpec.Length()) {
return NS_ERROR_NOT_AVAILABLE;
}
MOZ_ASSERT(NS_SUCCEEDED(rv));
IconData iconData;
iconData.spec.Assign(iconSpec);
@ -961,8 +953,13 @@ AsyncGetFaviconDataForPage::Run()
PageData pageData;
pageData.spec.Assign(mPageSpec);
rv = FetchIconInfo(mDB, iconData);
NS_ENSURE_SUCCESS(rv, rv);
if (!iconSpec.IsEmpty()) {
rv = FetchIconInfo(mDB, iconData);
if (NS_FAILED(rv)) {
iconData.spec.Truncate();
MOZ_NOT_REACHED("Fetching favicon information failed unexpectedly.");
}
}
nsCOMPtr<nsIRunnable> event =
new NotifyIconObservers(iconData, pageData, mCallback);
@ -1091,45 +1088,54 @@ NotifyIconObservers::Run()
"This should be called on the main thread");
nsCOMPtr<nsIURI> iconURI;
nsresult rv = NS_NewURI(getter_AddRefs(iconURI), mIcon.spec);
NS_ENSURE_SUCCESS(rv, rv);
// Notify observers only if something changed.
if (mIcon.status & ICON_STATUS_SAVED ||
mIcon.status & ICON_STATUS_ASSOCIATED) {
nsCOMPtr<nsIURI> pageURI;
rv = NS_NewURI(getter_AddRefs(pageURI), mPage.spec);
NS_ENSURE_SUCCESS(rv, rv);
nsFaviconService* favicons = nsFaviconService::GetFaviconService();
NS_ENSURE_STATE(favicons);
(void)favicons->SendFaviconNotifications(pageURI, iconURI, mPage.guid);
// If the page is bookmarked and the bookmarked url is different from the
// updated one, start a new task to update its icon as well.
if (!mPage.bookmarkedSpec.IsEmpty() &&
!mPage.bookmarkedSpec.Equals(mPage.spec)) {
// Create a new page struct to avoid polluting it with old data.
PageData bookmarkedPage;
bookmarkedPage.spec = mPage.bookmarkedSpec;
// This will be silent, so be sure to not pass in the current callback.
nsCOMPtr<nsIFaviconDataCallback> nullCallback;
nsRefPtr<AsyncAssociateIconToPage> event =
new AsyncAssociateIconToPage(mIcon, bookmarkedPage, nullCallback);
mDB->DispatchToAsyncThread(event);
if (!mIcon.spec.IsEmpty()) {
MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_NewURI(getter_AddRefs(iconURI), mIcon.spec)));
if (iconURI)
{
// Notify observers only if something changed.
if (mIcon.status & ICON_STATUS_SAVED ||
mIcon.status & ICON_STATUS_ASSOCIATED) {
SendGlobalNotifications(iconURI);
}
}
}
if (mCallback) {
(void)mCallback->OnFaviconDataAvailable(iconURI,
mIcon.data.Length(),
TO_INTBUFFER(mIcon.data),
mIcon.mimeType);
(void)mCallback->OnComplete(iconURI, mIcon.data.Length(),
TO_INTBUFFER(mIcon.data), mIcon.mimeType);
}
return NS_OK;
}
void
NotifyIconObservers::SendGlobalNotifications(nsIURI* aIconURI)
{
nsCOMPtr<nsIURI> pageURI;
MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_NewURI(getter_AddRefs(pageURI), mPage.spec)));
if (pageURI) {
nsFaviconService* favicons = nsFaviconService::GetFaviconService();
MOZ_ASSERT(favicons);
if (favicons) {
(void)favicons->SendFaviconNotifications(pageURI, aIconURI, mPage.guid);
}
}
// If the page is bookmarked and the bookmarked url is different from the
// updated one, start a new task to update its icon as well.
if (!mPage.bookmarkedSpec.IsEmpty() &&
!mPage.bookmarkedSpec.Equals(mPage.spec)) {
// Create a new page struct to avoid polluting it with old data.
PageData bookmarkedPage;
bookmarkedPage.spec = mPage.bookmarkedSpec;
// This will be silent, so be sure to not pass in the current callback.
nsCOMPtr<nsIFaviconDataCallback> nullCallback;
nsRefPtr<AsyncAssociateIconToPage> event =
new AsyncAssociateIconToPage(mIcon, bookmarkedPage, nullCallback);
mDB->DispatchToAsyncThread(event);
}
}
} // namespace places
} // namespace mozilla

View File

@ -259,8 +259,8 @@ protected:
};
/**
* Asynchronously tries to get the URL of a page's favicon. If this succeeds,
* notifies the given observer.
* Asynchronously tries to get the URL of a page's favicon, then notifies the
* given observer.
*/
class AsyncGetFaviconURLForPage : public AsyncFaviconHelperBase
{
@ -273,7 +273,7 @@ public:
* @param aPageURI
* URL of the page whose favicon's URL we're fetching
* @param aCallback
* function to be called once the URL is retrieved from the database
* function to be called once finished
*/
static nsresult start(nsIURI* aPageURI,
nsIFaviconDataCallback* aCallback);
@ -284,7 +284,7 @@ public:
* @param aPageSpec
* URL of the page whose favicon's URL we're fetching
* @param aCallback
* function to be called once the URL is retrieved from the database
* function to be called once finished
*/
AsyncGetFaviconURLForPage(const nsACString& aPageSpec,
nsCOMPtr<nsIFaviconDataCallback>& aCallback);
@ -297,8 +297,8 @@ private:
/**
* Asynchronously tries to get the URL and data of a page's favicon.
* If this succeeds, notifies the given observer.
* Asynchronously tries to get the URL and data of a page's favicon, then
* notifies the given observer.
*/
class AsyncGetFaviconDataForPage : public AsyncFaviconHelperBase
{
@ -311,7 +311,7 @@ public:
* @param aPageURI
* URL of the page whose favicon URL and data we're fetching
* @param aCallback
* function to be called once the URL and data is retrieved from the database
* function to be called once finished
*/
static nsresult start(nsIURI* aPageURI,
nsIFaviconDataCallback* aCallback);
@ -322,7 +322,7 @@ public:
* @param aPageSpec
* URL of the page whose favicon URL and data we're fetching
* @param aCallback
* function to be called once the URL is retrieved from the database
* function to be called once finished
*/
AsyncGetFaviconDataForPage(const nsACString& aPageSpec,
nsCOMPtr<nsIFaviconDataCallback>& aCallback);
@ -370,6 +370,16 @@ class NotifyIconObservers : public AsyncFaviconHelperBase
public:
NS_DECL_NSIRUNNABLE
/**
* Constructor.
*
* @param aIcon
* Icon information. Can be empty if no icon is associated to the page.
* @param aPage
* Page to which the icon information applies.
* @param aCallback
* Function to be notified in all cases.
*/
NotifyIconObservers(IconData& aIcon,
PageData& aPage,
nsCOMPtr<nsIFaviconDataCallback>& aCallback);
@ -378,6 +388,8 @@ public:
protected:
IconData mIcon;
PageData mPage;
void SendGlobalNotifications(nsIURI* aIconURI);
};
} // namespace places

View File

@ -160,14 +160,18 @@ interface mozIAsyncFavicons : nsISupports
[optional] in PRTime aExpiration);
/**
* Retrieve the URL of the favicon for the given page.
* Retrieves the favicon URI associated to the given page, if any.
*
* @param aPageURI
* URI of the page whose favicon's URL we're looking up
* URI of the page whose favicon URI we're looking up.
* @param aCallback
* Once we've found the favicon's URL, we invoke this callback. Note
* that the callback's aDataLen will be 0, aData will be null, and
* aMimeType will be empty -- only aURI will be non-zero/null/empty.
* This callback is always invoked to notify the result of the lookup.
* The aURI parameter will be the favicon URI, or null when no favicon
* is associated with the page or an error occurred while fetching it.
*
* @note When the callback is invoked, aDataLen will be always 0, aData will
* be an empty array, and aMimeType will be an empty string, regardless
* of whether a favicon is associated with the page.
*
* @see nsIFaviconDataCallback in nsIFaviconService.idl.
*/
@ -175,13 +179,18 @@ interface mozIAsyncFavicons : nsISupports
in nsIFaviconDataCallback aCallback);
/**
* Retrieve the URL and data of the favicon for the given page.
* Retrieves the favicon URI and data associated to the given page, if any.
*
* @param aPageURI
* URI of the page whose favicon's URL and data we're looking up
* URI of the page whose favicon URI and data we're looking up.
* @param aCallback
* Once we've found the favicon's URL, we invoke this callback with
* the favicon data.
* This callback is always invoked to notify the result of the lookup. The aURI
* parameter will be the favicon URI, or null when no favicon is
* associated with the page or an error occurred while fetching it. If
* aURI is not null, the other parameters may contain the favicon data.
* However, if no favicon data is currently associated with the favicon
* URI, aDataLen will be 0, aData will be an empty array, and aMimeType
* will be an empty string.
*
* @see nsIFaviconDataCallback in nsIFaviconService.idl.
*/

View File

@ -328,14 +328,14 @@ interface nsIFaviconService : nsISupports
readonly attribute nsIURI defaultFavicon;
};
[scriptable, function, uuid(22ebd780-f4ab-11de-8a39-0800200c9a66)]
[scriptable, function, uuid(c85e5c82-b70f-4621-9528-beb2aa47fb44)]
interface nsIFaviconDataCallback : nsISupports
{
/**
* Called when the required favicon's information is available.
*
* This callback will be called only if the operation is successful, otherwise
* you won't get notified.
* It's up to the invoking method to state if the callback is always invoked,
* or called on success only. Check the method documentation to ensure that.
*
* The caller will receive the most information we can gather on the icon,
* but it's not guaranteed that all of them will be set. For some method
@ -344,23 +344,26 @@ interface nsIFaviconDataCallback : nsISupports
* It's up to the caller to check aDataLen > 0 before using any data-related
* information like mime-type or data itself.
*
* @param aURI
* Depending on caller method it could be:
* - a dataURI: has "data:" scheme, with base64 encoded icon data.
* - a faviconURI: the URI of the icon in the favicon service.
* - a faviconLinkURI: has "moz-anno" scheme, used to get data async.
* @param aFaviconURI
* Receives the "favicon URI" (not the "favicon link URI") associated
* to the requested page. This can be null if there is no associated
* favicon URI, or the callback is notifying a failure.
* @param aDataLen
* Size of the icon data in bytes. Notice that a value of 0 does not
* necessarily mean that we don't have an icon.
* @param aData
* Icon data, null if aDataLen is 0.
* Icon data, or an empty array if aDataLen is 0.
* @param aMimeType
* Mime type of the icon, null if aDataLen is 0.
* Mime type of the icon, or an empty string if aDataLen is 0.
*
* @note If you want to open a network channel to access the favicon, it's
* recommended that you call the getFaviconLinkForIcon method to convert
* the "favicon URI" into a "favicon link URI".
*/
void onFaviconDataAvailable(in nsIURI aURI,
in unsigned long aDataLen,
[const,array,size_is(aDataLen)] in octet aData,
in AUTF8String aMimeType);
void onComplete(in nsIURI aFaviconURI,
in unsigned long aDataLen,
[const,array,size_is(aDataLen)] in octet aData,
in AUTF8String aMimeType);
};
%{C++

View File

@ -103,7 +103,7 @@ function waitForFaviconChanged(aExpectedPageURI, aExpectedFaviconURI,
function checkFaviconDataForPage(aPageURI, aExpectedMimeType, aExpectedData,
aCallback) {
PlacesUtils.favicons.getFaviconDataForPage(aPageURI,
function CFDFP_onFaviconDataAvailable(aURI, aDataLen, aData, aMimeType) {
function (aURI, aDataLen, aData, aMimeType) {
do_check_eq(aExpectedMimeType, aMimeType);
do_check_true(compareArrays(aExpectedData, aData));
do_check_guid_for_uri(aPageURI);
@ -120,21 +120,9 @@ function checkFaviconDataForPage(aPageURI, aExpectedMimeType, aExpectedData,
* This function is called after the check finished.
*/
function checkFaviconMissingForPage(aPageURI, aCallback) {
// Ask for the favicon associated with the page, expecting not to be notified.
let notificationReceived = false;
PlacesUtils.favicons.getFaviconURLForPage(aPageURI,
function CFMFP_onFaviconDataAvailable() {
notificationReceived = true;
});
// We must wait for the asynchronous database thread to finish the operation,
// and then for the main thread to process any pending notifications that came
// from the asynchronous thread, before we can be sure that
// onFaviconDataAvailable was not invoked in the meantime.
waitForAsyncUpdates(function CFMFP_asyncUpdates() {
do_execute_soon(function CFMFP_soon() {
do_check_false(notificationReceived);
function (aURI, aDataLen, aData, aMimeType) {
do_check_true(aURI === null);
aCallback();
});
});
}

View File

@ -42,8 +42,7 @@ add_test(function test_expireAllFavicons() {
PlacesUtils.bookmarks.DEFAULT_INDEX,
"Test bookmark");
PlacesUtils.favicons.setAndFetchFaviconForPage(
BOOKMARKED_PAGE_URI, SMALLPNG_DATA_URI, true,
function EAF_onFaviconDataAvailable() {
BOOKMARKED_PAGE_URI, SMALLPNG_DATA_URI, true, function () {
// Start expiration only after data has been saved in the database.
PlacesUtils.favicons.expireAllFavicons();
});

View File

@ -78,7 +78,7 @@ add_test(function test_set_and_get_favicon_setup() {
add_test(function test_set_and_get_favicon_getFaviconURLForPage() {
let [icon0] = icons;
PlacesUtils.favicons.getFaviconURLForPage(pages[0],
function GFUFP_onFaviconDataAvailable(aURI, aDataLen, aData, aMimeType) {
function (aURI, aDataLen, aData, aMimeType) {
do_check_true(icon0.uri.equals(aURI));
do_check_eq(aDataLen, 0);
do_check_eq(aData.length, 0);

View File

@ -0,0 +1,57 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* This file tests getFaviconDataForPage.
*/
////////////////////////////////////////////////////////////////////////////////
/// Globals
const FAVICON_URI = NetUtil.newURI(do_get_file("favicon-normal32.png"));
const FAVICON_DATA = readFileData(do_get_file("favicon-normal32.png"));
const FAVICON_MIMETYPE = "image/png";
////////////////////////////////////////////////////////////////////////////////
/// Tests
function run_test()
{
// Check that the favicon loaded correctly before starting the actual tests.
do_check_eq(FAVICON_DATA.length, 344);
run_next_test();
}
add_test(function test_normal()
{
let pageURI = NetUtil.newURI("http://example.com/normal");
addVisits(pageURI, function () {
PlacesUtils.favicons.setAndFetchFaviconForPage(
pageURI, FAVICON_URI, true, function () {
PlacesUtils.favicons.getFaviconDataForPage(pageURI,
function (aURI, aDataLen, aData, aMimeType) {
do_check_true(aURI.equals(FAVICON_URI));
do_check_eq(FAVICON_DATA.length, aDataLen);
do_check_true(compareArrays(FAVICON_DATA, aData));
do_check_eq(FAVICON_MIMETYPE, aMimeType);
run_next_test();
});
});
});
});
add_test(function test_missing()
{
let pageURI = NetUtil.newURI("http://example.com/missing");
PlacesUtils.favicons.getFaviconDataForPage(pageURI,
function (aURI, aDataLen, aData, aMimeType) {
// Check also the expected data types.
do_check_true(aURI === null);
do_check_true(aDataLen === 0);
do_check_true(aData.length === 0);
do_check_true(aMimeType === "");
run_next_test();
});
});

View File

@ -0,0 +1,50 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* This file tests getFaviconURLForPage.
*/
////////////////////////////////////////////////////////////////////////////////
/// Tests
function run_test()
{
run_next_test();
}
add_test(function test_normal()
{
let pageURI = NetUtil.newURI("http://example.com/normal");
addVisits(pageURI, function () {
PlacesUtils.favicons.setAndFetchFaviconForPage(
pageURI, SMALLPNG_DATA_URI, true, function () {
PlacesUtils.favicons.getFaviconURLForPage(pageURI,
function (aURI, aDataLen, aData, aMimeType) {
do_check_true(aURI.equals(SMALLPNG_DATA_URI));
// Check also the expected data types.
do_check_true(aDataLen === 0);
do_check_true(aData.length === 0);
do_check_true(aMimeType === "");
run_next_test();
});
});
});
});
add_test(function test_missing()
{
let pageURI = NetUtil.newURI("http://example.com/missing");
PlacesUtils.favicons.getFaviconURLForPage(pageURI,
function (aURI, aDataLen, aData, aMimeType) {
// Check also the expected data types.
do_check_true(aURI === null);
do_check_true(aDataLen === 0);
do_check_true(aData.length === 0);
do_check_true(aMimeType === "");
run_next_test();
});
});

View File

@ -9,6 +9,8 @@ fail-if = os == "android"
[test_favicons_conversions.js]
# Bug 676989: test fails consistently on Android
fail-if = os == "android"
[test_getFaviconDataForPage.js]
[test_getFaviconURLForPage.js]
[test_moz-anno_favicon_mime_type.js]
[test_query_result_favicon_changed_on_child.js]
[test_replaceFaviconData.js]

View File

@ -1242,9 +1242,8 @@ tests.push({
do_check_eq(as.getPageAnnotation(this._uri2, "anno"), "anno");
do_check_eq(as.getItemAnnotation(this._bookmarkId, "anno"), "anno");
fs.getFaviconURLForPage(this._uri2,
function AC_onFaviconDataAvailable(aURI) {
do_check_true(aURI.equals(SMALLPNG_DATA_URI));
fs.getFaviconURLForPage(this._uri2, function (aFaviconURI) {
do_check_true(aFaviconURI.equals(SMALLPNG_DATA_URI));
aCallback();
});
}

View File

@ -557,10 +557,10 @@ AsyncFaviconDataReady::AsyncFaviconDataReady(nsIURI *aNewURI,
}
NS_IMETHODIMP
AsyncFaviconDataReady::OnFaviconDataAvailable(nsIURI *aFaviconURI,
PRUint32 aDataLen,
const PRUint8 *aData,
const nsACString &aMimeType)
AsyncFaviconDataReady::OnComplete(nsIURI *aFaviconURI,
PRUint32 aDataLen,
const PRUint8 *aData,
const nsACString &aMimeType)
{
if (!aDataLen || !aData) {
return NS_OK;