Files.app: Show drive sync state in the progress center.

Files.app: Show drive sync state in the progress center.

This CL adds the drive sync handler to the background page and showes the drive
sync status obtained by the handler in the progress center.

BUG=327213
TEST=manually

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

[hirono, 2014-01-15 04:12:54]

PTAL the CL? Thanks!

[mtomasz, 2014-01-15 06:38:00]

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
File chrome/browser/resources/file_manager/background/js/drive_sync_handler.js
(right):

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:36: //
Register event.
nit: event -> events / the event.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:42: *
Complete event name.
nit: jsdoc missing.
nit: Complete -> Completion

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:59:
get synching() {
nit: synching -> syncing

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:61:
return true;
This looks tricky. Why not if (this.items_) { ... }?

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:68: *
Handle file transfer updated events.
nit: Handles

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:69: *
@param {Array.<object>} statusList List of drive status.
nit: object -> Object

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:81:
case 'completed':
Let's be consistent. Please choose one consistently: completion / completed.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:84: if
(!this.synching)
nit: ditto typo

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:85:
this.dispatchEvent(new Event(DriveSyncHandler.COMPLETE_EVENT));
ditto: -> completion event. 'Complete event' is misleading.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:102:
var fileName = status.fileUrl.replace(/.*\//, '');
Can we avoid working on URLs? Can we convert to Entry and use Entry.name?

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:135: *
@param {string} url URL of the file to be canceled.
Can we work on Entries instead of URLs?

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
File chrome/browser/resources/file_manager/foreground/css/file_manager.css
(right):

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/foreground/css/file_manager.css:2010: top:
17px;
Do we need so many magic numbers? Can't we use flex model instead? Will it work
well when font size is changed?

[hirono, 2014-01-16 05:05:52]

Thanks!

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
File chrome/browser/resources/file_manager/background/js/drive_sync_handler.js
(right):

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:36: //
Register event.
On 2014/01/15 06:38:01, mtomasz wrote:
> nit: event -> events / the event.

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:42: *
Complete event name.
On 2014/01/15 06:38:01, mtomasz wrote:
> nit: jsdoc missing.
> nit: Complete -> Completion

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:59:
get synching() {
On 2014/01/15 06:38:01, mtomasz wrote:
> nit: synching -> syncing

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:61:
return true;
On 2014/01/15 06:38:01, mtomasz wrote:
> This looks tricky. Why not if (this.items_) { ... }?

items_ is an object and this is checking if it does not have a property.
Added a comment.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:68: *
Handle file transfer updated events.
On 2014/01/15 06:38:01, mtomasz wrote:
> nit: Handles

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:69: *
@param {Array.<object>} statusList List of drive status.
On 2014/01/15 06:38:01, mtomasz wrote:
> nit: object -> Object

Replaced with FileTransferStatus, which is defined in file_browser_private.idl.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:81:
case 'completed':
On 2014/01/15 06:38:01, mtomasz wrote:
> Let's be consistent. Please choose one consistently: completion / completed.

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:84: if
(!this.synching)
On 2014/01/15 06:38:01, mtomasz wrote:
> nit: ditto typo

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:85:
this.dispatchEvent(new Event(DriveSyncHandler.COMPLETE_EVENT));
On 2014/01/15 06:38:01, mtomasz wrote:
> ditto: -> completion event. 'Complete event' is misleading.

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:102:
var fileName = status.fileUrl.replace(/.*\//, '');
On 2014/01/15 06:38:01, mtomasz wrote:
> Can we avoid working on URLs? Can we convert to Entry and use Entry.name?

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:135: *
@param {string} url URL of the file to be canceled.
On 2014/01/15 06:38:01, mtomasz wrote:
> Can we work on Entries instead of URLs?

Done.

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
File chrome/browser/resources/file_manager/foreground/css/file_manager.css
(right):

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/foreground/css/file_manager.css:2010: top:
17px;
On 2014/01/15 06:38:01, mtomasz wrote:
> Do we need so many magic numbers? Can't we use flex model instead? Will it
work
> well when font size is changed?

Yes, it will not work with different font size...
Let me update the structure of DOM and CSS!

[mtomasz, 2014-01-16 06:01:54]

https://codereview.chromium.org/131403003/diff/470001/chrome/browser/resource...
File chrome/browser/resources/file_manager/background/js/drive_sync_handler.js
(right):

https://codereview.chromium.org/131403003/diff/470001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:103:
webkitResolveLocalFileSystemURL(status.fileUrl, function(entry) {
We should convert fileUrl to an Entry in custom bindings of fileBrowserPrivate
api? Can you please file a bug and assign me? Thanks!

[hirono, 2014-01-16 08:46:24]

Thanks!

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
File chrome/browser/resources/file_manager/foreground/css/file_manager.css
(right):

https://codereview.chromium.org/131403003/diff/280001/chrome/browser/resource...
chrome/browser/resources/file_manager/foreground/css/file_manager.css:2010: top:
17px;
On 2014/01/16 05:05:52, hirono wrote:
> On 2014/01/15 06:38:01, mtomasz wrote:
> > Do we need so many magic numbers? Can't we use flex model instead? Will it
> work
> > well when font size is changed?
> 
> Yes, it will not work with different font size...
> Let me update the structure of DOM and CSS!

Done.

https://codereview.chromium.org/131403003/diff/470001/chrome/browser/resource...
File chrome/browser/resources/file_manager/background/js/drive_sync_handler.js
(right):

https://codereview.chromium.org/131403003/diff/470001/chrome/browser/resource...
chrome/browser/resources/file_manager/background/js/drive_sync_handler.js:103:
webkitResolveLocalFileSystemURL(status.fileUrl, function(entry) {
On 2014/01/16 06:01:55, mtomasz wrote:
> We should convert fileUrl to an Entry in custom bindings of fileBrowserPrivate
> api? Can you please file a bug and assign me? Thanks!

Thank you for handling it! Filed crbug.com/334989.

[mtomasz, 2014-01-17 02:49:48]

lgtm!

[hirono, 2014-01-17 02:54:21]

@dgozman - PTAL component_extension_resources.grd? Thanks!

[dgozman, 2014-01-17 05:46:53]

component_extension_resources.grd lgtm

[hirono, 2014-01-17 05:47:54]

On 2014/01/17 05:46:53, dgozman wrote:
> component_extension_resources.grd lgtm

Thanks!

[commit-bot: I haz the power, 2014-01-17 05:48:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hirono@chromium.org/131403003/550001

[commit-bot: I haz the power, 2014-01-17 10:01:46]

Change committed as 245473