Remove Profile* from TemplateURL

Remove Profile* from TemplateURL

Remove TemplateURL::profile_.
Remove TemplateURLService::AddAndSetProfile which does nothing other than calling Add().
Merge TemplateURLService::GenerateSearchURLUsingTermsData and GenerateSearchURLUsingTermsData.

BUG=383283
TEST=git cl try
TBR=sky@chromium.org

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

[hashimoto, 2014-06-17 20:48:12]

Peter, could you review this change?

[Peter Kasting, 2014-06-17 21:07:55]

LGTM

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/template_url_service.cc (right):

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_service.cc:618: host)) ?
initial_default_search_provider_.get() : NULL;
Nit: This line has gotten unwieldy.  How about this for this function:

  if (loaded_)
    return provider_map_->GetTemplateURLForHost(host);
  TemplateURL* initial_dsp = initial_default_search_provider_.get();
  if (!initial_dsp)
    return NULL;
  return (GenerateSearchURL(initial_dsp, search_terms_data()).host() == host) ?
      initial_dsp : NULL;

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/template_url_service_unittest.cc (right):

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_service_unittest.cc:215: result =
TemplateURLService::GenerateSearchURL(
Nit: Combine this with the line above

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_service_unittest.cc:555:
TestGenerateSearchURL(search_terms_data);
Nit: If this is now the only caller of this function, can the function body be
inlined here?

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/template_url_unittest.cc (right):

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_unittest.cc:857: TemplateURL
url(data);
Nit: Why not just do this?:

  TemplateURL url(TemplateURLData());

[hashimoto, 2014-06-17 21:58:03]

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/template_url_service.cc (right):

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_service.cc:618: host)) ?
initial_default_search_provider_.get() : NULL;
On 2014/06/17 21:07:54, Peter Kasting wrote:
> Nit: This line has gotten unwieldy.  How about this for this function:
> 
>   if (loaded_)
>     return provider_map_->GetTemplateURLForHost(host);
>   TemplateURL* initial_dsp = initial_default_search_provider_.get();
>   if (!initial_dsp)
>     return NULL;
>   return (GenerateSearchURL(initial_dsp, search_terms_data()).host() == host)
?
>       initial_dsp : NULL;

Makes sense.
Done.

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/template_url_service_unittest.cc (right):

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_service_unittest.cc:215: result =
TemplateURLService::GenerateSearchURL(
On 2014/06/17 21:07:54, Peter Kasting wrote:
> Nit: Combine this with the line above

Done.

Also removed the code below which prints the expected value and the actual
value.
EXPECT_EQ kindly prints these values when the expectation fails.

test_name is still kept as it can be helpful for debugging.

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_service_unittest.cc:555:
TestGenerateSearchURL(search_terms_data);
On 2014/06/17 21:07:54, Peter Kasting wrote:
> Nit: If this is now the only caller of this function, can the function body be
> inlined here?

Done.

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
File chrome/browser/search_engines/template_url_unittest.cc (right):

https://codereview.chromium.org/338363004/diff/20001/chrome/browser/search_en...
chrome/browser/search_engines/template_url_unittest.cc:857: TemplateURL
url(data);
On 2014/06/17 21:07:55, Peter Kasting wrote:
> Nit: Why not just do this?:
> 
>   TemplateURL url(TemplateURLData());

Done.
Please note that the additional parenthesis are needed to avoid compile error.
(with Clang, it says "error: parentheses were disambiguated as a function
declaration [-Werror,-Wvexing-parse]")
Seems this sentence can be parsed as a function declaration by the compiler
without these parenthesis.

[Peter Kasting, 2014-06-17 21:59:35]

On 2014/06/17 21:58:03, hashimoto wrote:
> Please note that the additional parenthesis are needed to avoid compile error.
> (with Clang, it says "error: parentheses were disambiguated as a function
> declaration [-Werror,-Wvexing-parse]")
> Seems this sentence can be parsed as a function declaration by the compiler
> without these parenthesis.

C++'s "Most Vexing Parse" strikes again...

[hashimoto, 2014-06-17 22:03:44]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2014-06-17 22:05:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hashimoto@chromium.org/338363004/60001

[hashimoto, 2014-06-17 22:09:57]

On 2014/06/17 21:59:35, Peter Kasting wrote:
> On 2014/06/17 21:58:03, hashimoto wrote:
> > Please note that the additional parenthesis are needed to avoid compile
error.
> > (with Clang, it says "error: parentheses were disambiguated as a function
> > declaration [-Werror,-Wvexing-parse]")
> > Seems this sentence can be parsed as a function declaration by the compiler
> > without these parenthesis.
> 
> C++'s "Most Vexing Parse" strikes again...

I've imagined I can bitten by this rule only with simple sentences like "T
x();".
C++ is hard :(

[hashimoto, 2014-06-17 23:52:12]

TBRing sky@chromium.org for mechanical changes in
chrome/browser/{extensions/api/settings_overrides,importer,profile_resetter,renderer_context_menu,search,sync/test/integration}

[hashimoto, 2014-06-17 23:52:20]

The CQ bit was unchecked by hashimoto@chromium.org

[hashimoto, 2014-06-17 23:52:20]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2014-06-17 23:53:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hashimoto@chromium.org/338363004/60001

[commit-bot: I haz the power, 2014-06-18 07:13:32]

Change committed as 277965