[Icons NTP] Add path_index() to ParsedFallbackIconPath.

[Icons NTP] Add path_index() to ParsedFallbackIconPath.

Making ParsedFallbackIconPath resemble ParsedFaviconPath, to prepare it
for chrome-search:// URL with <view_id>/<restricted_id> . Details:
- Replacing url() with url_string(). Need to update chrome://large-icon
  endpoint and test to accommodate.
- Adding path_index().

BUG=467712
Committed: https://crrev.com/8b3af2bbf44062156199aeec48beeb92f3e674c2
Cr-Commit-Position: refs/heads/master@{#321615}

[huangs, 2015-03-17 20:00:58]

huangs@chromium.org changed reviewers:
+ jhawkins@google.com

[huangs, 2015-03-17 20:00:58]

PTAL.

[Roger McFarlane (Chromium), 2015-03-17 20:06:48]

rogerm@chromium.org changed reviewers:
+ rogerm@chromium.org

[Roger McFarlane (Chromium), 2015-03-17 20:06:49]

lgtm

[beaudoin, 2015-03-18 00:05:38]

beaudoin@chromium.org changed reviewers:
+ beaudoin@chromium.org

[beaudoin, 2015-03-18 00:05:38]

https://codereview.chromium.org/1017803003/diff/1/chrome/browser/ui/webui/fal...
File chrome/browser/ui/webui/fallback_icon_source.cc (right):

https://codereview.chromium.org/1017803003/diff/1/chrome/browser/ui/webui/fal...
chrome/browser/ui/webui/fallback_icon_source.cc:52: if (!url.is_empty() &&
!url.is_valid()) {
A short comment like:
// Ensure URL is empty or valid.
would help clarify, because the logic here is a bit unexpected, compared to the
more frequent case where an empty URL is invalid, ie: (url.is_empty() ||
!url.is_valid())

https://codereview.chromium.org/1017803003/diff/1/chrome/common/favicon/fallb...
File chrome/common/favicon/fallback_icon_url_parser.cc (right):

https://codereview.chromium.org/1017803003/diff/1/chrome/common/favicon/fallb...
chrome/common/favicon/fallback_icon_url_parser.cc:62: //  
"chrome-search://fallback-icon/48/<most-visited-item-with-given-id>".
Same comment here as the previous CL. I believe this is not really a
url_string...

[huangs, 2015-03-18 18:38:49]

Updated, PTAL.

https://codereview.chromium.org/1017803003/diff/1/chrome/browser/ui/webui/fal...
File chrome/browser/ui/webui/fallback_icon_source.cc (right):

https://codereview.chromium.org/1017803003/diff/1/chrome/browser/ui/webui/fal...
chrome/browser/ui/webui/fallback_icon_source.cc:52: if (!url.is_empty() &&
!url.is_valid()) {
The original reason was that I want to render the blank page icon for empty URL.
 But this is not implemented and the logic look bug like.  So now considering
empty to be invalid.

https://codereview.chromium.org/1017803003/diff/1/chrome/common/favicon/fallb...
File chrome/common/favicon/fallback_icon_url_parser.cc (right):

https://codereview.chromium.org/1017803003/diff/1/chrome/common/favicon/fallb...
chrome/common/favicon/fallback_icon_url_parser.cc:62: //  
"chrome-search://fallback-icon/48/<most-visited-item-with-given-id>".
As before, prefer to defer to crbug.com/468320.

[beaudoin, 2015-03-18 19:03:18]

LGTM

https://codereview.chromium.org/1017803003/diff/1/chrome/browser/ui/webui/fal...
File chrome/browser/ui/webui/fallback_icon_source.cc (right):

https://codereview.chromium.org/1017803003/diff/1/chrome/browser/ui/webui/fal...
chrome/browser/ui/webui/fallback_icon_source.cc:52: if (!url.is_empty() &&
!url.is_valid()) {
On 2015/03/18 18:38:49, huangs wrote:
> The original reason was that I want to render the blank page icon for empty
URL.
>  But this is not implemented and the logic look bug like.  So now considering
> empty to be invalid.

Acknowledged.

https://codereview.chromium.org/1017803003/diff/1/chrome/common/favicon/fallb...
File chrome/common/favicon/fallback_icon_url_parser.cc (right):

https://codereview.chromium.org/1017803003/diff/1/chrome/common/favicon/fallb...
chrome/common/favicon/fallback_icon_url_parser.cc:62: //  
"chrome-search://fallback-icon/48/<most-visited-item-with-given-id>".
On 2015/03/18 18:38:49, huangs wrote:
> As before, prefer to defer to crbug.com/468320.

Acknowledged.

[huangs, 2015-03-18 22:05:12]

Ping jhawkins@, PTAL.  Thanks!

[James Hawkins, 2015-03-20 16:27:58]

jhawkins@chromium.org changed reviewers:
+ jhawkins@chromium.org

[James Hawkins, 2015-03-20 16:27:59]

lgtm

[huangs, 2015-03-20 17:09:11]

Thanks! Commiting.

[huangs, 2015-03-20 17:09:16]

The CQ bit was checked by huangs@chromium.org

[huangs, 2015-03-20 17:09:16]

The patchset sent to the CQ was uploaded after l-g-t-m from rogerm@chromium.org
Link to the patchset: https://codereview.chromium.org/1017803003/#ps20001
(title: "Render default on empty path.")

[commit-bot: I haz the power, 2015-03-20 17:09:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1017803003/20001

[commit-bot: I haz the power, 2015-03-20 17:54:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-20 17:54:51]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[huangs, 2015-03-20 18:03:45]

The CQ bit was checked by huangs@chromium.org

[huangs, 2015-03-20 18:03:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
beaudoin@chromium.org, rogerm@chromium.org, jhawkins@chromium.org
Link to the patchset: https://codereview.chromium.org/1017803003/#ps40001
(title: "Fix unit test.")

[commit-bot: I haz the power, 2015-03-20 18:04:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1017803003/40001

[commit-bot: I haz the power, 2015-03-20 19:39:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-20 19:40:47]

Patchset 3 (id:??) landed as
https://crrev.com/8b3af2bbf44062156199aeec48beeb92f3e674c2
Cr-Commit-Position: refs/heads/master@{#321615}