Bug 977177 - Add size ref fragment to icon protocols. r=adw

Both page-icon: and moz-anno:favicon: should support a simple #size=NNN ref
fragment to allow consumers to request a specific sized icon (if available).
The service will do its best to satisfy the request, but it's not guaranteed.
If no size hint is found, it will default to the biggest icon payload available.

It's currently not possible to test the fragment on moz-anno:favicon: since that
requires imagelib support to extract payloads from ico files. Thus a test will
be added at a later time.

MozReview-Commit-ID: G221MKY7rfs

--HG--
extra : rebase_source : 029a6fcaa9dadda24f8d3ebc9b36c1a0ec6314c6
This commit is contained in:
Marco Bonardo 2017-03-15 16:08:28 +01:00
parent 19cfdf37f2
commit 9683655025
9 changed files with 157 additions and 24 deletions

View File

@ -92,7 +92,8 @@ PageIconProtocolHandler.prototype = {
channel.contentStream = pipe.inputStream;
channel.loadInfo = loadInfo;
let pageURI = NetUtil.newURI(uri.path);
let pageURI = NetUtil.newURI(uri.cloneIgnoringRef().path);
let preferredSize = PlacesUtils.favicons.preferredSizeFromURI(uri);
PlacesUtils.favicons.getFaviconDataForPage(pageURI, (iconURI, len, data, mimeType) => {
channel.contentType = mimeType;
if (len == 0) {
@ -104,7 +105,7 @@ PageIconProtocolHandler.prototype = {
streamDefaultFavicon(uri, loadInfo, pipe.outputStream);
}
}
});
}, preferredSize);
return channel;
} catch (ex) {

View File

@ -1671,6 +1671,23 @@ this.PlacesUtils = {
return deferred.promise;
},
/**
* Returns the passed URL with a #size ref for the specified size and
* devicePixelRatio.
*
* @param window
* The window where the icon will appear.
* @param href
* The string href we should add the ref to.
* @param size
* The target image size
* @return The URL with the fragment at the end, in the same formar as input.
*/
urlWithSizeRef(window, href, size) {
return href + (href.includes("#") ? "&" : "#") +
"size=" + (Math.round(size) * window.devicePixelRatio);
},
/**
* Get the unique id for an item (a bookmark, a folder or a separator) given
* its item id.

View File

@ -73,12 +73,16 @@ namespace {
class faviconAsyncLoader : public AsyncStatementCallback
{
public:
faviconAsyncLoader(nsIChannel *aChannel, nsIStreamListener *aListener)
faviconAsyncLoader(nsIChannel *aChannel, nsIStreamListener *aListener,
uint16_t aPreferredSize)
: mChannel(aChannel)
, mListener(aListener)
, mPreferredSize(aPreferredSize)
{
MOZ_ASSERT(aChannel, "Not providing a channel will result in crashes!");
MOZ_ASSERT(aListener, "Not providing a stream listener will result in crashes!");
MOZ_ASSERT(aChannel, "Not providing a channel!");
// Set the default content type.
Unused << mChannel->SetContentType(NS_LITERAL_CSTRING(PNG_MIME_TYPE));
}
@ -90,16 +94,16 @@ public:
{
nsCOMPtr<mozIStorageRow> row;
while (NS_SUCCEEDED(aResultSet->GetNextRow(getter_AddRefs(row))) && row) {
// TODO: For now just return the biggest icon, that is the first one.
// Later this should allow to return a specific size.
if (!mData.IsEmpty()) {
return NS_OK;
}
int32_t width;
nsresult rv = row->GetInt32(1, &width);
NS_ENSURE_SUCCESS(rv, rv);
// Check if we already found an image >= than the preferred size,
// otherwise keep examining the next results.
if (width < mPreferredSize && !mData.IsEmpty()) {
return NS_OK;
}
// Eventually override the default mimeType for svg.
if (width == UINT16_MAX) {
rv = mChannel->SetContentType(NS_LITERAL_CSTRING(SVG_MIME_TYPE));
@ -165,6 +169,7 @@ private:
nsCOMPtr<nsIChannel> mChannel;
nsCOMPtr<nsIStreamListener> mListener;
nsCString mData;
uint16_t mPreferredSize;
};
} // namespace
@ -315,15 +320,23 @@ nsAnnoProtocolHandler::NewFaviconChannel(nsIURI *aURI, nsIURI *aAnnotationURI,
};
// Now we go ahead and get our data asynchronously for the favicon.
nsCOMPtr<mozIStorageStatementCallback> callback =
new faviconAsyncLoader(channel, listener);
// Ignore the ref part of the URI before querying the database because
// we may have added a size fragment for rendering purposes.
nsFaviconService* faviconService = nsFaviconService::GetFaviconService();
nsAutoCString faviconSpec;
nsresult rv = annotationURI->GetSpecIgnoringRef(faviconSpec);
// Any failures fallback to the default icon channel.
if (!callback || !faviconService)
if (NS_FAILED(rv) || !faviconService)
return fallback();
nsresult rv = faviconService->GetFaviconDataAsync(annotationURI, callback);
uint16_t preferredSize = UINT16_MAX;
MOZ_ALWAYS_SUCCEEDS(faviconService->PreferredSizeFromURI(annotationURI, &preferredSize));
nsCOMPtr<mozIStorageStatementCallback> callback =
new faviconAsyncLoader(channel, listener, preferredSize);
if (!callback)
return fallback();
rv = faviconService->GetFaviconDataAsync(faviconSpec, callback);
if (NS_FAILED(rv))
return fallback();

View File

@ -718,23 +718,20 @@ nsFaviconService::OptimizeIconSizes(IconData& aIcon)
}
nsresult
nsFaviconService::GetFaviconDataAsync(nsIURI* aFaviconURI,
nsFaviconService::GetFaviconDataAsync(const nsCString& aFaviconURI,
mozIStorageStatementCallback *aCallback)
{
MOZ_ASSERT(aCallback, "Doesn't make sense to call this without a callback");
nsCOMPtr<mozIStorageAsyncStatement> stmt = mDB->GetAsyncStatement(
"/*Do not warn (bug no: not worth adding an index */ "
"SELECT data, width FROM moz_icons "
"WHERE fixed_icon_url_hash = hash(fixup_url(:url)) AND icon_url = :url "
"ORDER BY width DESC"
);
NS_ENSURE_STATE(stmt);
// Ignore the ref part of the URI before querying the database because
// we may have added a media fragment for rendering purposes.
nsAutoCString faviconURI;
aFaviconURI->GetSpecIgnoringRef(faviconURI);
nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), faviconURI);
nsresult rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("url"), aFaviconURI);
NS_ENSURE_SUCCESS(rv, rv);
nsCOMPtr<mozIStoragePendingStatement> pendingStatement;
@ -762,6 +759,39 @@ nsFaviconService::ConvertUnsupportedPayloads(mozIStorageConnection* aDBConn)
}
}
NS_IMETHODIMP
nsFaviconService::PreferredSizeFromURI(nsIURI* aURI, uint16_t* _size)
{
*_size = UINT16_MAX;
nsAutoCString ref;
// Check for a ref first.
if (NS_FAILED(aURI->GetRef(ref)) || ref.Length() == 0)
return NS_OK;
// Look for a "size=" fragment.
int32_t start = ref.RFind("size=");
if (start >= 0 && ref.Length() > static_cast<uint32_t>(start) + 5) {
nsDependentCSubstring size;
// This is safe regardless, since Rebind checks start is not over Length().
size.Rebind(ref, start + 5);
// Check if the string contains any non-digit.
auto begin = size.BeginReading(), end = size.EndReading();
for (auto ch = begin; ch < end; ++ch) {
if (*ch < '0' || *ch > '9') {
// Not a digit.
return NS_OK;
}
}
// Convert the string to an integer value.
nsresult rv;
uint16_t val = PromiseFlatCString(size).ToInteger(&rv);
if (NS_SUCCEEDED(rv)) {
*_size = val;
}
}
return NS_OK;
}
////////////////////////////////////////////////////////////////////////////////
//// ExpireFaviconsStatementCallbackNotifier

