Compile file_transfer_controller in gyp v2

Compile file_transfer_controller in gyp v2

- Removed dependency from file_transfor_controller to CommandHandler by
registering the needed function from FileManager to
FileTransferController as a constructor parameter.
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2830223003
Cr-Commit-Position: refs/heads/master@{#467912}
Committed: https://chromium.googlesource.com/chromium/src/+/8a09e380ca0d6d41a52d066da4b288dacc29f40e

[oka, 2017-04-21 06:31:24]

Description was changed from

==========
Compile file_transfer_controller.

BUG=636289
==========

to

==========
Compile file_transfer_controller.

BUG=636289
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 06:38:25]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 06:38:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 06:41:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 06:41:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oka, 2017-04-21 06:44:36]

Description was changed from

==========
Compile file_transfer_controller.

BUG=636289
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoid to use private memebers of file_manager in file_manager_command

BUG=636289
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 06:45:22]

Description was changed from

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoid to use private memebers of file_manager in file_manager_command

BUG=636289
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private memebers of file_manager in file_manager_command
    (used the equivalent getters instead)

BUG=636289
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 06:45:35]

Description was changed from

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private memebers of file_manager in file_manager_command
    (used the equivalent getters instead)

BUG=636289
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private memebers of file_manager in file_manager_command
    (used the equivalent getters instead)

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 06:57:33]

rebase.

[oka, 2017-04-21 06:58:24]

.

[oka, 2017-04-21 07:00:39]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 07:00:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oka, 2017-04-21 07:01:05]

Description was changed from

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private memebers of file_manager in file_manager_command
    (used the equivalent getters instead)

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private members of file_manager in file_manager_command
    (used the equivalent getters instead)

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 07:02:00]

Description was changed from

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private members of file_manager in file_manager_command
    (used the equivalent getters instead)

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private members of file_manager in file_manager_command
    (used the equivalent getters instead)
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 07:04:45]

.

[oka, 2017-04-21 07:10:06]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[oka, 2017-04-21 07:10:18]

.

[oka, 2017-04-21 07:12:35]

.

[oka, 2017-04-21 07:12:39]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[oka, 2017-04-21 07:12:47]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 07:13:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oka, 2017-04-21 07:13:17]

Description was changed from

==========
Compile file_transfer_controller.

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private members of file_manager in file_manager_command
    (used the equivalent getters instead)
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller in gyp v2

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private members of file_manager in file_manager_command
    (used the equivalent getters instead)
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-21 07:14:44]

oka@chromium.org changed reviewers:
+ fukino@chromium.org

[oka, 2017-04-21 07:14:45]

PTAL.

