Stop calling WritePrivateDataToCacheFileTask() on the UI thread during shutdown

Stop calling WritePrivateDataToCacheFileTask() on the UI thread during shutdown

Let InMemoryURLIndex use SequencedTaskRunner instead of FILE thread to perform file tasks.
Post WritePrivateDataToCacheFileTask() in dtor.

BUG=480822
Committed: https://crrev.com/c33ec5d12a7994ea3d8c03fe8cd9efc6745dc837
Cr-Commit-Position: refs/heads/master@{#327003}

[hashimoto, 2015-04-24 15:10:05]

hashimoto@chromium.org changed reviewers:
+ pkasting@chromium.org

[hashimoto, 2015-04-24 15:10:06]

Peter, could you review this change?

[Peter Kasting, 2015-04-24 23:15:33]

LGTM with question.

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
File chrome/browser/autocomplete/in_memory_url_index.cc (right):

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
chrome/browser/autocomplete/in_memory_url_index.cc:197: if (needs_to_be_cached_
&& GetCacheFilePath(&path)) {
Nit: No {}

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
File chrome/browser/autocomplete/in_memory_url_index.h (right):

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
chrome/browser/autocomplete/in_memory_url_index.h:281:
scoped_refptr<base::SequencedTaskRunner> task_runner_;
Nit: This seems to be inserted somewhat randomly into this list, maybe list it
next to the task trackers below?  I would also add a comment like "Task runner
from the worker pool, used for operations which require disk access."

Also, to be clear, even if this class is destoryed and drops its ref to the task
runner, the tasks we posted will all still execute, right?  Their lifetime isn't
scoped to the task runner, or else someone else will keep a ref to it until they
execute?

[hashimoto, 2015-04-27 04:58:02]

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
File chrome/browser/autocomplete/in_memory_url_index.cc (right):

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
chrome/browser/autocomplete/in_memory_url_index.cc:197: if (needs_to_be_cached_
&& GetCacheFilePath(&path)) {
On 2015/04/24 23:15:33, Peter Kasting wrote:
> Nit: No {}

Done.

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
File chrome/browser/autocomplete/in_memory_url_index.h (right):

https://codereview.chromium.org/1101253002/diff/1/chrome/browser/autocomplete...
chrome/browser/autocomplete/in_memory_url_index.h:281:
scoped_refptr<base::SequencedTaskRunner> task_runner_;
On 2015/04/24 23:15:33, Peter Kasting wrote:
> Nit: This seems to be inserted somewhat randomly into this list, maybe list it
> next to the task trackers below?  I would also add a comment like "Task runner
> from the worker pool, used for operations which require disk access."
Done.
> 
> Also, to be clear, even if this class is destoryed and drops its ref to the
task
> runner, the tasks we posted will all still execute, right?  Their lifetime
isn't
> scoped to the task runner, or else someone else will keep a ref to it until
they
> execute?

SequencedWorkerPoolSequencedTaskRunner's PostTask() implementation is just a
wrapper of SequencedWorkerPool::PostSequencedWorkerTaskWithShutdownBehavior().
(https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/seq...)
The posted task will be executed regardless of whether the task runner is alive
or not.

[hashimoto, 2015-04-27 04:58:15]

The CQ bit was checked by hashimoto@chromium.org

[hashimoto, 2015-04-27 04:58:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1101253002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2015-04-27 04:58:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1101253002/20001

[commit-bot: I haz the power, 2015-04-27 05:11:56]

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

[commit-bot: I haz the power, 2015-04-27 05:11:57]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hashimoto, 2015-04-27 05:15:07]

The CQ bit was checked by hashimoto@chromium.org

[hashimoto, 2015-04-27 05:15:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1101253002/#ps40001
(title: "Fix initialization order")

[commit-bot: I haz the power, 2015-04-27 05:15:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1101253002/40001

[commit-bot: I haz the power, 2015-04-27 06:12:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-27 06:13:05]

Patchset 3 (id:??) landed as
https://crrev.com/c33ec5d12a7994ea3d8c03fe8cd9efc6745dc837
Cr-Commit-Position: refs/heads/master@{#327003}