View File

@ -94,14 +94,14 @@ public:
/**
* Obtains the favicon data asynchronously.
*
* @param aFaviconURI
* The URI representing the favicon we are looking for.
* @param aFaviconSpec
* The spec of the URI representing the favicon we are looking for.
* @param aCallback
* The callback where results or errors will be dispatch to. In the
* returned result, the favicon binary data will be at index 0, and the
* mime type will be at index 1.
*/
nsresult GetFaviconDataAsync(nsIURI* aFaviconURI,
nsresult GetFaviconDataAsync(const nsCString& aFaviconSpec,
mozIStorageStatementCallback* aCallback);
/**

View File

@ -54,6 +54,15 @@ interface nsIFaviconService : nsISupports
*/
void expireAllFavicons();
/**
* Tries to extract the preferred size from an icon uri ref fragment.
*
* @param aURI
* The URI to parse.
* @return The preferred size, or UINT16_MAX if not present.
*/
unsigned short preferredSizeFromURI(in nsIURI aURI);
/**
* Adds a given favicon's URI to the failed favicon cache.
*

View File

@ -0,0 +1,60 @@
/**
* This file tests the size ref on the icons protocols.
*/
const PAGE_URL = "http://icon.mozilla.org/";
const ICON16_URL = "http://mochi.test:8888/tests/toolkit/components/places/tests/browser/favicon-normal16.png";
const ICON32_URL = "http://mochi.test:8888/tests/toolkit/components/places/tests/browser/favicon-normal32.png";
add_task(function* () {
yield PlacesTestUtils.addVisits(PAGE_URL);
// Add 2 differently sized favicons for this page.
let data = readFileData(do_get_file("favicon-normal16.png"));
PlacesUtils.favicons.replaceFaviconData(NetUtil.newURI(ICON16_URL),
data, data.length, "image/png");
yield setFaviconForPage(PAGE_URL, ICON16_URL);
data = readFileData(do_get_file("favicon-normal32.png"));
PlacesUtils.favicons.replaceFaviconData(NetUtil.newURI(ICON32_URL),
data, data.length, "image/png");
yield setFaviconForPage(PAGE_URL, ICON32_URL);
const PAGE_ICON_URL = "page-icon:" + PAGE_URL;
yield compareFavicons(PAGE_ICON_URL,
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Not specifying a ref should return the bigger icon");
// Fake window object.
let win = { devicePixelRatio: 1.0 };
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 16),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON16_URL)),
"Size=16 should return the 16px icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Size=32 should return the 32px icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 33),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Size=33 should return the 32px icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 17),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Size=17 should return the 32px icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 1),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON16_URL)),
"Size=1 should return the 16px icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 0),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Size=0 should return the bigger icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, -1),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Invalid size should return the bigger icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL + "#other§=12", 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Pre-existing refs should be ignored");
win = { devicePixelRatio: 1.1 };
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 16),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Size=16 with HIDPI should return the 32px icon");
yield compareFavicons(PlacesUtils.urlWithSizeRef(win, PAGE_ICON_URL, 32),
PlacesUtils.favicons.getFaviconLinkForIcon(NetUtil.newURI(ICON32_URL)),
"Size=32 with HIDPI should return the 32px icon");
});

View File

@ -21,6 +21,7 @@ support-files =
[test_expireAllFavicons.js]
[test_favicons_conversions.js]
[test_favicons_protocols_ref.js]
[test_getFaviconDataForPage.js]
[test_getFaviconURLForPage.js]
[test_moz-anno_favicon_mime_type.js]

View File

@ -887,7 +887,7 @@ function* compareFavicons(icon1, icon2, msg) {
icon2 = new URL(icon2 instanceof Ci.nsIURI ? icon2.spec : icon2);
function getIconData(icon) {
new Promise((resolve, reject) => {
return new Promise((resolve, reject) => {
NetUtil.asyncFetch({
uri: icon.href, loadUsingSystemPrincipal: true,
contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_IMAGE_FAVICON
@ -901,6 +901,8 @@ function* compareFavicons(icon1, icon2, msg) {
}
let data1 = yield getIconData(icon1);
Assert.ok(data1.length > 0, "Should fetch icon data");
let data2 = yield getIconData(icon2);
Assert.ok(data2.length > 0, "Should fetch icon data");
Assert.deepEqual(data1, data2, msg);
}