Add new search engine logos to template url data....

Add new search engine logos to template url data.  This CL also reorganizes the way logos and search engine type data are stored.  Because the search engine dialog is no longer an experiment running on few locales, move the data and logos into the template_url_prepopulate_data for each search engine.

BUG=42612
TEST=search engine dialog works as expected.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53870

[Miranda Callahan, 2010-07-27 00:27:54]

Thanks, Nico (or Evan, if you feel like reviewing)...

[Nico, 2010-07-27 17:19:07]

LG, the only real comment is about moving the constructor definition

http://codereview.chromium.org/3040022/diff/25001/26001
File chrome/app/theme/theme_resources.grd (right):

http://codereview.chromium.org/3040022/diff/25001/26001#newcode257
chrome/app/theme/theme_resources.grd:257: <include
name="IDR_SEARCH_ENGINE_LOGO_ABCSOK" file="google_chrome/search_abcsok.png"
type="BINDATA" />
i'm not sure if we put references to branded images into the open-source
chromium grd files

…oh wait, looking at IDR_PRODUCT_ICON_32 etc, it seems that this is fine as long
as it's in a <if expr="pp_ifdef('_google_chrome')"> block which it is.

http://codereview.chromium.org/3040022/diff/25001/26003
File chrome/browser/search_engines/template_url.h (right):

http://codereview.chromium.org/3040022/diff/25001/26003#newcode277
chrome/browser/search_engines/template_url.h:277:
search_engine_type_(TemplateURLPrepopulateData::SEARCH_ENGINE_OTHER),
move the definition of the constructor to the cc file, then you don't need to
add an include to this file, which makes erg happy

http://codereview.chromium.org/3040022/diff/25001/26004
File chrome/browser/search_engines/template_url_prepopulate_data.cc (right):

http://codereview.chromium.org/3040022/diff/25001/26004#newcode49
chrome/browser/search_engines/template_url_prepopulate_data.cc:49: const int
logo_id;  // Id for logo image in search engine dialog.
optional nit: wouldn't it be nicer to have a map from |id|->(search_engine_type,
logo_id) instead of adding these fields here? Then there'd be quite a bit less
duplication.

[Miranda Callahan, 2010-07-27 17:41:45]

http://codereview.chromium.org/3040022/diff/25001/26001
File chrome/app/theme/theme_resources.grd (right):

http://codereview.chromium.org/3040022/diff/25001/26001#newcode257
chrome/app/theme/theme_resources.grd:257: <include
name="IDR_SEARCH_ENGINE_LOGO_ABCSOK" file="google_chrome/search_abcsok.png"
type="BINDATA" />
On 2010/07/27 17:19:07, Nico wrote:
> i'm not sure if we put references to branded images into the open-source
> chromium grd files
> 
> …oh wait, looking at IDR_PRODUCT_ICON_32 etc, it seems that this is fine as
long
> as it's in a <if expr="pp_ifdef('_google_chrome')"> block which it is.

:-)

http://codereview.chromium.org/3040022/diff/25001/26003
File chrome/browser/search_engines/template_url.h (right):

http://codereview.chromium.org/3040022/diff/25001/26003#newcode277
chrome/browser/search_engines/template_url.h:277:
search_engine_type_(TemplateURLPrepopulateData::SEARCH_ENGINE_OTHER),
On 2010/07/27 17:19:07, Nico wrote:
> move the definition of the constructor to the cc file, then you don't need to
> add an include to this file, which makes erg happy

Hmm, because we return a SearchEngineType below, and you can't forward declare
an Enum, I think we have to include this file anyway.

http://codereview.chromium.org/3040022/diff/25001/26004
File chrome/browser/search_engines/template_url_prepopulate_data.cc (right):

http://codereview.chromium.org/3040022/diff/25001/26004#newcode49
chrome/browser/search_engines/template_url_prepopulate_data.cc:49: const int
logo_id;  // Id for logo image in search engine dialog.
On 2010/07/27 17:19:07, Nico wrote:
> optional nit: wouldn't it be nicer to have a map from
|id|->(search_engine_type,
> logo_id) instead of adding these fields here? Then there'd be quite a bit less
> duplication.

Hmm.  Unfortunately, it's not entirely a one-to-one mapping.  YahooJP, for
example, has a different logo from regular Yahoo, but the same |id|.  Pogodok
and Pogodak also differ in this way.  Before these fields were added, there was
a map in the way you suggest, but this seems less messy -- esp. if the dialog
evolves, and more logos become country-specific.

[Nico, 2010-07-27 17:43:43]

http://codereview.chromium.org/3040022/diff/25001/26003
File chrome/browser/search_engines/template_url.h (right):

http://codereview.chromium.org/3040022/diff/25001/26003#newcode277
chrome/browser/search_engines/template_url.h:277:
search_engine_type_(TemplateURLPrepopulateData::SEARCH_ENGINE_OTHER),
On 2010/07/27 17:41:45, Miranda Callahan wrote:
> On 2010/07/27 17:19:07, Nico wrote:
> > move the definition of the constructor to the cc file, then you don't need
to
> > add an include to this file, which makes erg happy
> 
> Hmm, because we return a SearchEngineType below, and you can't forward declare
> an Enum, I think we have to include this file anyway.

oh, right

*shakes fist at enums*