0

Favicon: Use camelCase params instead of snake_case in JS context

Doc:
https://docs.google.com/document/d/1ksvSq1zF-9jYSfGOvNluyoTV0Ud9xl6r9HiAKONqmz8

Context: crrev.com/c/3673461

Bug: 104102
Change-Id: I23cb2168ddf14aeb7915115f4e8ef56171d5652c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3696447
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Commit-Queue: Solomon Kinard <solomonkinard@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013721}
This commit is contained in:
Solomon Kinard
2022-06-13 23:45:53 +00:00
committed by Chromium LUCI CQ
parent 1c7650cd7f
commit 78e45e119c
14 changed files with 86 additions and 88 deletions
chrome
browser
extensions
resources
new_tab_page
ui
test
data
extensions
api_test
favicon
permission_missing
webui
components/favicon_base
ui/webui/resources
cr_components
most_visited
js

@ -22,12 +22,12 @@ TEST(FaviconUtilUnittest, Parse) {
{false, "chrome-extension://id/_favicon"},
{false, "chrome-extension://id/_favicon/"},
{false, "chrome-extension://id/_favicon/?"},
{true, "chrome-extension://id/_favicon?page_url=https://ok.com"},
{true, "chrome-extension://id/_favicon/?page_url=https://ok.com"},
{true, "chrome-extension://id/_favicon/?page_url=https://ok.com&size=16"},
{true, "chrome-extension://id/_favicon?pageUrl=https://ok.com"},
{true, "chrome-extension://id/_favicon/?pageUrl=https://ok.com"},
{true, "chrome-extension://id/_favicon/?pageUrl=https://ok.com&size=16"},
{true,
"chrome-extension://id/_favicon/?page_url=https://"
"ok.com&size=16&scale_factor=1.0x&server_fallback=1"}};
"chrome-extension://id/_favicon/?pageUrl=https://"
"ok.com&size=16&scaleFactor=1.0x&server_fallback=1"}};
for (const auto& test_case : test_cases) {
GURL url(test_case.url);
chrome::ParsedFaviconPath parsed;

@ -215,9 +215,9 @@ export class DiscountConsentCard extends I18nMixin
private getFaviconUrl_(url: string): string {
const faviconUrl = new URL('chrome://favicon2/');
faviconUrl.searchParams.set('size', '20');
faviconUrl.searchParams.set('scale_factor', '1x');
faviconUrl.searchParams.set('show_fallback_monogram', '');
faviconUrl.searchParams.set('page_url', url);
faviconUrl.searchParams.set('scaleFactor', '1x');
faviconUrl.searchParams.set('showFallbackMonogram', '');
faviconUrl.searchParams.set('pageUrl', url);
return faviconUrl.href;
}

@ -182,9 +182,9 @@ export class ChromeCartModuleElement extends I18nMixin
private getFaviconUrl_(url: string): string {
const faviconUrl = new URL('chrome://favicon2/');
faviconUrl.searchParams.set('size', '24');
faviconUrl.searchParams.set('scale_factor', '1x');
faviconUrl.searchParams.set('show_fallback_monogram', '');
faviconUrl.searchParams.set('page_url', url);
faviconUrl.searchParams.set('scaleFactor', '1x');
faviconUrl.searchParams.set('showFallbackMonogram', '');
faviconUrl.searchParams.set('pageUrl', url);
return faviconUrl.href;
}

@ -266,8 +266,8 @@ TEST_F(FaviconSourceTestWithFavicon2Format,
source()->StartDataRequest(
GURL(base::StrCat(
{kDummyPrefix,
"?size=16&scale_factor=1x&page_url=https%3A%2F%2Fwww.google."
"com&allow_google_server_fallback=0"})),
"?size=16&scale_factor=1x&pageUrl=https%3A%2F%2Fwww.google."
"com&allowGoogleServerFallback=0"})),
test_web_contents_getter_, base::BindOnce(&Noop));
}
@ -283,8 +283,8 @@ TEST_F(FaviconSourceTestWithFavicon2Format,
source()->StartDataRequest(
GURL(base::StrCat(
{kDummyPrefix,
"?size=16&scale_factor=1x&page_url=https%3A%2F%2Fwww.google."
"com&allow_google_server_fallback=1"})),
"?size=16&scale_factor=1x&pageUrl=https%3A%2F%2Fwww.google."
"com&allowGoogleServerFallback=1"})),
test_web_contents_getter_, base::BindOnce(&Noop));
}
@ -301,8 +301,8 @@ TEST_F(
source()->StartDataRequest(
GURL(base::StrCat(
{kDummyPrefix,
"?size=16&scale_factor=1x&page_url=https%3A%2F%2Fwww.google."
"com&allow_google_server_fallback=1"})),
"?size=16&scale_factor=1x&pageUrl=https%3A%2F%2Fwww.google."
"com&allowGoogleServerFallback=1"})),
test_web_contents_getter_, base::BindOnce(&Noop));
}

