We are currently prototyping some potential NTP experiences for Chrome (chrome://flags#enable-chrome-home) on Android that may change how we show doodles/logos.  So, it might be prudent to wait a bit of time to see how the dust settles, but let's figure out what all would need to be changed to maybe do this in parallel.

When we originally designed and built the logo service fetching logic, we were thinking about this exact use case, so I would like to see a path where this is supported sooner than later.

I think in the logo_service code, we'd need to add support to distinguish a normal logo from a doodle to allow more flexibility in our UI code (maybe we are more aggressive about showing doodles vs regular logos).

Also, the format is currently very specific to the google doodles.  We'd need to see if we can generalize that or if each provider needs to implement their equivalent of https://cs.chromium.org/chromium/src/components/search_provider_logos/google_logo_api.h

I've added some folk from product and UX to add any additional points (as well as the resident NTP expert).

My overall idea is to have an URL endpoint (for example, hardcoded in prepopulated_engines.json, like ntp url is now) that can be queried to retrieve logo description with urls to the actual image data or maybe with image data encoded.

>We are currently prototyping some potential NTP experiences for Chrome (chrome://flags#enable-chrome-home) on Android that may change how we show doodles/logos.  So, it might be prudent to wait a bit of time to see how the dust settles, but let's figure out what all would need to be changed to maybe do this in parallel.

I think that the work is still worth doing unless there will be no logo at all in the new NTP. UI-related code may change over time as well as the logo format (e.g. going from full ~200*60 dp that is used now to small favicon-like logos) but the fetching logic should not be affected too much. The logo description format may have to be updated though.

>I think in the logo_service code, we'd need to add support to distinguish a normal logo from a doodle to allow more flexibility in our UI code (maybe we are more aggressive about showing doodles vs regular logos).

I don't have a particular opinion about this. Could you please give some example what this support may be used for? Currently UI code just displays what it gets and falls back to built-in Google's logo if the logo service explicitly gives it "nothing". In a prototype that I hacked to study feasibility of this idea I'm just always pushing a downloaded default logo into the UI. Ability to distinguish logos from doodles isn't hard to add anyway.

What I think is the default logo and doodle should be handled differently under the hood, at least in terms of caching and expiration. IMO if the default logo was downloaded once it should not be discarded unless a new logo is available or search engine is changed.

>Also, the format is currently very specific to the google doodles.  We'd need to see if we can generalize that or if each provider needs to implement their equivalent of https://cs.chromium.org/chromium/src/components/search_provider_logos/google_logo_api.h

Well, its not like there is a line of search providers trying to add their logos :) I'd personally prefer generic approach because it is easier to scale if other parties decide to catch up. It shouldn't be too hard to move to provider specific APIs if the need of this arise. I think that existing google_logo_api isn't well suited for regular logos though - it has lots of stuff specific for doodles only, it requires image data to be embedded into json response. Embedded image has to be "one-size-fits-all" while it may be better to provide several different sizes of the logo to avoid up- or downscaling. So some replacement has to be written.

I'm currently concerned about is the recently introduced //components/doodle ( https://crbug.com/690467 ). It seems that search_provider_logos component is now deprecated and planned for removal. This affects implementation choices because each component has different parsing logic and different caching strategy. I see several options:
  - repurpose the logo_service/search_provider_logos for regular logos and keep the doodle component for Google's doodles.
  - build regular logo downloading/caching logic into the doodle component (which I don't like because of the component's name and in particular because its data types are tied to the specific Google's format).
There is of course an option to build something from scratch and hide it behind existing logo_bridge.

+Marc for the doodle component

Indeed, the doodle component was meant as a replacement for search_provider_logos, which really is just as Google-specific. Despite the "doodle" name, it shouldn't be too hard to extend it to support third party logos. I definitely wouldn't want to keep both components around, since they implement essentially the same logic.
However, components/doodle isn't launched yet, and it's not clear if it'll make it in M61. Let's get together and see what the best path forward is here.

OK, I have a CL up that adds a logo_url field to the predefined list of search engines: https://chromium-review.googlesource.com/c/539417.

Mikhail, do you know a logo URL that we can add there for Yandex? Thanks :)

> OK, I have a CL up that adds a logo_url field to the predefined list of search engines: https://chromium-review.googlesource.com/c/539417.

Since this is a new field, could we enforce HTTPS for all new entries?

That seems reasonable… were you thinking of enforcing it in code, or would the review process for search_engines.json be sufficient?

How about a comment in the JSON and a unit test of the JSON to avoid runtime overhead?

Larger scope: Would it be possible to set a content security policy for the NTP to only allow Chrome resources and https://* ?

The NTP on Android is a native UI, not a web page, so CSP isn't an option. Comment in the json + unit test sounds good to me.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41c477c1f50d312865c33b4f4d71fa660520300d

commit 41c477c1f50d312865c33b4f4d71fa660520300d
Author: Bernhard Bauer <bauerb@chromium.org>
Date: Thu Jun 22 16:39:35 2017

Add support for a `logo_url` field in the list of prepopulated search engines.

BUG= 725435 

Change-Id: I65244403904f4b32bb369fda33f0bf5d13858782
Reviewed-on: https://chromium-review.googlesource.com/539417
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481558}
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/android/java/src/org/chromium/chrome/browser/ntp/ChromeHomeNewTabPage.java
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoDelegateImpl.java
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/android/java/src/org/chromium/chrome/browser/ntp/LogoView.java
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/android/java/src/org/chromium/chrome/browser/search_engines/TemplateUrlService.java
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/browser/android/logo_service.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/browser/search_engines/template_url_service_android.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/chrome/browser/search_engines/template_url_service_android.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/default_search_manager.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/default_search_manager.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/prepopulated_engines.json
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/prepopulated_engines_schema.json
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/template_url.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/template_url_data.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/template_url_data.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/template_url_data_util.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_engines/template_url_prepopulate_data_unittest.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/BUILD.gn
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/OWNERS
[add] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/fixed_logo_api.cc
[add] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/fixed_logo_api.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/google_logo_api.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/google_logo_api.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/logo_tracker.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/logo_tracker.h
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/components/search_provider_logos/logo_tracker_unittest.cc
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/ios/chrome/browser/google/google_logo_service.mm
[modify] https://crrev.com/41c477c1f50d312865c33b4f4d71fa660520300d/tools/json_to_struct/json_to_struct.gni

