Compiling file_manager, part 2: remove all warnings not caused by Object.freeze()-like enums

Compiling file_manager, part 2: remove all warnings not caused by Object.freeze()-like enums

BUG=393873
R=dbeam@chromium.org,mtomasz@chromium.org

Committed: https://crrev.com/12905b0c255733f273c9b6932885b0c2449e61f2
Cr-Commit-Position: refs/heads/master@{#296299}

[Vitaly Pavlenko, 2014-09-20 01:34:17]

mtomasz@, please, review ui/file_manager/ part.
dbeam@, please, review ui/webui/ part.

[mtomasz, 2014-09-20 01:43:23]

mtomasz@chromium.org changed reviewers:
+ fukino@chromium.org
- mtomasz@chromium.org

[mtomasz, 2014-09-20 01:44:11]

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

[mtomasz, 2014-09-20 01:44:11]

Thanks for the fixes! Deferring the review to fukino@ who was planning to enable
the closure compiler on Files app.

[mtomasz, 2014-09-21 14:24:09]

mtomasz@chromium.org changed reviewers:
- mtomasz@chromium.org

[fukino, 2014-09-22 06:05:57]

lgtm with nits. Thanks!

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/thumbnail_loader.js (right):

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:139: * @param
{Function=} opt_onError Error callback.
{function()=} looks more consistent with annotations for other parameters.

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:140: * @param
{Function=} opt_onGeneric Callback for generic image used.
ditto

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/image_loader...
File ui/file_manager/image_loader/image_loader_client.js (right):

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/image_loader...
ui/file_manager/image_loader/image_loader_client.js:126: * @param {Function=}
opt_isValid Function returning false in case
More specifically, could you annotate this param as "{function(): boolean=}"?

[Vitaly Pavlenko, 2014-09-22 17:34:26]

On 2014/09/22 06:05:57, fukino wrote:
> lgtm with nits. Thanks!
> 
>
https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
> File ui/file_manager/file_manager/foreground/js/thumbnail_loader.js (right):
> 
>
https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
> ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:139: * @param
> {Function=} opt_onError Error callback.
> {function()=} looks more consistent with annotations for other parameters.
> 
>
https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
> ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:140: * @param
> {Function=} opt_onGeneric Callback for generic image used.
> ditto

dbeam@, what do you think about Function/function()?

[Dan Beam, 2014-09-22 18:39:51]

On 2014/09/22 17:34:26, Vitaly Pavlenko wrote:
> On 2014/09/22 06:05:57, fukino wrote:
> > lgtm with nits. Thanks!
> > 
> >
>
https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
> > File ui/file_manager/file_manager/foreground/js/thumbnail_loader.js (right):
> > 
> >
>
https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
> > ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:139: * @param
> > {Function=} opt_onError Error callback.
> > {function()=} looks more consistent with annotations for other parameters.
> > 
> >
>
https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
> > ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:140: * @param
> > {Function=} opt_onGeneric Callback for generic image used.
> > ditto
> 
> dbeam@, what do you think about Function/function()?

function() can improve typechecking, but I don't have a strong opinion either
way.

[Vitaly Pavlenko, 2014-09-22 20:10:18]

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/thumbnail_loader.js (right):

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:139: * @param
{Function=} opt_onError Error callback.
On 2014/09/22 06:05:57, fukino wrote:
> {function()=} looks more consistent with annotations for other parameters.

Done.

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/thumbnail_loader.js:140: * @param
{Function=} opt_onGeneric Callback for generic image used.
On 2014/09/22 06:05:57, fukino wrote:
> ditto

Done.

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/image_loader...
File ui/file_manager/image_loader/image_loader_client.js (right):

https://codereview.chromium.org/575313004/diff/1/ui/file_manager/image_loader...
ui/file_manager/image_loader/image_loader_client.js:126: * @param {Function=}
opt_isValid Function returning false in case
On 2014/09/22 06:05:57, fukino wrote:
> More specifically, could you annotate this param as "{function(): boolean=}"?

Done.

[Dan Beam, 2014-09-23 02:26:33]

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/splitter.js (right):

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/splitter.js:41: * @param {number} doc The zoom
factor of the document.
doc is not a number

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/table/table_column.js (right):

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/table/table_column.js:37: endAlign_: false,
^ can you just remove all these?

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/table/table_splitter.js (right):

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/table/table_splitter.js:44: handleSplitterDragStart:
function() {
either these aren't used or they're @override-ing something

[Vitaly Pavlenko, 2014-09-23 20:21:59]

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/splitter.js (right):

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/splitter.js:41: * @param {number} doc The zoom
factor of the document.
On 2014/09/23 02:26:32, Dan Beam wrote:
> doc is not a number

Done.

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/table/table_column.js (right):

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/table/table_column.js:37: endAlign_: false,
On 2014/09/23 02:26:33, Dan Beam wrote:
> ^ can you just remove all these?

Done.

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
File ui/webui/resources/js/cr/ui/table/table_splitter.js (right):

https://codereview.chromium.org/575313004/diff/20001/ui/webui/resources/js/cr...
ui/webui/resources/js/cr/ui/table/table_splitter.js:44: handleSplitterDragStart:
function() {
On 2014/09/23 02:26:33, Dan Beam wrote:
> either these aren't used or they're @override-ing something

Done.

[Dan Beam, 2014-09-23 20:25:35]

lgtm

[Vitaly Pavlenko, 2014-09-23 22:23:48]

The CQ bit was checked by vitalyp@chromium.org

[commit-bot: I haz the power, 2014-09-23 22:25:27]

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

[commit-bot: I haz the power, 2014-09-23 23:35:51]

Committed patchset #4 (id:60001) as 3142f9528216ef8722c7ad13c7097b49f4d2c9ae

[commit-bot: I haz the power, 2014-09-23 23:36:31]

Patchset 4 (id:??) landed as
https://crrev.com/12905b0c255733f273c9b6932885b0c2449e61f2
Cr-Commit-Position: refs/heads/master@{#296299}