[Favicon] Adding chrome://fallback-icon host for Fallback Icon "Explicit Flow"

[Favicon] Adding chrome://fallback-icon host for Fallback Icon "Explicit Flow"

Design: go/chrome-fallback-icons

We're chrome://fallback-icon host, managed by FallbackIconSource, so we can
specify URLs such as
  chrome://fallback-icon/128,#f00,black,0.8,0.1/http://www.google.com
to render a fallback icon. This is useful for testing fallback icons.

This CL is sliced off from https://codereview.chromium.org/835903005/ .

BUG=455063
Committed: https://crrev.com/eca0396a32627c4dd40400e94f8c247d8dd4293a
Cr-Commit-Position: refs/heads/master@{#319333}

[huangs, 2015-03-03 19:00:16]

huangs@chromium.org changed reviewers:
+ estade@chromium.org, jered@chromium.org, sdefresne@chromium.org

[huangs, 2015-03-03 19:00:17]

OWNER reviews:

(generic):
  chrome/common/url_constants.*
  chrome/chrome_browser_ui.gypi

sdefresne@:
  components/favicon_base/fallback_icon_service.cc

jered@:
  chrome/browser/search/instant_service.cc

estade@:
  chrome/browser/ui/webui/fallback_icon_source.cc
  chrome/browser/ui/webui/fallback_icon_source.h
  chrome/browser/ui/webui/ntp/most_visited_handler.cc

The files were reviewed in https://codereview.chromium.org/835903005/ , but I
split the CL.  PTAL.

[Jered, 2015-03-03 19:21:44]

still lgtm

[sdefresne, 2015-03-04 10:11:38]

lgtm for components/favicon_base

[huangs, 2015-03-04 18:11:49]

Ping estade@ for review.  Thanks.

[Evan Stade, 2015-03-05 02:54:42]

lgtm

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/fallback_icon_source.cc (right):

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fallback_icon_source.cc:25: const std::string
kIOSFontFamily = "Helvetica Neue";
just inline the string literal imo

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/fallback_icon_source.h (right):

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fallback_icon_source.h:13: // requests for fallback
icons and the backend that renders these.
"the backend that renders these"? How about:

FallbackIconSource services chrome:// requests for fallback icons.

[huangs, 2015-03-05 19:47:47]

Thanks. Committing!

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/fallback_icon_source.cc (right):

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fallback_icon_source.cc:25: const std::string
kIOSFontFamily = "Helvetica Neue";
On 2015/03/05 02:54:42, Evan Stade wrote:
> just inline the string literal imo

Done.

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/fallback_icon_source.h (right):

https://codereview.chromium.org/973883002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/fallback_icon_source.h:13: // requests for fallback
icons and the backend that renders these.
Done, added "explicit" since there will be an "implicit" path later on.

Please note that the comments were adapted from favicon_source.h.

[huangs, 2015-03-05 19:47:54]

The CQ bit was checked by huangs@chromium.org

[huangs, 2015-03-05 19:47:55]

The patchset sent to the CQ was uploaded after l-g-t-m from jered@chromium.org,
sdefresne@chromium.org, estade@chromium.org
Link to the patchset: https://codereview.chromium.org/973883002/#ps60001 (title:
"Comment fix and string cleanup.")

[commit-bot: I haz the power, 2015-03-05 19:48:43]

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

[commit-bot: I haz the power, 2015-03-05 21:44:00]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-05 21:44:43]

Patchset 4 (id:??) landed as
https://crrev.com/eca0396a32627c4dd40400e94f8c247d8dd4293a
Cr-Commit-Position: refs/heads/master@{#319333}

[huangs, 2015-03-09 18:17:55]

The patch was reverted in https://codereview.chromium.org/988863002/
due to use after free.  Culprit: In ParseFallbackIconPath::parseColor, the
fragment

("#" + color_str).c_str()

creates temp string and sends pointer.  The return value |end| comes from this
guy, which gets deallocated after then line, but used in the next line to see if
string was consumed.

Hurray for ASAN!  Going to create new CL for this.

[huangs, 2015-03-09 18:17:55]

The patch was reverted in https://codereview.chromium.org/988863002/
due to use after free.  Culprit: In ParseFallbackIconPath::parseColor, the
fragment

("#" + color_str).c_str()

creates temp string and sends pointer.  The return value |end| comes from this
guy, which gets deallocated after then line, but used in the next line to see if
string was consumed.

Hurray for ASAN!  Going to create new CL for this.