De-dupe input encodings in TemplateURLs.

De-dupe input encodings in TemplateURLs.

BUG=134695
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=144387

[SteveT, 2012-06-27 01:11:14]

Comment and a nit. lgtm upon fixing the compilation and test issues.

https://chromiumcodereview.appspot.com/10676012/diff/1/chrome/browser/search_...
File chrome/browser/search_engines/template_url_service_sync_unittest.cc
(right):

https://chromiumcodereview.appspot.com/10676012/diff/1/chrome/browser/search_...
chrome/browser/search_engines/template_url_service_sync_unittest.cc:1393:
keyword_change.sync_data().GetSpecifics().search_engine().keyword());
Did you mean input_encodings() instead of keyword() here?

https://chromiumcodereview.appspot.com/10676012/diff/1/chrome/browser/search_...
File chrome/browser/search_engines/template_url_service_unittest.cc (right):

https://chromiumcodereview.appspot.com/10676012/diff/1/chrome/browser/search_...
chrome/browser/search_engines/template_url_service_unittest.cc:1501:
ASSERT_TRUE(loaded_url != NULL);
nit: I find it a bit odd that in the other file, you ASSERT_FALSE(keyword ==
NULL) where as here you ASSERT_TRUE. Do you have a preferred way to check
pointers within assertions? If so, use that consistently.

[SteveT, 2012-06-27 01:45:24]

One more comment after seeing the test results.

https://chromiumcodereview.appspot.com/10676012/diff/1/chrome/browser/search_...
File chrome/browser/search_engines/template_url_service_sync_unittest.cc
(right):

https://chromiumcodereview.appspot.com/10676012/diff/1/chrome/browser/search_...
chrome/browser/search_engines/template_url_service_sync_unittest.cc:1389:
ASSERT_TRUE(processor()->contains_guid("keyword"));
You didn't actually set the guid above?

[Peter Kasting, 2012-06-27 03:35:40]

http://codereview.chromium.org/10676012/diff/1/chrome/browser/search_engines/...
File chrome/browser/search_engines/template_url_service_sync_unittest.cc
(right):

http://codereview.chromium.org/10676012/diff/1/chrome/browser/search_engines/...
chrome/browser/search_engines/template_url_service_sync_unittest.cc:1389:
ASSERT_TRUE(processor()->contains_guid("keyword"));
On 2012/06/27 01:45:24, SteveT wrote:
> You didn't actually set the guid above?

Both these observations are the sort of obvious thing I miss when I can't
actually run my tests locally :P

http://codereview.chromium.org/10676012/diff/1/chrome/browser/search_engines/...
File chrome/browser/search_engines/template_url_service_unittest.cc (right):

http://codereview.chromium.org/10676012/diff/1/chrome/browser/search_engines/...
chrome/browser/search_engines/template_url_service_unittest.cc:1501:
ASSERT_TRUE(loaded_url != NULL);
On 2012/06/27 01:11:14, SteveT wrote:
> nit: I find it a bit odd that in the other file, you ASSERT_FALSE(keyword ==
> NULL) where as here you ASSERT_TRUE. Do you have a preferred way to check
> pointers within assertions? If so, use that consistently.

I think this partly comes from different people writing these bits... in both
files, I copy and pasted existing code.

I'll fix.