Reduce the use of ui::ScaleFactor in webui/favicon

Reduce the use of ui::ScaleFactor in webui/favicon

First step for renaming ui::ScaleFactor to ui::ResourceScaleFactor

BUG=372212
R=pkotwicz@chromium.org, sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271909

[pkotwicz, 2014-05-20 03:31:18]

LGTM with nits

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/search/lo...
File chrome/browser/search/local_ntp_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/search/lo...
chrome/browser/search/local_ntp_source.cc:152: float scale;
Nit: default initialize

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/favicon_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/favicon_source.cc:83:
ui::GetSupportedScaleFactor(parsed.scale_factor);
It is really confusing keeping track what it a float and what is a
ui::ScaleFactor now.

At one point, I did not want any floats to be named |scale_factor|.
Can you promise that when you rename ui::ScaleFactor to ui::ResourceScaleFactor
that you will rename all places which use ui::ResourceScaleFactor to something
unique to avoid confusion?

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/fileicon_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fileicon_source.cc:85:
FileIconSource::IconRequestDetails::IconRequestDetails() {
The constructor should default initialize here

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fileicon_source.cc:107:
->GetRepresentation(scale_factor)
Nit: no new line. (Similar to how the code is structured on line 157)

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fileicon_source.cc:140: float scale_factor;
Nit: default initialize

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/theme_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/theme_source.cc:65: float scale_factor;
Nit: default initialize float

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
File chrome/common/favicon/favicon_url_parser.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
chrome/common/favicon/favicon_url_parser.cc:79: float scale_factor = 1.0f;
Is this change necessary?

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
File chrome/common/favicon/favicon_url_parser_unittest.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
chrome/common/favicon/favicon_url_parser_unittest.cc:62:
EXPECT_EQ(ui::SCALE_FACTOR_100P,
Nit: Remove this check. It checks the functionality of
ui::GetSupportedScaleFactor() which is unrelated.

[oshima, 2014-05-20 18:23:28]

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/search/lo...
File chrome/browser/search/local_ntp_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/search/lo...
chrome/browser/search/local_ntp_source.cc:152: float scale;
On 2014/05/20 03:31:19, pkotwicz wrote:
> Nit: default initialize

Done.

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/favicon_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/favicon_source.cc:83:
ui::GetSupportedScaleFactor(parsed.scale_factor);
On 2014/05/20 03:31:19, pkotwicz wrote:
> It is really confusing keeping track what it a float and what is a
> ui::ScaleFactor now.
> 
> At one point, I did not want any floats to be named |scale_factor|.
> Can you promise that when you rename ui::ScaleFactor to
ui::ResourceScaleFactor
> that you will rename all places which use ui::ResourceScaleFactor to something
> unique to avoid confusion?

These will be renamed either

device_scale_factor (float) or

ui::ResourceScaleFactor/resource_scale_factor

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/fileicon_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fileicon_source.cc:85:
FileIconSource::IconRequestDetails::IconRequestDetails() {
On 2014/05/20 03:31:19, pkotwicz wrote:
> The constructor should default initialize here

Done.

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fileicon_source.cc:107:
->GetRepresentation(scale_factor)
On 2014/05/20 03:31:19, pkotwicz wrote:
> Nit: no new line. (Similar to how the code is structured on line 157)

Done.

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fileicon_source.cc:140: float scale_factor;
On 2014/05/20 03:31:19, pkotwicz wrote:
> Nit: default initialize

Done.

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/theme_source.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/browser/ui/webui/...
chrome/browser/ui/webui/theme_source.cc:65: float scale_factor;
On 2014/05/20 03:31:19, pkotwicz wrote:
> Nit: default initialize float

Done.

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
File chrome/common/favicon/favicon_url_parser.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
chrome/common/favicon/favicon_url_parser.cc:79: float scale_factor = 1.0f;
On 2014/05/20 03:31:19, pkotwicz wrote:
> Is this change necessary?

Done.

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
File chrome/common/favicon/favicon_url_parser_unittest.cc (right):

https://codereview.chromium.org/291733004/diff/50014/chrome/common/favicon/fa...
chrome/common/favicon/favicon_url_parser_unittest.cc:62:
EXPECT_EQ(ui::SCALE_FACTOR_100P,
On 2014/05/20 03:31:19, pkotwicz wrote:
> Nit: Remove this check. It checks the functionality of
> ui::GetSupportedScaleFactor() which is unrelated.

Done.

[oshima, 2014-05-20 18:28:48]

sky -> OWNERS

[oshima, 2014-05-20 20:58:14]

sky -> OWNERS

[sky, 2014-05-20 23:24:23]

LGTM

[oshima, 2014-05-20 23:26:06]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-05-20 23:28:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/oshima@chromium.org/291733004/110001

[commit-bot: I haz the power, 2014-05-21 11:27:32]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[oshima, 2014-05-21 16:05:57]

Committed patchset #3 manually as r271909 (presubmit successful).