Change HistoryService::ScheduleAutocomplete interface

Change HistoryService::ScheduleAutocomplete interface
    
    Change HistoryService::ScheduleAutocomplete interface to take
    a base::Callback<void(HistoryBackend*, HistoryDatabase*)> and propagate
    to HistoryBackend::ScheduleAutocomplete and to the client.
    With this change HistoryService and HistoryBackend no longer need to be
    aware of the HistoryURLProvider and HistoryURLProviderParams.
    
    BUG=374728
TEST=unit_tets --gtest_filter=History*

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

[nshaik, 2014-07-08 02:00:18]

Please review.

[Peter Kasting, 2014-07-08 02:02:52]

On 2014/07/08 02:00:18, naiem.shaik wrote:
> Please review.

When you specify multiple reviewers, you need to say what you want each to
review.

This change looks small, so in general you would want one OWNER to review, or
possibly one reviewer + one OWNER.  Sending to 4 people is overkill.

Also, don't put "R=" lines in your description manually.  The bot will add those
based on who actually reviewed the change once this is checked in.

[nshaik, 2014-07-08 05:59:00]

+pkasting for

chrome/browser/autocomplete/history_url_provider.h
chrome/browser/autocomplete/history_url_provider.cc

+blundell for

chrome/browser/history/history_backend.h
chrome/browser/history/history_backend.cc
chrome/browser/history/history_service.h
chrome/browser/history/history_service.cc

[Peter Kasting, 2014-07-08 06:21:07]

I think your CL description is backwards; you're removing a dependency of
HistoryService/HistoryBackend on HistoryURLProvider, not the other way around.

LGTM other than that.

[nshaik, 2014-07-08 07:17:43]

On 2014/07/08 06:21:07, Peter Kasting wrote:
> I think your CL description is backwards; you're removing a dependency of
> HistoryService/HistoryBackend on HistoryURLProvider, not the other way around.
> 
> LGTM other than that.

Thanks for your comment.
After the change HistoryService/HistoryBackend no longer need to be aware of
HistoryURLProvider.
I thought that is what it means, by saying removing dependency of
HistoryURLProvider on HistoryService/HistoryBackend.
I will write it in a less confusing manner.

[blundell, 2014-07-08 09:10:05]

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_backend.cc (right):

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_backend.cc:1544: // ExecuteWithDB should handle
the NULL database case.
This comment is out of date now; instead, the comment on the declaration should
specify that the callback must be able to handle the case where the database is
null.

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_backend.h (right):

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_backend.h:153: // ScheduleAutocomplete() never
frees |provider| (which is globally live).
This comment is out of date now: it should describe when the callback is called.

[nshaik, 2014-07-08 18:53:49]

Please review.

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_backend.cc (right):

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_backend.cc:1544: // ExecuteWithDB should handle
the NULL database case.
On 2014/07/08 09:10:05, blundell wrote:
> This comment is out of date now; instead, the comment on the declaration
should
> specify that the callback must be able to handle the case where the database
is
> null.

Done.

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_backend.h (right):

https://codereview.chromium.org/376703007/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_backend.h:153: // ScheduleAutocomplete() never
frees |provider| (which is globally live).
On 2014/07/08 09:10:05, blundell wrote:
> This comment is out of date now: it should describe when the callback is
called.

Done.

[blundell, 2014-07-09 08:06:29]

LGTM

[nshaik, 2014-07-09 08:24:46]

The CQ bit was checked by naiem.shaik@gmail.com

[commit-bot: I haz the power, 2014-07-09 08:26:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/naiem.shaik@gmail.com/376703007/40001

[commit-bot: I haz the power, 2014-07-09 16:34:59]

Change committed as 282064