[filebrowser] Show progress indication, while files from GData are being downloaded.

[filebrowser] Show progress indication, while files from GData are being downloaded.

BUG=chromium-os:27504
TEST=See bug.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128920

[dgozman, 2012-03-22 16:15:28]

May you have a look, please?

[zel, 2012-03-22 17:21:46]

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1842:
FilePath("/special/gdata").Append(file_path),
THis does not look right. Why do you need /special here?

[Vladislav Kaznacheev, 2012-03-23 07:20:52]

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/resource...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/resource...
chrome/browser/resources/file_manager/js/file_manager.js:4010:
this.callSelectFilesApiAndClose_(selection);
We used to make this call from setTimeout. I trust you checked it is not needed
anymore.

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/resource...
chrome/browser/resources/file_manager/js/file_manager.js:4056:
chrome.fileBrowserPrivate.onFileTransfersUpdated.removeListener(
I am paranoid today:) Are you absolutely sure that there is no way to close the
dialog without calling cleanup? This would leave a listener dangling

[dgozman, 2012-03-23 12:44:05]

PTAL

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/file_browser_private_api.cc (right):

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/file_browser_private_api.cc:1842:
FilePath("/special/gdata").Append(file_path),
On 2012/03/22 17:21:46, zel wrote:
> THis does not look right. Why do you need /special here?

This is no different from ProgessStatusToDictionaryValue.
ConvertFileToFileSystemUrl expects url starting with '/special/gdata' and passes
it to CrosMountPointProvider.

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/resource...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/resource...
chrome/browser/resources/file_manager/js/file_manager.js:4010:
this.callSelectFilesApiAndClose_(selection);
On 2012/03/23 07:20:52, Vladislav Kaznacheev wrote:
> We used to make this call from setTimeout. I trust you checked it is not
needed
> anymore.

Good catch. Missed this here.

https://chromiumcodereview.appspot.com/9839009/diff/1/chrome/browser/resource...
chrome/browser/resources/file_manager/js/file_manager.js:4056:
chrome.fileBrowserPrivate.onFileTransfersUpdated.removeListener(
On 2012/03/23 07:20:52, Vladislav Kaznacheev wrote:
> I am paranoid today:) Are you absolutely sure that there is no way to close
the
> dialog without calling cleanup? This would leave a listener dangling

Hmm.. But we do not remove listeners for other events at all (e.g.
onMountCompleted). I think, listeners dies with the tab/renderer.

[Vladislav Kaznacheev, 2012-03-23 13:11:13]

LGTM with a nit.

https://chromiumcodereview.appspot.com/9839009/diff/7001/chrome/browser/resou...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

https://chromiumcodereview.appspot.com/9839009/diff/7001/chrome/browser/resou...
chrome/browser/resources/file_manager/js/file_manager.js:4062: // Accroding to
API cancel may fail, but there is no proper UI to reflect
Accroding -> According

[Vladislav Kaznacheev, 2012-03-23 14:32:10]

http://codereview.chromium.org/9839009/diff/8001/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

http://codereview.chromium.org/9839009/diff/8001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/file_manager.js:4043: done +=
status.processed - old;
if old == 1 it will still participate in the computation. Is this what you
wanted?

[dgozman, 2012-03-23 14:37:31]

http://codereview.chromium.org/9839009/diff/8001/chrome/browser/resources/fil...
File chrome/browser/resources/file_manager/js/file_manager.js (right):

http://codereview.chromium.org/9839009/diff/8001/chrome/browser/resources/fil...
chrome/browser/resources/file_manager/js/file_manager.js:4043: done +=
status.processed - old;
On 2012/03/23 14:32:10, Vladislav Kaznacheev wrote:
> if old == 1 it will still participate in the computation. Is this what you
> wanted?

Fixed.

[Vladislav Kaznacheev, 2012-03-23 14:40:24]

lgtm

[zel, 2012-03-23 15:38:27]

lgtm

[zel, 2012-03-25 00:27:08]

On 2012/03/23 15:38:27, zel wrote:
> lgtm

What's the status here? Why this hasn't landed yet? There are string changes
that must get in.

[zel, 2012-03-25 01:03:17]

http://codereview.chromium.org/9839009/diff/6003/chrome/app/generated_resourc...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/9839009/diff/6003/chrome/app/generated_resourc...
chrome/app/generated_resources.grd:10245: +        Preparing
you will most likely need to revert this since I've included this string with
other missing strings in https://chromiumcodereview.appspot.com/9838101/

[commit-bot: I haz the power, 2012-03-26 14:25:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dgozman@chromium.org/9839009/15001

[commit-bot: I haz the power, 2012-03-26 16:29:37]

Change committed as 128920