Add button to add new FSP services to Files app.

Add button to add new FSP services to Files app.

This CL adds a FSP suggest dialog which can be shown by clicking the button at
the end of the left nav.

TEST=Tested manually by clicking on the new button in the left nav.
BUG=471722
Committed: https://crrev.com/8cb8d4d8aa70bac94d331efa9b94b2007536a54f
Cr-Commit-Position: refs/heads/master@{#323462}

[mtomasz, 2015-04-01 10:38:08]

mtomasz@chromium.org changed reviewers:
+ hirono@chromium.org

[mtomasz, 2015-04-01 10:38:59]

@hirono: PTAL. Thanks.

[hirono, 2015-04-01 11:50:36]

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/navigation_list_model.js
(right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:69: * Item
of NavigationListModel for commands.
Cannot be this wrapper of cr.ui.Command? We usually assign cr.ui.Command to UI
element. We can handle visibility change or shortcut key in general way if
needed.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:75: *
@extends {NavigationModelItem}
Can we add @struct?

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:81:
Object.freeze(this);
We no longer relay on Object.freeze. crbug.com/412239

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:274:
(this.commandModel_ ? 1 : 0);
nit: Please fix indent.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:299: //
TODO(mtomasz, yoshiki): Implement when needed.
If we know when it becomes to be needed, please add comment. Otherwise, maybe we
can drop this.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/directory_tree.js (right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/directory_tree.js:835: * @param
{Event} e
Just @override is enough for overriding method.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/directory_tree.js:988:
console.log(modelItem.type);
nit: debug log.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js
(right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js:189:
file_extension: (extension && extension[0] === '.')
How about assert instead of this handling as the description at #179.

[hirono, 2015-04-01 12:35:17]

On 2015/04/01 11:50:36, hirono wrote:
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> File ui/file_manager/file_manager/foreground/js/navigation_list_model.js
> (right):
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:69: * Item
> of NavigationListModel for commands.
> Cannot be this wrapper of cr.ui.Command? We usually assign cr.ui.Command to UI
> element. We can handle visibility change or shortcut key in general way if
> needed.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:75: *
> @extends {NavigationModelItem}
> Can we add @struct?
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:81:
> Object.freeze(this);
> We no longer relay on Object.freeze. crbug.com/412239
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:274:
> (this.commandModel_ ? 1 : 0);
> nit: Please fix indent.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:299: //
> TODO(mtomasz, yoshiki): Implement when needed.
> If we know when it becomes to be needed, please add comment. Otherwise, maybe
we
> can drop this.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> File ui/file_manager/file_manager/foreground/js/ui/directory_tree.js (right):
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/ui/directory_tree.js:835: * @param
> {Event} e
> Just @override is enough for overriding method.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/ui/directory_tree.js:988:
> console.log(modelItem.type);
> nit: debug log.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> File ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js
> (right):
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js:189:
> file_extension: (extension && extension[0] === '.')
> How about assert instead of this handling as the description at #179.

Also please double check png files are optimized by pngcrush.

[mtomasz, 2015-04-02 02:24:27]

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/navigation_list_model.js
(right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:69: * Item
of NavigationListModel for commands.
On 2015/04/01 11:50:36, hirono wrote:
> Cannot be this wrapper of cr.ui.Command? We usually assign cr.ui.Command to UI
> element. We can handle visibility change or shortcut key in general way if
> needed.

Done.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:75: *
@extends {NavigationModelItem}
On 2015/04/01 11:50:36, hirono wrote:
> Can we add @struct?

Done.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:81:
Object.freeze(this);
On 2015/04/01 11:50:36, hirono wrote:
> We no longer relay on Object.freeze. crbug.com/412239

Done.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/directory_tree.js (right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/directory_tree.js:835: * @param
{Event} e
On 2015/04/01 11:50:36, hirono wrote:
> Just @override is enough for overriding method.

Done.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js
(right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js:189:
file_extension: (extension && extension[0] === '.')
On 2015/04/01 11:50:36, hirono wrote:
> How about assert instead of this handling as the description at #179.

Added an assert. I think keeping the commend is useful, as users can find out
the format of the argument without checking the code.

[mtomasz, 2015-04-02 03:55:54]

On 2015/04/02 02:24:27, mtomasz wrote:
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> File ui/file_manager/file_manager/foreground/js/navigation_list_model.js
> (right):
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:69: * Item
> of NavigationListModel for commands.
> On 2015/04/01 11:50:36, hirono wrote:
> > Cannot be this wrapper of cr.ui.Command? We usually assign cr.ui.Command to
UI
> > element. We can handle visibility change or shortcut key in general way if
> > needed.
> 
> Done.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:75: *
> @extends {NavigationModelItem}
> On 2015/04/01 11:50:36, hirono wrote:
> > Can we add @struct?
> 
> Done.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/navigation_list_model.js:81:
> Object.freeze(this);
> On 2015/04/01 11:50:36, hirono wrote:
> > We no longer relay on Object.freeze. crbug.com/412239
> 
> Done.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> File ui/file_manager/file_manager/foreground/js/ui/directory_tree.js (right):
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/ui/directory_tree.js:835: * @param
> {Event} e
> On 2015/04/01 11:50:36, hirono wrote:
> > Just @override is enough for overriding method.
> 
> Done.
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> File ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js
> (right):
> 
>
https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
> ui/file_manager/file_manager/foreground/js/ui/suggest_apps_dialog.js:189:
> file_extension: (extension && extension[0] === '.')
> On 2015/04/01 11:50:36, hirono wrote:
> > How about assert instead of this handling as the description at #179.
> 
> Added an assert. I think keeping the commend is useful, as users can find out
> the format of the argument without checking the code.

I reverted pngcrush, as it breaks alpha. After some tweaks png I made it
preserve alpha but it broke colors. As the assets are small, keeping it not
crushed.

[hirono, 2015-04-02 04:27:24]

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/navigation_list_model.js
(right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:274:
(this.commandModel_ ? 1 : 0);
On 2015/04/01 11:50:36, hirono wrote:
> nit: Please fix indent.

Please fix this.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:299: //
TODO(mtomasz, yoshiki): Implement when needed.
On 2015/04/01 11:50:36, hirono wrote:
> If we know when it becomes to be needed, please add comment. Otherwise, maybe
we
> can drop this.

ditto.

[mtomasz, 2015-04-02 04:51:50]

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/navigation_list_model.js
(right):

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:274:
(this.commandModel_ ? 1 : 0);
On 2015/04/02 04:27:24, hirono wrote:
> On 2015/04/01 11:50:36, hirono wrote:
> > nit: Please fix indent.
> 
> Please fix this.

Done.

https://codereview.chromium.org/1056433003/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/navigation_list_model.js:299: //
TODO(mtomasz, yoshiki): Implement when needed.
On 2015/04/02 04:27:24, hirono wrote:
> On 2015/04/01 11:50:36, hirono wrote:
> > If we know when it becomes to be needed, please add comment. Otherwise,
maybe
> we
> > can drop this.
> 
> ditto.

Done.

[hirono, 2015-04-02 04:52:34]

lgtm! Thanks!

[mtomasz, 2015-04-02 10:44:28]

The CQ bit was checked by mtomasz@chromium.org

[mtomasz, 2015-04-02 10:44:28]

The patchset sent to the CQ was uploaded after l-g-t-m from hirono@chromium.org
Link to the patchset: https://codereview.chromium.org/1056433003/#ps140001
(title: "Fixed tests.")

[commit-bot: I haz the power, 2015-04-02 10:44:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056433003/140001

[commit-bot: I haz the power, 2015-04-02 11:14:20]

Committed patchset #8 (id:140001)

[mfomitchev, 2015-04-02 20:26:32]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1058873004/ by mfomitchev@chromium.org.

The reason for reverting is: Broke the build:
http://build.chromium.org/p/chromium.fyi/builders/Closure%20Compilation%20Lin...

##
/mnt/data/b/build/slave/Closure_Compilation_Linux/build/src/ui/file_manager/file_manager/foreground/js/navigation_list_model.js:33:
ERROR - NavigationModelShortcutItem cannot extend this type; structs can only
extend structs
## function NavigationModelShortcutItem(label, entry) {
.

[commit-bot: I haz the power, 2015-04-03 20:24:20]

Patchset 8 (id:??) landed as
https://crrev.com/8cb8d4d8aa70bac94d331efa9b94b2007536a54f
Cr-Commit-Position: refs/heads/master@{#323462}