Video Player: Supports multiple files

Video Player: Supports multiple files

This patch adds a support of opening multiple files to Video Player. Previously, the player can open only a single file.

This patch does:
* Remove file-tasks filter which removes VideoPlayer on opening a single file.
* Add left/right arrows which can switches to prev/next video.
  - Arrows are hidden when opening a single file.
  - Left (or right) arrow are hidden when playing the first (or last) file.

BUG=355545
TEST=manual
R=mtomasz@chromium.org

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

[yoshiki, 2014-06-05 14:46:55]

@mtomasz: PTAL. Thanks

[mtomasz, 2014-06-06 00:24:07]

Please add descriptions to CLs and briefly explain what this CL does. It makes
it much easier to review code.

[yoshiki, 2014-06-06 02:51:02]

On 2014/06/06 00:24:07, mtomasz wrote:
> Please add descriptions to CLs and briefly explain what this CL does. It makes
> it much easier to review code.

Done. PTAL.

[mtomasz, 2014-06-06 03:41:44]

lgtm with nits

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
File ui/file_manager/video_player/css/arrow_box.css (right):

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/css/arrow_box.css:17: opacity: 0;
Why not (1) visibility: hidden, or (2) display: none, or (3) hidden (attribute)?
Setting opacity to 0 and pointer events to none may be unfriendly for chromevox?

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
File ui/file_manager/video_player/js/video_player.js (right):

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/js/video_player.js:217: if (this.currentPos_ ==
(this.videos_.length - 1))
nit: == -> ===

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/js/video_player.js:324: if (newPos < 0 ||
this.videos_.length <= newPos)
optional: newPos <= this.videos_.length? It is hard to read the other way. But
it is up to your preference.

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/js/video_player.js:335: *
Please be consistent with \n in jsdoc. In #344 the style is different.

[yoshiki, 2014-06-06 08:02:23]

Thanks!

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
File ui/file_manager/video_player/css/arrow_box.css (right):

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/css/arrow_box.css:17: opacity: 0;
On 2014/06/06 03:41:43, mtomasz wrote:
> Why not (1) visibility: hidden, or (2) display: none, or (3) hidden
(attribute)?
> Setting opacity to 0 and pointer events to none may be unfriendly for
chromevox?

I've just found 'tools'-related feature is controlled by media_controller.js, so
I removed here.

I assume the reason why we don't use either is to animate them.

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
File ui/file_manager/video_player/js/video_player.js (right):

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/js/video_player.js:217: if (this.currentPos_ ==
(this.videos_.length - 1))
On 2014/06/06 03:41:43, mtomasz wrote:
> nit: == -> ===

Done.

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/js/video_player.js:324: if (newPos < 0 ||
this.videos_.length <= newPos)
On 2014/06/06 03:41:43, mtomasz wrote:
> optional: newPos <= this.videos_.length? It is hard to read the other way. But
> it is up to your preference.

Done. I don't have a strong opinion.

https://codereview.chromium.org/313273002/diff/70001/ui/file_manager/video_pl...
ui/file_manager/video_player/js/video_player.js:335: *
On 2014/06/06 03:41:43, mtomasz wrote:
> Please be consistent with \n in jsdoc. In #344 the style is different.

Done.

[yoshiki, 2014-06-06 08:02:27]

The CQ bit was checked by yoshiki@chromium.org

[commit-bot: I haz the power, 2014-06-06 08:03:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yoshiki@chromium.org/313273002/110001

[commit-bot: I haz the power, 2014-06-06 11:18:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-06 12:19:55]

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

[commit-bot: I haz the power, 2014-06-06 12:19:56]

Try jobs failed on following builders:
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[yoshiki, 2014-06-06 13:52:43]

Committed patchset #2 manually as r275432 (presubmit successful).