[fukino, 2017-04-21 07:36:59]

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp (right):

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp:186:
'../../background/js/compiled_resources2.gyp:file_operation_manager',
TODO comment is dropped unintentionally?

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js
(right):

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js:401:
VolumeManagerWrapper.prototype.shouldShowMenuItemForEntry = function(entry) {
Considering the VolumeManagerWrapper's responsibility ("Thin wrapper for
VolumeManager."), having these UI-related methods looks a bit strange to me.

If it doesn't fit in the CommandUtil, how about moving them to common or util
(and replace {VolumeManagerWrapper} with
{VolumeManagerCommon.VolumeInfoProvider}?

[commit-bot: I haz the power, 2017-04-21 07:40:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 07:40:15]

Dry run: This issue passed the CQ dry run.

[oka, 2017-04-27 05:00:53]

PTAL

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp (right):

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp:186:
'../../background/js/compiled_resources2.gyp:file_operation_manager',
On 2017/04/21 07:36:58, fukino wrote:
> TODO comment is dropped unintentionally?

Done.

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js
(right):

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js:401:
VolumeManagerWrapper.prototype.shouldShowMenuItemForEntry = function(entry) {
On 2017/04/21 07:36:59, fukino wrote:
> Considering the VolumeManagerWrapper's responsibility ("Thin wrapper for
> VolumeManager."), having these UI-related methods looks a bit strange to me.
> 
> If it doesn't fit in the CommandUtil, how about moving them to common or util
> (and replace {VolumeManagerWrapper} with
> {VolumeManagerCommon.VolumeInfoProvider}?

Maybe can I just rename this method to isValidNonRootEntry?

[fukino, 2017-04-27 06:07:53]

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js
(right):

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js:401:
VolumeManagerWrapper.prototype.shouldShowMenuItemForEntry = function(entry) {
On 2017/04/27 05:00:53, oka wrote:
> On 2017/04/21 07:36:59, fukino wrote:
> > Considering the VolumeManagerWrapper's responsibility ("Thin wrapper for
> > VolumeManager."), having these UI-related methods looks a bit strange to me.
> > 
> > If it doesn't fit in the CommandUtil, how about moving them to common or
util
> > (and replace {VolumeManagerWrapper} with
> > {VolumeManagerCommon.VolumeInfoProvider}?
> 
> Maybe can I just rename this method to isValidNonRootEntry?

Even when the function is renamed, the login inside it have to be tightly
connected with UI spec.
Didn't my suggestions work?

[oka, 2017-04-27 06:35:18]

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
File ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js
(right):

https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js:401:
VolumeManagerWrapper.prototype.shouldShowMenuItemForEntry = function(entry) {
On 2017/04/27 06:07:53, fukino wrote:
> On 2017/04/27 05:00:53, oka wrote:
> > On 2017/04/21 07:36:59, fukino wrote:
> > > Considering the VolumeManagerWrapper's responsibility ("Thin wrapper for
> > > VolumeManager."), having these UI-related methods looks a bit strange to
me.
> > > 
> > > If it doesn't fit in the CommandUtil, how about moving them to common or
> util
> > > (and replace {VolumeManagerWrapper} with
> > > {VolumeManagerCommon.VolumeInfoProvider}?
> > 
> > Maybe can I just rename this method to isValidNonRootEntry?
> 
> Even when the function is renamed, the login inside it have to be tightly
> connected with UI spec.
> Didn't my suggestions work?

OK. To which file did you suggest to move the function?

[oka, 2017-04-27 07:07:28]

On 2017/04/27 06:35:18, oka wrote:
>
https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
> File ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js
> (right):
> 
>
https://codereview.chromium.org/2830223003/diff/100001/ui/file_manager/file_m...
> ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js:401:
> VolumeManagerWrapper.prototype.shouldShowMenuItemForEntry = function(entry) {
> On 2017/04/27 06:07:53, fukino wrote:
> > On 2017/04/27 05:00:53, oka wrote:
> > > On 2017/04/21 07:36:59, fukino wrote:
> > > > Considering the VolumeManagerWrapper's responsibility ("Thin wrapper for
> > > > VolumeManager."), having these UI-related methods looks a bit strange to
> me.
> > > > 
> > > > If it doesn't fit in the CommandUtil, how about moving them to common or
> > util
> > > > (and replace {VolumeManagerWrapper} with
> > > > {VolumeManagerCommon.VolumeInfoProvider}?
> > > 
> > > Maybe can I just rename this method to isValidNonRootEntry?
> > 
> > Even when the function is renamed, the login inside it have to be tightly
> > connected with UI spec.
> > Didn't my suggestions work?
> 
> OK. To which file did you suggest to move the function?

Talked offline. To remove dependency from file_transfer_controller to
file_manager_commands, FileManager will register the function in CommandUtil to
FileTransfer as a callback.

[oka, 2017-04-27 07:29:56]

Register callback from FileManager to FileTransferController.

[oka, 2017-04-27 07:30:49]

Format.

[oka, 2017-04-27 07:33:22]

.

[oka, 2017-04-27 07:37:02]

Description was changed from

==========
Compile file_transfer_controller in gyp v2

- Extracted the function to extend cr.ui.Command into a separate file
- Moved two functions in CommandUtil to VolumeManagerWrapper
- Avoided to use private members of file_manager in file_manager_command
    (used the equivalent getters instead)
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller in gyp v2

- Removed dependency from file_transfor_controller to CommandHandler by
registering the needed function from FileManager to
FileTransferController as a constructor parameter.
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-04-27 07:39:59]

.

[oka, 2017-04-27 07:41:10]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 07:41:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oka, 2017-04-27 07:41:48]

PTAL.

[fukino, 2017-04-27 07:46:29]

lgtm. Thanks!

[oka, 2017-04-27 07:51:58]

Fix typo.

[oka, 2017-04-27 07:52:33]

The CQ bit was checked by oka@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-27 07:52:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oka, 2017-04-27 07:55:42]

The CQ bit was checked by oka@chromium.org

[oka, 2017-04-27 07:55:42]

The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org
Link to the patchset: https://codereview.chromium.org/2830223003/#ps220001
(title: "Fix typo.")

[oka, 2017-04-27 07:55:45]

The CQ bit was unchecked by oka@chromium.org

[commit-bot: I haz the power, 2017-04-27 07:55:51]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[oka, 2017-04-28 05:48:49]

The CQ bit was checked by oka@chromium.org

[commit-bot: I haz the power, 2017-04-28 05:49:07]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 05:54:21]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1493358529205480,
"parent_rev": "88d1d186845613b22258b6ad4eb168a4f63d8432", "commit_rev":
"8a09e380ca0d6d41a52d066da4b288dacc29f40e"}

[commit-bot: I haz the power, 2017-04-28 05:54:28]

Description was changed from

==========
Compile file_transfer_controller in gyp v2

- Removed dependency from file_transfor_controller to CommandHandler by
registering the needed function from FileManager to
FileTransferController as a constructor parameter.
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Compile file_transfer_controller in gyp v2

- Removed dependency from file_transfor_controller to CommandHandler by
registering the needed function from FileManager to
FileTransferController as a constructor parameter.
- Added drop_effect_and_label to compile target.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2830223003
Cr-Commit-Position: refs/heads/master@{#467912}
Committed:
https://chromium.googlesource.com/chromium/src/+/8a09e380ca0d6d41a52d066da4b2...
==========

[commit-bot: I haz the power, 2017-04-28 05:54:29]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/8a09e380ca0d6d41a52d066da4b2...