drive/syncFS: Switch to ID based download URL.

drive/syncFS: Switch to ID based download URL.

File downloading from drive server is now done from a fixed
URL generatable from resource ID, not an ephemeral URL got
by preceding additional API call.

To make the patch single-purposed, it will not yet remove the
"additional API call" to get the old download URL. That'll be
in a coming separate patch.

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

[kinaba, 2013-07-02 09:46:49]

ptal.

This needs https://codereview.chromium.org/18211008/ to be landed first.

[hidehiko, 2013-07-02 11:20:31]

lgtm

[tzik, 2013-07-02 12:34:27]

Looks good.

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/drive_backend/api_util.cc (right):

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/drive_backend/api_util.cc:490:
drive_service_->GetResourceEntry(
If |local_file_md5| is empty, we might able to skip this GetResourceEntry call
and can call DriveServiceInterface::DownloadFile directly.

[kinaba, 2013-07-02 13:15:08]

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/drive_backend/api_util.cc (right):

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/drive_backend/api_util.cc:490:
drive_service_->GetResourceEntry(
On 2013/07/02 12:34:27, tzik wrote:
> If |local_file_md5| is empty, we might able to skip this GetResourceEntry call
> and can call DriveServiceInterface::DownloadFile directly.

yeah, the same goes for Drive side as well.
Can I try that in the next separate patch?
I feel the current patch is already big enough.

[hidehiko, 2013-07-02 15:35:15]

On 2013/07/02 13:15:08, kinaba wrote:
>
https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
> File chrome/browser/sync_file_system/drive_backend/api_util.cc (right):
> 
>
https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
> chrome/browser/sync_file_system/drive_backend/api_util.cc:490:
> drive_service_->GetResourceEntry(
> On 2013/07/02 12:34:27, tzik wrote:
> > If |local_file_md5| is empty, we might able to skip this GetResourceEntry
call
> > and can call DriveServiceInterface::DownloadFile directly.
> 
> yeah, the same goes for Drive side as well.
> Can I try that in the next separate patch?
> I feel the current patch is already big enough.

Seems like all trybots failed due to apply-patch. Could you run them again?

[kinaba, 2013-07-03 02:18:56]

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/drive_backend/api_util.cc (right):

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/drive_backend/api_util.cc:490:
drive_service_->GetResourceEntry(
On 2013/07/02 13:15:09, kinaba wrote:
> On 2013/07/02 12:34:27, tzik wrote:
> > If |local_file_md5| is empty, we might able to skip this GetResourceEntry
call
> > and can call DriveServiceInterface::DownloadFile directly.

Fm, I also thought this is possible, but it turned out to be difficult.
APIUtil::DownloadFile calls back with (new) md5, size, and modification time
that can only be retrieved from GetResourceEntry.

I'm now thinking keeping this part as is. What do you think?

[tzik, 2013-07-03 02:40:42]

lgtm

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/drive_backend/api_util.cc (right):

https://codereview.chromium.org/18506002/diff/1/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/drive_backend/api_util.cc:490:
drive_service_->GetResourceEntry(
On 2013/07/03 02:18:56, kinaba wrote:
> On 2013/07/02 13:15:09, kinaba wrote:
> > On 2013/07/02 12:34:27, tzik wrote:
> > > If |local_file_md5| is empty, we might able to skip this GetResourceEntry
> call
> > > and can call DriveServiceInterface::DownloadFile directly.
> 
> Fm, I also thought this is possible, but it turned out to be difficult.
> APIUtil::DownloadFile calls back with (new) md5, size, and modification time
> that can only be retrieved from GetResourceEntry.
> 
> I'm now thinking keeping this part as is. What do you think?

Hmm, right it's not trivial.
OK, please leave it as is.

[commit-bot: I haz the power, 2013-07-03 03:40:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/18506002/15002

[commit-bot: I haz the power, 2013-07-03 10:00:08]

Change committed as 209943