ServiceWorker: SWDiskCacheMigrator should just move DiskCache files on Android

ServiceWorker: SWDiskCacheMigrator should just move DiskCache files on Android

ServiceWorkerDiskCacheMigrator is designed to migrate DiskCache files from
the BlockFile backend to the Simple backend, but Android has already used the
Simple backend and the migrator just have to move files into a new directory.

This CL also includes...
- making the migrator more self-contained in order to simplify callsites of
the migrator (ie. ServiceWorkerStorage). The ctor of the migrator receives
diskcache paths and a diskcache thread instead of already initialized
diskcaches.
- removing the dest directory before migration to clean up partial results
of the previous attempt.

BUG=487482
TEST=content_unittests --gtest_filter=ServiceWorker*

Committed: https://crrev.com/ba504e7e2f5b57c93766245ffe6aa2c07188f5b6
Cr-Commit-Position: refs/heads/master@{#334117}

[nhiroki, 2015-06-11 18:20:17]

nhiroki@chromium.org changed reviewers:
+ michaeln@chromium.org

[nhiroki, 2015-06-11 18:25:20]

Hi Michael, can you review this?

As commented at https://codereview.chromium.org/1152543002/#msg49, I factored
out the migrator changes from that CL to reduce a CL complexity. Thanks.

[michaeln, 2015-06-11 22:21:31]

good idea to split this part out

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_disk_cache_migrator.cc
(right):

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_disk_cache_migrator.cc:263:
Complete(base::Move(src_path_, dest_path_) ? SERVICE_WORKER_OK
I don't think we can call DirExists() and Move() on the IO thread?

Maybe do this is start?

 248   PostTaskAndReplyWithResult(
 249       disk_cache_thread_.get(), FROM_HERE,
 250       base::Bind(&base::MigrateForAndroid, src_path_ dest_path_),
 251       base::Bind(&ServiceWorkerDiskCacheMigrator::Complete,
 252                  weak_factory_.GetWeakPtr()));

Where MigrateForAndroid() does it all and returns SW_OK or _FAILED. The linker
should dead strip almost evrerything in here on android.

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
File
content/browser/service_worker/service_worker_disk_cache_migrator_unittest.cc
(right):

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_disk_cache_migrator_unittest.cc:201:
ServiceWorkerStorage* storage() { return context_->storage(); }
is this really needed? i don't see where its used.

[nhiroki, 2015-06-12 01:08:25]

Thank you! Updated.

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_disk_cache_migrator.cc
(right):

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_disk_cache_migrator.cc:263:
Complete(base::Move(src_path_, dest_path_) ? SERVICE_WORKER_OK
On 2015/06/11 22:21:31, michaeln wrote:
> I don't think we can call DirExists() and Move() on the IO thread?
> 
> Maybe do this is start?
> 
>  248   PostTaskAndReplyWithResult(
>  249       disk_cache_thread_.get(), FROM_HERE,
>  250       base::Bind(&base::MigrateForAndroid, src_path_ dest_path_),
>  251       base::Bind(&ServiceWorkerDiskCacheMigrator::Complete,
>  252                  weak_factory_.GetWeakPtr()));
> 
> Where MigrateForAndroid() does it all and returns SW_OK or _FAILED. The linker
> should dead strip almost evrerything in here on android.

Oh... yes, these have to be called on the diskcache thread. Fixed.

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
File
content/browser/service_worker/service_worker_disk_cache_migrator_unittest.cc
(right):

https://codereview.chromium.org/1181573002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_disk_cache_migrator_unittest.cc:201:
ServiceWorkerStorage* storage() { return context_->storage(); }
On 2015/06/11 22:21:31, michaeln wrote:
> is this really needed? i don't see where its used.

Removed (This will be used in the main CL)

[michaeln, 2015-06-12 01:58:51]

lgtm!

https://codereview.chromium.org/1181573002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_disk_cache_migrator.h
(right):

https://codereview.chromium.org/1181573002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_disk_cache_migrator.h:6: #define
CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_DISK_CACHE_MIGRATOR_H_
doh :)

[nhiroki, 2015-06-12 02:14:22]

Thank you!!!

[nhiroki, 2015-06-12 02:14:33]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2015-06-12 02:14:56]

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

[commit-bot: I haz the power, 2015-06-12 02:47:29]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-06-12 02:48:26]

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