>Mikhail, do you know a logo URL that we can add there for Yandex?

@bauerb, please find the logo at https://storage.ape.yandex.net/get/browser/Doodles/yandex/drawable-xxhdpi/yandex.png I'm sorry for a delay.

Thank you! I'll have a CL up shortly. Attached is a screenshot of what this looks like on a Nexus 5X.

Deleted: device-2017-06-29-165935.png 195 KB

	device-2017-06-29-165935.png 195 KB View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef23a3a329bc08acb698f1295fb050371be08b23

commit ef23a3a329bc08acb698f1295fb050371be08b23
Author: Bernhard Bauer <bauerb@chromium.org>
Date: Thu Jun 29 18:16:35 2017

Add static logo URL for Yandex on Android.

Screenshot:  https://crbug.com/725435#c18 

Bug:  725435 
Change-Id: Icb552139eea0caa82b3708f37f84170cc7e5ae78
Reviewed-on: https://chromium-review.googlesource.com/556031
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483426}
[modify] https://crrev.com/ef23a3a329bc08acb698f1295fb050371be08b23/components/search_engines/prepopulated_engines.json

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7fcbab0795f0c90186482834de9ca68f951e4be

commit a7fcbab0795f0c90186482834de9ca68f951e4be
Author: Bernhard Bauer <bauerb@chromium.org>
Date: Thu Jul 06 10:34:18 2017

Add --search-provider-logo-url command line flag on Android.

Bug:  725435 
Change-Id: I2bb70879d365f7c9b90bee62aa3c8d2e20dd6cb3
Reviewed-on: https://chromium-review.googlesource.com/558925
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484532}
[modify] https://crrev.com/a7fcbab0795f0c90186482834de9ca68f951e4be/chrome/browser/android/logo_service.cc
[modify] https://crrev.com/a7fcbab0795f0c90186482834de9ca68f951e4be/chrome/browser/search_engines/template_url_service_android.cc
[modify] https://crrev.com/a7fcbab0795f0c90186482834de9ca68f951e4be/chrome/common/chrome_switches.cc
[modify] https://crrev.com/a7fcbab0795f0c90186482834de9ca68f951e4be/chrome/common/chrome_switches.h

Hi, any updates here? Is this fixed?

Yeah, fixed and in launch review. (Sorry, I'm horrible about marking bugs as fixed.)