Drive: add a method to clear all local cache.

Drive: add a method to clear all local cache.

Mainly, this cl add the two methods to clear cache.
1) GDataCache::ClearAllOnUIThread():
- remove all the files in the cache directory.
- re-create the |metadata_| instance.
2) GDataSystemService::ClearCacheAndReset():
- unmount Drive
- cancel all the running tasks.
- call GDataCache::ClearAllOnUIThread()
- re-mount Drive

BUG=135197
TEST=out/Debug/unit_tests:GData* and out/Debug/browser_tests:GData* passes.

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

[yoshiki, 2012-08-01 20:28:51]

satorux: could you take a look? This CL is one of 4 patches (maybe) to implement
crbug.com/135197.

[yoshiki, 2012-08-01 20:28:51]

satorux: could you take a look? This CL is one of 4 patches (maybe) to implement
crbug.com/135197.

[satorux1, 2012-08-01 20:50:30]

thank you for working on this!

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_cache.h (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache.h:269: void ClearAllOnUIThread(const
SetMountedStateCallback& callback);
The callback type name looks awkward for this function. Maybe rename it to
something more generic, like ChangeCacheStateCallback?

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_cache_unittest.cc (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:729: size_t
CountCache(const std::string& resource_id,
function comment is missing. Maybe CountCacheFiles()?

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:730: const std::string&
md5) {
indentation is off

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:111:
base::Unretained(this)
please use weak_ptr.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:115: void
GDataSystemService::ClearCacheAndReset(
The function name doesn't seem to match with what it does?

Maybe AddBackDrivemountPoint()?

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:116: GDataFileError error,
const FilePath& file_path) {
shouldn't we check |error|?

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_system_service.h (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.h:45: void
ClearCacheAndResetOnUIThread();
function comment is missing.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.h:64: void
ClearCacheAndReset(GDataFileError error, const FilePath& file_path);
function comment is missing.

[yoshiki, 2012-08-01 21:44:59]

Thanks, PTAL?

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_cache.h (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache.h:269: void ClearAllOnUIThread(const
SetMountedStateCallback& callback);
On 2012/08/01 20:50:31, satorux1 wrote:
> The callback type name looks awkward for this function. Maybe rename it to
> something more generic, like ChangeCacheStateCallback?

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_cache_unittest.cc (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:729: size_t
CountCache(const std::string& resource_id,
On 2012/08/01 20:50:31, satorux1 wrote:
> function comment is missing. Maybe CountCacheFiles()?

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_cache_unittest.cc:730: const std::string&
md5) {
On 2012/08/01 20:50:31, satorux1 wrote:
> indentation is off

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:111:
base::Unretained(this)
On 2012/08/01 20:50:31, satorux1 wrote:
> please use weak_ptr.

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:115: void
GDataSystemService::ClearCacheAndReset(
On 2012/08/01 20:50:31, satorux1 wrote:
> The function name doesn't seem to match with what it does?
> 
> Maybe AddBackDrivemountPoint()?

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:116: GDataFileError error,
const FilePath& file_path) {
On 2012/08/01 20:50:31, satorux1 wrote:
> shouldn't we check |error|?

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_system_service.h (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.h:45: void
ClearCacheAndResetOnUIThread();
On 2012/08/01 20:50:31, satorux1 wrote:
> function comment is missing.

Done.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.h:64: void
ClearCacheAndReset(GDataFileError error, const FilePath& file_path);
On 2012/08/01 20:50:31, satorux1 wrote:
> function comment is missing.

Done.

[yoshiki, 2012-08-01 21:44:59]

Thanks, PTAL?

[satorux1, 2012-08-01 21:56:30]

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_cache.cc:242: void
RunSetMountedStateCallback(const ChangeCacheStateCallback& callback,
Please rename this function. please also update the function comment.

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_system_service.cc:43:
weak_ptr_factory_(this) {
ALLOW_THIS_IN_INITIALIZER_LISt

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_system_service.cc:108:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
add a blank like after DCHECK

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_system_service.cc:119:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
please swap the order of DCHECKs, and add a blank line after that.

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
File chrome/browser/chromeos/gdata/gdata_system_service.h (right):

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_system_service.h:48: void
ClearCacheAndResetOnUIThread();
Please rename it to ClearCache(). This should suffice.

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_system_service.h:67: // Used to implement
ClearCacheAndResetOnUIThread().
Please write some more, like:

// Adds back the drive mount point.
// Used to implement ClearCacheAndResetOnUIThread().

https://chromiumcodereview.appspot.com/10823125/diff/12003/chrome/browser/chr...
chrome/browser/chromeos/gdata/gdata_system_service.h:68: void
AddBackDrivemountPoint(GDataFileError error, const FilePath& file_path);
Drivemount -> DriveMount

[yoshiki, 2012-08-01 23:07:58]

PTAL.

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

http://codereview.chromium.org/10823125/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_system_service.cc:116: GDataFileError error,
const FilePath& file_path) {
On 2012/08/01 21:44:59, yoshiki wrote:
> On 2012/08/01 20:50:31, satorux1 wrote:
> > shouldn't we check |error|?
> 
> Done.

I was wrong in the patchset #2 and |error| is not a pointer. |error| is always
GDATA_FILE_OK so far. I added a TODO comment instead of checking.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_cache.cc:242: void
RunSetMountedStateCallback(const ChangeCacheStateCallback& callback,
On 2012/08/01 21:56:31, satorux1 wrote:
> Please rename this function. please also update the function comment.

Done.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.cc:43:
weak_ptr_factory_(this) {
On 2012/08/01 21:56:31, satorux1 wrote:
> ALLOW_THIS_IN_INITIALIZER_LISt

Done.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.cc:108:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
On 2012/08/01 21:56:31, satorux1 wrote:
> add a blank like after DCHECK

Done.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.cc:119:
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
On 2012/08/01 21:56:31, satorux1 wrote:
> please swap the order of DCHECKs, and add a blank line after that.

Done.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_system_service.h (right):

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.h:48: void
ClearCacheAndResetOnUIThread();
On 2012/08/01 21:56:31, satorux1 wrote:
> Please rename it to ClearCache(). This should suffice.

Done.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.h:67: // Used to implement
ClearCacheAndResetOnUIThread().
On 2012/08/01 21:56:31, satorux1 wrote:
> Please write some more, like:
> 
> // Adds back the drive mount point.
> // Used to implement ClearCacheAndResetOnUIThread().

Done.

http://codereview.chromium.org/10823125/diff/12003/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.h:68: void
AddBackDrivemountPoint(GDataFileError error, const FilePath& file_path);
On 2012/08/01 21:56:31, satorux1 wrote:
> Drivemount -> DriveMount

Done.

[satorux1, 2012-08-02 01:03:20]

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_cache.cc:1456:
DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
Add this instead: AssertOnSequencedWorkerPool();

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.cc:121: // TODO(yoshiki):
Notify to user when |error| != GDATA_FILE_OK.
Hmm, would it be difficult to do this now?

[satorux1, 2012-08-02 01:04:23]

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_cache.cc:1459:
file_util::Delete(cache_root_path_, true);
Check the return value?

[yoshiki, 2012-08-02 07:12:11]

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_cache.cc:1456:
DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
On 2012/08/02 01:03:21, satorux1 wrote:
> Add this instead: AssertOnSequencedWorkerPool();

Done.

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_cache.cc:1459:
file_util::Delete(cache_root_path_, true);
On 2012/08/02 01:04:23, satorux1 wrote:
> Check the return value?

Done.

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
File chrome/browser/chromeos/gdata/gdata_system_service.cc (right):

http://codereview.chromium.org/10823125/diff/14002/chrome/browser/chromeos/gd...
chrome/browser/chromeos/gdata/gdata_system_service.cc:121: // TODO(yoshiki):
Notify to user when |error| != GDATA_FILE_OK.
We need to think how to notify the failure to JS-side. Anyway, it is not
difficult only in the part of this CL.

On 2012/08/02 01:03:21, satorux1 wrote:
> Hmm, would it be difficult to do this now?

[yoshiki, 2012-08-03 07:15:24]

PTAL

[satorux1, 2012-08-03 17:17:31]

LGTM with nits

http://codereview.chromium.org/10823125/diff/6011/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

http://codereview.chromium.org/10823125/diff/6011/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_cache.cc:1462: *error = GDATA_FILE_OK;
*error = success ? GDATA_FILE_OK : GDATA_FILE_ERROR_FAILED

http://codereview.chromium.org/10823125/diff/6011/chrome/browser/chromeos/gda...
File chrome/browser/chromeos/gdata/gdata_system_service.h (right):

http://codereview.chromium.org/10823125/diff/6011/chrome/browser/chromeos/gda...
chrome/browser/chromeos/gdata/gdata_system_service.h:48: void ClearCache(const
base::Callback<void(bool)>& callback);
I changed my mind. I think we should give it a more descriptive name.

// Clears all the local cache files and in-memory data, and remounts
// the file system.
ClearCacheAndRemountFileSystem()

[yoshiki, 2012-08-03 18:23:15]

Thanks! I'm going to check-in this after trybots success.

https://chromiumcodereview.appspot.com/10823125/diff/6011/chrome/browser/chro...
File chrome/browser/chromeos/gdata/gdata_cache.cc (right):

https://chromiumcodereview.appspot.com/10823125/diff/6011/chrome/browser/chro...
chrome/browser/chromeos/gdata/gdata_cache.cc:1462: *error = GDATA_FILE_OK;
On 2012/08/03 17:17:31, satorux1 wrote:
> *error = success ? GDATA_FILE_OK : GDATA_FILE_ERROR_FAILED

Done.

https://chromiumcodereview.appspot.com/10823125/diff/6011/chrome/browser/chro...
File chrome/browser/chromeos/gdata/gdata_system_service.h (right):

https://chromiumcodereview.appspot.com/10823125/diff/6011/chrome/browser/chro...
chrome/browser/chromeos/gdata/gdata_system_service.h:48: void ClearCache(const
base::Callback<void(bool)>& callback);
On 2012/08/03 17:17:31, satorux1 wrote:
> I changed my mind. I think we should give it a more descriptive name.
> 
> // Clears all the local cache files and in-memory data, and remounts
> // the file system.
> ClearCacheAndRemountFileSystem()

Done.