Port HistoryService::ScheduleDBTask to CancelableTaskTracker

Port HistoryService::ScheduleDBTask to CancelableTaskTracker

The HistoryService::ScheduleDBTask sends HistoryDBTasks to HistoryBackend.
The tasks are later processed unless canceled. If the task RunOnDBThread
method returns false, the task is later re-executed. This pattern requires
the use of CancelableTaskTracker::NewTrackedTaskId to get a Callback that
can be used to check if the task has been canceled yet.

Port all the client code and unit tests to pass a CancelableTaskTracker.

Since CancelableTaskTracker is not thread-safe UrlIndexPrivateData cannot
create its own CancelableTaskTracker, so it is provided by the containing
InMemoryUrlIndex instance and cleaned up before resetting the private_data_
pointer.

BUG=371818
TBR=atwilson,tburkard,pkasting,jcivelli

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

[sdefresne, 2014-06-25 22:27:20]

Please take a look.

[blundell, 2014-06-26 09:55:39]

LGTM with nits/questions

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

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_backend.cc:2340:
queued_history_db_tasks_.pop_front();
this will cause the task's destructor to run?

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_marshaling.h (left):

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_marshaling.h:10: 
does this file not contain any code now?

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

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.cc:389:
base::ThreadTaskRunnerHandle::Get(),
Could you explain this parameter?

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/url_i...
File chrome/browser/history/url_index_private_data.cc (right):

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/url_i...
chrome/browser/history/url_index_private_data.cc:297: IndexRow(NULL,
history_service, new_row, languages, scheme_whitelist, tracker);
nit: 80 chars?

https://codereview.chromium.org/352913002/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service_typed_url_unittest.cc (right):

https://codereview.chromium.org/352913002/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_typed_url_unittest.cc:159: //
ScheduleDBTask takes ownership of its task argument, so we
this comment can be killed now?

[sdefresne, 2014-06-28 08:14:26]

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

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_backend.cc:2340:
queued_history_db_tasks_.pop_front();
On 2014/06/26 09:55:39, blundell wrote:
> this will cause the task's destructor to run?

Yes, this queued_history_db_tasks is of type std::list<QueuedHistoryDBTask>
(note the absence of pointer).

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
File chrome/browser/history/history_marshaling.h (left):

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_marshaling.h:10: 
On 2014/06/26 09:55:39, blundell wrote:
> does this file not contain any code now?

I'll clean this file in a followup CL.

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

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/histo...
chrome/browser/history/history_service.cc:389:
base::ThreadTaskRunnerHandle::Get(),
On 2014/06/26 09:55:39, blundell wrote:
> Could you explain this parameter?

Done.

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/url_i...
File chrome/browser/history/url_index_private_data.cc (right):

https://codereview.chromium.org/352913002/diff/1/chrome/browser/history/url_i...
chrome/browser/history/url_index_private_data.cc:297: IndexRow(NULL,
history_service, new_row, languages, scheme_whitelist, tracker);
On 2014/06/26 09:55:39, blundell wrote:
> nit: 80 chars?

Done.

https://codereview.chromium.org/352913002/diff/1/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service_typed_url_unittest.cc (right):

https://codereview.chromium.org/352913002/diff/1/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service_typed_url_unittest.cc:159: //
ScheduleDBTask takes ownership of its task argument, so we
On 2014/06/26 09:55:39, blundell wrote:
> this comment can be killed now?

Done.

[sdefresne, 2014-06-28 08:17:02]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2014-06-28 08:17:23]

The CQ bit was unchecked by sdefresne@chromium.org

[sdefresne, 2014-07-02 13:38:29]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-07-02 13:39:34]

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

[sdefresne, 2014-07-02 13:49:31]

The CQ bit was unchecked by sdefresne@chromium.org

[sdefresne, 2014-07-02 13:54:13]

TBR-ing OWNERS of client code affected by the refactoring

TBR=pkasting for //chrome/browser/ui/{search,sync}
TBR=jcivelli for //chrome/test/base
TBR=atwilson for //chrome/browser/sync & //chrome/browser/ui/sync
TBR=tburkard for //chrome/browser/prerender

[sdefresne, 2014-07-02 15:57:40]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-07-02 15:58:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sdefresne@chromium.org/352913002/80001

[sdefresne, 2014-07-02 16:28:40]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-07-02 16:29:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sdefresne@chromium.org/352913002/100001

[Peter Kasting, 2014-07-02 18:10:15]

LGTM

[commit-bot: I haz the power, 2014-07-02 20:29:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-07-02 20:56:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-02 20:56:36]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[sdefresne, 2014-07-03 08:36:27]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2014-07-03 08:38:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sdefresne@chromium.org/352913002/120001

[commit-bot: I haz the power, 2014-07-03 10:29:09]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)

[commit-bot: I haz the power, 2014-07-03 11:41:20]

Change committed as 281233