drive: Add DownloadOperation::EnsureFileDownloadedByResourceId

drive: Add DownloadOperation::EnsureFileDownloadedByResourceId

FileSystem::GetFileByResourceId was converting resource ID to file path and then calling EnsureFileDownloaded.
Rename EnsureFileDownloaded to EnsureFileDownloadedByPath and add an additional version EnsureFlieDownloadedByResourceID
to avoid the unnecessary conversion from resource ID to file path.

BUG=244135
TEST=unit_tests
R=satorux@chromium.org

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

[hashimoto, 2013-05-29 07:41:01]

On 2013/05/29 07:38:36, hashimoto wrote:

Changing the argument of EnsureFileDownloaded from file path to resource ID
seems not so good when considering that 3 out of 4 users of the function are
initiating the operation with file path.
I'd like to add another variant which takes resource ID as an argument instead.

[satorux1, 2013-05-30 01:14:35]

LGTM with a nit and a request.

https://codereview.chromium.org/16190003/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/download_operation.h (right):

https://codereview.chromium.org/16190003/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/download_operation.h:48: // Ensures
that the file content is locally downloaded.
the file content, specified by |resource_id|,

https://codereview.chromium.org/16190003/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/download_operation.h:64: void
EnsureFileDownloadedByResourceId(
Could you add a test for this function? you might want to introduce
download_operation_unittest.cc first... kinaba@ is in the process of moving test
code from file_system_unittest.cc to file_system/*_unittest.cc, so he may have
already started doing this for download_operation.

if it's too much of hassle, you can file a bug and do it later.

[hashimoto, 2013-05-30 01:47:25]

https://codereview.chromium.org/16190003/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/file_system/download_operation.h (right):

https://codereview.chromium.org/16190003/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/download_operation.h:48: // Ensures
that the file content is locally downloaded.
On 2013/05/30 01:14:35, satorux1 wrote:
> the file content, specified by |resource_id|,

Done.

https://codereview.chromium.org/16190003/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/file_system/download_operation.h:64: void
EnsureFileDownloadedByResourceId(
On 2013/05/30 01:14:35, satorux1 wrote:
> Could you add a test for this function? you might want to introduce
> download_operation_unittest.cc first... kinaba@ is in the process of moving
test
> code from file_system_unittest.cc to file_system/*_unittest.cc, so he may have
> already started doing this for download_operation.
> 
> if it's too much of hassle, you can file a bug and do it later.
This method is tested by FileSystemTest.GetFileByResourceID*.
To make independent unit test for download_operation.cc, kinaba@ has filed
http://crbug.com/244344.

[commit-bot: I haz the power, 2013-05-30 01:47:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/16190003/9001

[hashimoto, 2013-05-30 03:28:33]

Committed patchset #3 manually as r203040 (presubmit successful).