@ -9,7 +9,7 @@ window.onload = function() {
const id = chrome.runtime.id;
const pageUrl =
`http://www.example.com:${port}extensions/favicon/test_file.html`;
const url = `chrome-extension://${id}/_favicon/?page_url=${pageUrl}`;
const url = `chrome-extension://${id}/_favicon/?pageUrl=${pageUrl}`;
fetch(url).then(() => chrome.test.fail()).catch(error => {
chrome.test.assertEq('Failed to fetch', error.message);
chrome.test.succeed();

@ -10,10 +10,10 @@ import {assertEquals} from 'chrome://webui-test/chai_assert.js';
suite('UrlUtilTest', function() {
function getExpectedImageSet(url: string): string {
return '-webkit-image-set(' +
'url("chrome://favicon2/?size=20&scale_factor=1x&page_url=' +
encodeURIComponent(url) + '&allow_google_server_fallback=0") 1x, ' +
'url("chrome://favicon2/?size=20&scale_factor=2x&page_url=' +
encodeURIComponent(url) + '&allow_google_server_fallback=0") 2x)';
'url("chrome://favicon2/?size=20&scaleFactor=1x&pageUrl=' +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") 1x, ' +
'url("chrome://favicon2/?size=20&scaleFactor=2x&pageUrl=' +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") 2x)';
}
test('favicon for normal URL', function() {

@ -12,13 +12,13 @@ suite('IconModuleTest', function() {
function getExpectedImageSet(size: number): string {
const expectedDesktop = '-webkit-image-set(' +
`url("chrome://favicon2/?size=${size}&scale_factor=1x&page_url=` +
encodeURIComponent(url) + '&allow_google_server_fallback=0") 1x, ' +
`url("chrome://favicon2/?size=${size}&scale_factor=2x&page_url=` +
encodeURIComponent(url) + '&allow_google_server_fallback=0") 2x)';
`url("chrome://favicon2/?size=${size}&scaleFactor=1x&pageUrl=` +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") 1x, ' +
`url("chrome://favicon2/?size=${size}&scaleFactor=2x&pageUrl=` +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") 2x)';
const expectedOther = '-webkit-image-set(' +
`url("chrome://favicon2/?size=${size}&scale_factor=1x&page_url=` +
encodeURIComponent(url) + '&allow_google_server_fallback=0") ' +
`url("chrome://favicon2/?size=${size}&scaleFactor=1x&pageUrl=` +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") ' +
window.devicePixelRatio + 'x)';
const isDesktop = isMac || isChromeOS || isWindows || isLinux || isLacros;
@ -33,12 +33,12 @@ suite('IconModuleTest', function() {
test('GetFavicon', function() {
const url = 'http://foo.com/foo.ico';
const expectedDesktop = '-webkit-image-set(' +
'url("chrome://favicon2/?size=16&scale_factor=1x&icon_url=' +
'url("chrome://favicon2/?size=16&scaleFactor=1x&iconUrl=' +
encodeURIComponent('http://foo.com/foo.ico') + '") 1x, ' +
'url("chrome://favicon2/?size=16&scale_factor=2x&icon_url=' +
'url("chrome://favicon2/?size=16&scaleFactor=2x&iconUrl=' +
encodeURIComponent('http://foo.com/foo.ico') + '") 2x)';
const expectedOther = '-webkit-image-set(' +
'url("chrome://favicon2/?size=16&scale_factor=1x&icon_url=' +
'url("chrome://favicon2/?size=16&scaleFactor=1x&iconUrl=' +
encodeURIComponent('http://foo.com/foo.ico') + '") ' +
window.devicePixelRatio + 'x)';

@ -162,8 +162,8 @@ suite('NewTabPageDiscountConsentCartTest', () => {
});
function buildFaviconUrl(merchantUrl: string): string {
return 'chrome://favicon2/?size=20&scale_factor=1x&show_fallback_monogram=&page_url=' +
encodeURIComponent(merchantUrl);
const query = '?size=20&scaleFactor=1x&showFallbackMonogram=&pageUrl=';
return `chrome://favicon2/${query}${encodeURIComponent(merchantUrl)}`;
}
test('Verify favicon is loaded', async () => {

@ -23,10 +23,10 @@ suite('SiteFavicon', function() {
function formExpected(url: string): string {
return '-webkit-image-set(' +
'url("chrome://favicon2/?size=16&scale_factor=1x&page_url=' +
encodeURIComponent(url) + '&allow_google_server_fallback=0") 1x, ' +
'url("chrome://favicon2/?size=16&scale_factor=2x&page_url=' +
encodeURIComponent(url) + '&allow_google_server_fallback=0") 2x)';
'url("chrome://favicon2/?size=16&scaleFactor=1x&pageUrl=' +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") 1x, ' +
'url("chrome://favicon2/?size=16&scaleFactor=2x&pageUrl=' +
encodeURIComponent(url) + '&allowGoogleServerFallback=0") 2x)';
}
test('normal URL', function() {

@ -97,20 +97,18 @@ bool ParseFaviconPathWithFavicon2Format(const std::string& path,
const base::StringPiece key = it.GetKey();
// Note: each of these keys can be used in chrome://favicon2 path. See file
// "favicon_url_parser.h" for a description of what each one does.
if (key == "allow_google_server_fallback" ||
key == "allowGoogleServerFallback") {
if (key == "allowGoogleServerFallback") {
const std::string val = it.GetUnescapedValue();
if (!(val == "0" || val == "1"))
return false;
parsed->allow_favicon_server_fallback = val == "1";
} else if (key == "show_fallback_monogram" ||
key == "showFallbackMonogram") {
} else if (key == "showFallbackMonogram") {
parsed->show_fallback_monogram = true;
} else if (key == "icon_url" || key == "iconUrl") {
} else if (key == "iconUrl") {
parsed->icon_url = it.GetUnescapedValue();
} else if (key == "page_url" || key == "pageUrl") {
} else if (key == "pageUrl") {
parsed->page_url = it.GetUnescapedValue();
} else if ((key == "scale_factor" || key == "scaleFactor") &&
} else if (key == "scaleFactor" &&
!webui::ParseScaleFactor(it.GetUnescapedValue(),
&parsed->device_scale_factor)) {
return false;

@ -47,7 +47,7 @@ struct ParsedFaviconPath {
// Enum describing the two possible url formats: the legacy chrome://favicon
// and chrome://favicon2.
// - chrome://favicon format:
// chrome://favicon/size&scalefactor/iconurl/url
// chrome://favicon/size&scaleFactor/iconUrl/url
// Some parameters are optional as described below. However, the order of the
// parameters is not interchangeable.
//
@ -56,7 +56,7 @@ struct ParsedFaviconPath {
// Specifies the page URL of the requested favicon. If the 'iconurl'
// parameter is specified, the URL refers to the URL of the favicon image
// instead.
// 'size&scalefactor' Optional
// 'size&scaleFactor' Optional
// Values: ['size/aa@bx/']
// Specifies the requested favicon's size in DIP (aa) and the requested
// favicon's scale factor. (b).
@ -64,41 +64,41 @@ struct ParsedFaviconPath {
// If the parameter is unspecified, the requested favicon's size defaults
// to 16 and the requested scale factor defaults to 1x.
// Example: chrome://favicon/size/16@2x/https://www.google.com/
// 'iconurl' Optional
// Values: ['iconurl']
// 'iconUrl' Optional
// Values: ['iconUrl']
// 'iconurl': Specifies that the url parameter refers to the URL of
// the favicon image as opposed to the URL of the page that the favicon is
// on.
// Example: chrome://favicon/iconurl/https://www.google.com/favicon.ico
//
// - chrome://favicon2 format:
// chrome://favicon2/?query_parameters
// chrome://favicon2/?queryParameters
// Standard URL query parameters are used as described below.
//
// URL Parameters:
// 'page_url'
// 'pageUrl'
// URL pointing to the page whose favicon we want.
// 'icon_url'
// URL pointing directly to favicon image associated with |page_url|.
// 'iconUrl'
// URL pointing directly to favicon image associated with `pageUrl`.
// Pointed image will not necessarily have the most appropriate resolution
// to the user's device.
//
// At least one of the two must be provided and non-empty. If both |page_url|
// and |icon_url| are passed, |page_url| will have precedence.
// At least one of the two must be provided and non-empty. If both `pageUrl`
// and `iconUrl` are passed, `pageUrl` will have precedence.
//
// Other parameters:
// 'size' Optional
// Specifies the requested favicon's size in DIP. If unspecified, defaults
// to 16.
// Example: chrome://favicon2/?size=32
// TODO(victorvianna): Refactor to remove scale_factor parameter.
// 'scale_factor' Optional
// TODO(victorvianna): Refactor to remove scaleFactor parameter.
// 'scaleFactor' Optional
// Values: ['SCALEx']
// Specifies the requested favicon's scale factor. If unspecified, defaults
// to 1x.
// Example: chrome://favicon2/?scale_factor=1.2x
// Example: chrome://favicon2/?scaleFactor=1.2x
//
// 'allow_google_server_fallback' Optional
// 'allowGoogleServerFallback' Optional
// Values: ['1', '0']
// Specifies whether we are allowed to fall back to an external server
// request (by page url) in case the icon is not found locally.

@ -119,26 +119,26 @@ TEST_F(FaviconUrlParserTest, LegacyParsingSizeParamAndUrlModifier) {
TEST_F(FaviconUrlParserTest, Favicon2ParsingSizeParam) {
chrome::ParsedFaviconPath parsed;
EXPECT_TRUE(chrome::ParseFaviconPath("?size=32&page_url=https%3A%2F%2Fg.com",
EXPECT_TRUE(chrome::ParseFaviconPath("?size=32&pageUrl=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2,
&parsed));
EXPECT_EQ(32, parsed.size_in_dip);
EXPECT_FALSE(
chrome::ParseFaviconPath("?size=abc&page_url=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_FALSE(chrome::ParseFaviconPath("?size=abc&pageUrl=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2,
&parsed));
}
TEST_F(FaviconUrlParserTest, Favicon2ParsingScaleFactorParam) {
chrome::ParsedFaviconPath parsed;
EXPECT_TRUE(chrome::ParseFaviconPath(
"?scale_factor=2.1x&page_url=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_TRUE(
chrome::ParseFaviconPath("?scaleFactor=2.1x&pageUrl=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_EQ(2.1f, parsed.device_scale_factor);
EXPECT_FALSE(
chrome::ParseFaviconPath("?scale_factor=-1&page_url=https%3A%2F%2Fg.com",
chrome::ParseFaviconPath("?scaleFactor=-1&pageUrl=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
}
@ -146,12 +146,12 @@ TEST_F(FaviconUrlParserTest, Favicon2ParsingUrlParams) {
chrome::ParsedFaviconPath parsed;
EXPECT_TRUE(
chrome::ParseFaviconPath("?icon_url=https%3A%2F%2Fg.com%2Ffavicon.ico",
chrome::ParseFaviconPath("?iconUrl=https%3A%2F%2Fg.com%2Ffavicon.ico",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_EQ(parsed.icon_url, "https://g.com/favicon.ico");
EXPECT_EQ(parsed.page_url, "");
EXPECT_TRUE(chrome::ParseFaviconPath("?page_url=https%3A%2F%2Fg.com",
EXPECT_TRUE(chrome::ParseFaviconPath("?pageUrl=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2,
&parsed));
EXPECT_EQ(parsed.icon_url, "");
@ -162,20 +162,20 @@ TEST_F(FaviconUrlParserTest, Favicon2ParsingAllowFallbackParam) {
chrome::ParsedFaviconPath parsed;
EXPECT_FALSE(chrome::ParseFaviconPath(
"?allow_google_server_fallback=invalid&page_url=https%"
"?allowGoogleServerFallback=invalid&pageUrl=https%"
"3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_TRUE(chrome::ParseFaviconPath(
"?allow_google_server_fallback=0&page_url=https%3A%"
"2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_TRUE(
chrome::ParseFaviconPath("?allowGoogleServerFallback=0&pageUrl=https%3A%"
"2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_FALSE(parsed.allow_favicon_server_fallback);
EXPECT_TRUE(chrome::ParseFaviconPath(
"?allow_google_server_fallback=1&page_url=https%3A%"
"2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_TRUE(
chrome::ParseFaviconPath("?allowGoogleServerFallback=1&pageUrl=https%3A%"
"2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_TRUE(parsed.allow_favicon_server_fallback);
}
@ -183,14 +183,14 @@ TEST_F(FaviconUrlParserTest, Favicon2ParsingShowFallbackMonogram) {
chrome::ParsedFaviconPath parsed;
parsed.show_fallback_monogram = true;
EXPECT_TRUE(chrome::ParseFaviconPath("?page_url=https%3A%2F%2Fg.com",
EXPECT_TRUE(chrome::ParseFaviconPath("?pageUrl=https%3A%2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2,
&parsed));
EXPECT_FALSE(parsed.show_fallback_monogram);
parsed.show_fallback_monogram = false;
EXPECT_TRUE(
chrome::ParseFaviconPath("?show_fallback_monogram&page_url=https%3A%"
chrome::ParseFaviconPath("?showFallbackMonogram&pageUrl=https%3A%"
"2F%2Fg.com",
chrome::FaviconUrlFormat::kFavicon2, &parsed));
EXPECT_TRUE(parsed.show_fallback_monogram);

@ -548,9 +548,9 @@ export class MostVisitedElement extends MostVisitedElementBase {
private getFaviconUrl_(url: Url): string {
const faviconUrl = new URL('chrome://favicon2/');
faviconUrl.searchParams.set('size', '24');
faviconUrl.searchParams.set('scale_factor', '1x');
faviconUrl.searchParams.set('show_fallback_monogram', '');
faviconUrl.searchParams.set('page_url', url.url);
faviconUrl.searchParams.set('scaleFactor', '1x');
faviconUrl.searchParams.set('showFallbackMonogram', '');
faviconUrl.searchParams.set('pageUrl', url.url);
return faviconUrl.href;
}

@ -107,7 +107,7 @@ export function getImage(path) {
function getBaseFaviconUrl() {
const faviconUrl = new URL('chrome://favicon2/');
faviconUrl.searchParams.set('size', '16');
faviconUrl.searchParams.set('scale_factor', 'SCALEFACTORx');
faviconUrl.searchParams.set('scaleFactor', 'SCALEFACTORx');
return faviconUrl;
}
@ -119,7 +119,7 @@ function getBaseFaviconUrl() {
*/
export function getFavicon(url) {
const faviconUrl = getBaseFaviconUrl();
faviconUrl.searchParams.set('icon_url', url);
faviconUrl.searchParams.set('iconUrl', url);
return getImageSet(faviconUrl.toString());
}
@ -142,13 +142,13 @@ export function getFaviconForPageURL(
// chrome://favicon2 format in components/favicon_base/favicon_url_parser.h.
const faviconUrl = getBaseFaviconUrl();
faviconUrl.searchParams.set('size', size);
faviconUrl.searchParams.set('page_url', url);
// TODO(dbeam): use the presence of 'allow_google_server_fallback' to
faviconUrl.searchParams.set('pageUrl', url);
// TODO(dbeam): use the presence of 'allowGoogleServerFallback' to
// indicate true, otherwise false.
const fallback = isSyncedUrlForHistoryUi ? '1' : '0';
faviconUrl.searchParams.set('allow_google_server_fallback', fallback);
faviconUrl.searchParams.set('allowGoogleServerFallback', fallback);
if (isSyncedUrlForHistoryUi) {
faviconUrl.searchParams.set('icon_url', remoteIconUrlForUma);
faviconUrl.searchParams.set('iconUrl', remoteIconUrlForUma);
}
return getImageSet(faviconUrl.toString());