Video Player: Call video element's load method on loading video file.

Video Player: Call video element's load method on loading video file.

Issue 598355 is a regression by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called to load it properly.
In addition, we don't need the workaround anymore on ToT.
This CL brings back the call to video element's load() again.

BUG=598355
TEST=manually test using Chromecast

Committed: https://crrev.com/26f5fe00a7713e0ca05a9b70089164c871b2465d
Cr-Commit-Position: refs/heads/master@{#383693}

[fukino, 2016-03-29 07:55:12]

Description was changed from

==========
Video Player: Call video element's load method on loading video file.

BUG=598355
TEST=manually test using Chromecast
==========

to

==========
Video Player: Call video element's load method on loading video file.

The issue is introduced by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.

BUG=598355
TEST=manually test using Chromecast
==========

[fukino, 2016-03-29 07:57:15]

Description was changed from

==========
Video Player: Call video element's load method on loading video file.

The issue is introduced by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.

BUG=598355
TEST=manually test using Chromecast
==========

to

==========
Video Player: Call video element's load method on loading video file.

Issue 598355 is a regression by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.
This CL brings back the call to video element's load() again.

BUG=598355
TEST=manually test using Chromecast
==========

[fukino, 2016-03-29 07:57:33]

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

[fukino, 2016-03-29 07:57:34]

Awano-san, could you take a look?

[yawano, 2016-03-29 08:29:23]

lgtm. Thank you!

https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
File ui/file_manager/video_player/js/media_controls.js (right):

https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
ui/file_manager/video_player/js/media_controls.js:622: if
(this.media_.textTracks && this.media_.textTracks.length > 0) {
Very small optional nit: I don't think we need to check this here. As far as I
check the W3C specification, textTracks won't be null. -
https://www.w3.org/TR/2011/WD-html5-20110405/video.html#media-elements

[fukino, 2016-03-29 08:48:28]

https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
File ui/file_manager/video_player/js/media_controls.js (right):

https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
ui/file_manager/video_player/js/media_controls.js:622: if
(this.media_.textTracks && this.media_.textTracks.length > 0) {
On 2016/03/29 08:29:23, yawano wrote:
> Very small optional nit: I don't think we need to check this here. As far as I
> check the W3C specification, textTracks won't be null. -
> https://www.w3.org/TR/2011/WD-html5-20110405/video.html#media-elements

If the media is CastVideoElement, the textTracks is undefined.
Maybe we should add textTracks property to CastVideoElement?

[yawano, 2016-03-29 08:59:14]

https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
File ui/file_manager/video_player/js/media_controls.js (right):

https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
ui/file_manager/video_player/js/media_controls.js:622: if
(this.media_.textTracks && this.media_.textTracks.length > 0) {
I get it. Thank you!

No, I don't think we need to add the property. It might make the element
consistent with the native VideoElement, but I think this check is okay for now.

[fukino, 2016-03-29 09:05:20]

On 2016/03/29 08:59:14, yawano wrote:
>
https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
> File ui/file_manager/video_player/js/media_controls.js (right):
> 
>
https://codereview.chromium.org/1839953002/diff/1/ui/file_manager/video_playe...
> ui/file_manager/video_player/js/media_controls.js:622: if
> (this.media_.textTracks && this.media_.textTracks.length > 0) {
> I get it. Thank you!
> 
> No, I don't think we need to add the property. It might make the element
> consistent with the native VideoElement, but I think this check is okay for
now.

Thank you!
I'll go with this patch for now.
At some point, I'll update CastVideoElement more consistent with video element,
including textTracks property.

[fukino, 2016-03-29 09:05:30]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2016-03-29 09:05:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1839953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1839953002/1

[commit-bot: I haz the power, 2016-03-29 09:50:30]

Description was changed from

==========
Video Player: Call video element's load method on loading video file.

Issue 598355 is a regression by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.
This CL brings back the call to video element's load() again.

BUG=598355
TEST=manually test using Chromecast
==========

to

==========
Video Player: Call video element's load method on loading video file.

Issue 598355 is a regression by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.
This CL brings back the call to video element's load() again.

BUG=598355
TEST=manually test using Chromecast
==========

[commit-bot: I haz the power, 2016-03-29 09:50:33]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-29 09:52:01]

Description was changed from

==========
Video Player: Call video element's load method on loading video file.

Issue 598355 is a regression by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.
This CL brings back the call to video element's load() again.

BUG=598355
TEST=manually test using Chromecast
==========

to

==========
Video Player: Call video element's load method on loading video file.

Issue 598355 is a regression by https://codereview.chromium.org/1782363003.
In the CL, video element's load() was commented out to work around issue 594537.
However, when the video element is CastVideoElement, load() needs to be called
to load it properly.
In addition, we don't need the workaround anymore on ToT.
This CL brings back the call to video element's load() again.

BUG=598355
TEST=manually test using Chromecast

Committed: https://crrev.com/26f5fe00a7713e0ca05a9b70089164c871b2465d
Cr-Commit-Position: refs/heads/master@{#383693}
==========

[commit-bot: I haz the power, 2016-03-29 09:52:02]

Patchset 1 (id:??) landed as
https://crrev.com/26f5fe00a7713e0ca05a9b70089164c871b2465d
Cr-Commit-Position: refs/heads/master@{#383693}