Misc. small cleanups to minimize TemplateURL refactoring diffs:

Misc. small cleanups to minimize TemplateURL refactoring diffs:

Non-functional changes:

* 0 -> TemplateURLRef::NO_SUGGESTIONS_AVAILABLE
* GURL x = GURL(...) -> GURL x(...)
* Infobar -> InfoBar
* AllSources -> AllBrowserContextsAndSources
* Pass a Profile* to SearchProviderInstallData, which it will need for more purposes once UIThreadSearchTermsData takes a Profile*
* Add/modify comments
* "" -> std::string()
* Other trivial changes to minimize diffs

Functional changes:

* TemplateURLFetcher::RequestDelegate::AddSearchProvider() no longer clears the keyword for a newly-added, explicitly-requested TemplateURL that conflicts with a non-replaceable keyword.  This means the conflicting keyword will appear in the dialog box and the box will indicate a conflict.  This is mostly to avoid giving a TemplateURL an empty keyword, which will later be made illegal.
* TemplateURLFetcher::ScheduleDownload() now expects the provided keyword to be non-NULL for autodetected cases (which was always true already) and ignores the keyword for explicit cases (which only matters to tests, as the actual browser code always passed an empty keyword here anyway).
* TemplateURLService::CanReplaceKeyword() now always sets its outparam.  This prevents any possible use-without-set on the caller side.
* Properly order (expected, actual)
* ASSERT -> EXPECT
* Construct the TemplateURL for TemplateURLTableModel::Add() in that function rather than the caller.  This will make it easier to change TemplateURLs to require a Profile* later.
* Check all fields at the end of WebDatabaseMigrationTest.MigrateVersion27ToCurrent

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

[Peter Kasting, 2012-03-21 23:03:43]

I just lumped these all together because they're mostly super-tiny.

[sky, 2012-03-22 00:29:07]

https://chromiumcodereview.appspot.com/9811022/diff/1/chrome/browser/search_e...
File chrome/browser/search_engines/search_provider_install_data.h (right):

https://chromiumcodereview.appspot.com/9811022/diff/1/chrome/browser/search_e...
chrome/browser/search_engines/search_provider_install_data.h:49:
SearchProviderInstallData(Profile* profile,
I would prefer this to keep this taking a WDS. The reason I say that is that
this class is used on other threads. If you make it take a Profile it's all too
easy to make the profile a member and access from another thread - badness.

You said you're doing this because:
' Pass a Profile* to SearchProviderInstallData, which it will need for more
purposes once UIThreadSearchTermsData takes a Profile*.' Could you make
UIThreadSearchTermsData take the Profile but call this with the WDS?

[Peter Kasting, 2012-03-22 18:54:26]

https://chromiumcodereview.appspot.com/9811022/diff/1/chrome/browser/search_e...
File chrome/browser/search_engines/search_provider_install_data.h (right):

https://chromiumcodereview.appspot.com/9811022/diff/1/chrome/browser/search_e...
chrome/browser/search_engines/search_provider_install_data.h:49:
SearchProviderInstallData(Profile* profile,
On 2012/03/22 00:29:07, sky wrote:
> Could you make
> UIThreadSearchTermsData take the Profile but call this with the WDS?

The constructor here actually will need the Profile for multiple objects.  In
addition to the UIThreadSearchTermsData requiring a Profile to get the correct
Profile's Google URL, the GoogleURLObserver is also going to need to take a
Profile, so that when it's notified that the Google URL has changed, it can get
the correct Profile's new URL.

(I should probably also change the GoogleURLTracker from notifying AllSources to
using a source of |this| or of the relevant Profile.)

Given this, it's not clear to me how to avoid passing in the Profile.

[sky, 2012-03-22 19:21:00]

I would prefer the constructor taking n-thread safe objects, then one non-thread
safe object. But if that's too painful, leave what you have. LGTM

[sky, 2012-03-22 19:39:01]

Peter pointed out my comments don't make sense. Sorry about that, LGTM