A video tag should play content src is set asynchronously with preload is disabled.

A video tag should play content src is set asynchronously with preload is disabled.

On a video tag, when preload is set to none, if  play() has been
called and a src has not been set, the video tag should play
content when the src is set later asynchronously.

BUG=633591

[George Joseph, 2017-05-26 18:11:10]

kottackal.george@gmail.com changed reviewers:
+ mlamouri@chromium.org

[George Joseph, 2017-05-26 18:11:10]

Hello Mlamouri, 

 I did a quick fix of this issue.With this patch , chrome's behavior would match
Firefox behavior. Do review and let me know your thoughts.

Thanks,
George.

[mlamouri (slow - plz ping), 2017-05-30 10:08:38]

mlamouri@chromium.org changed reviewers:
+ foolip@chromium.org

[mlamouri (slow - plz ping), 2017-05-30 10:08:39]

Thanks for the CL George. Before adding more states to HTMLMediaElement, I would
like to make sure we are actually fixing a bug and not implementing a bug that
Edge and Firefox have.

+foolip@ in case of he has some insights.

Let's discuss details in the bug.

[foolip, 2017-05-30 10:41:27]

Commented on the bug, although this workaround probably works, it's adding more
state that'd be nice to avoid. Let's discuss on the bug. Starting a dry run
anyway to see if this would regress anything.

[foolip, 2017-05-30 10:41:32]

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

[commit-bot: I haz the power, 2017-05-30 10:41:56]

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-05-30 12:38:04]

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

[commit-bot: I haz the power, 2017-05-30 12:38:05]

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

[George Joseph, 2017-06-05 20:01:58]

On 2017/05/30 10:41:27, foolip wrote:
> Commented on the bug, although this workaround probably works, it's adding
more
> state that'd be nice to avoid. Let's discuss on the bug. Starting a dry run
> anyway to see if this would regress anything.

Hello foolip and  mlamouri, 

 I have updated the patch and added a test. All the tests should now pass.
Please do review and let me know your thoughts.

Thanks.

[foolip, 2017-06-07 10:03:05]

I'll be OOO for the rest of the week and maybe Monday and won't be able to give
this the attention it deserved. Removing self as reviewer to make that clear.

[foolip, 2017-06-07 10:03:17]

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

[George Joseph, 2017-06-14 14:32:58]

On 2017/06/07 10:03:17, foolip wrote:

Hello mlamouri, 

 Any thought about this patch?

Thanks,
George.

[George Joseph, 2017-06-20 10:44:20]

kottackal.george@gmail.com changed reviewers:
+ foolip@chromium.org

[George Joseph, 2017-06-20 10:44:20]

Hello mlamouri and foolip, 

Apologies for asking again. Would you have any thoughts about this patch.

Thanks.

[foolip, 2017-07-05 14:37:58]

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

[foolip, 2017-07-05 14:37:59]

Removing self as reviewer again to make clear that I won't have time to look
into this properly. Srirama might be able to review, I don't know.

[George Joseph, 2017-07-06 09:42:12]

kottackal.george@gmail.com changed reviewers:
+ srirama.m@samsung.com

[Srirama, 2017-07-07 09:59:28]

Have you though of avoiding the extra state we are introducing here as mentioned
by foolip@?
Starting trybot to see the results on new patch.

https://codereview.chromium.org/2910683003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/video-play-when-src-set-asynchronously.html
(right):

https://codereview.chromium.org/2910683003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-play-when-src-set-asynchronously.html:8:
var testobject = async_test(function(t) {
you can remove var testobject.

https://codereview.chromium.org/2910683003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-play-when-src-set-asynchronously.html:12:
video.onplaying =  t.step_func(function() {t.done()});
use t_step_func_done();

[Srirama, 2017-07-07 09:59:34]

The CQ bit was checked by srirama.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 09:59:41]

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-07-07 12:03:54]

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

[commit-bot: I haz the power, 2017-07-07 12:03:55]

Dry run: This issue passed the CQ dry run.

[George Joseph, 2017-07-10 06:58:45]

Hello Srirama,
  Thanks for reviewing the commit. I have made the changes as you have
suggested. Please do review. I did try not using a boolean - however, this means
checking a combination of states in the other variables - I did not have much
luck with this.

Do let me know your feedback,

Thanks,
George.

https://codereview.chromium.org/2910683003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/video-play-when-src-set-asynchronously.html
(right):

https://codereview.chromium.org/2910683003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-play-when-src-set-asynchronously.html:8:
var testobject = async_test(function(t) {
On 2017/07/07 09:59:27, Srirama wrote:
> you can remove var testobject.

Done.

https://codereview.chromium.org/2910683003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-play-when-src-set-asynchronously.html:12:
video.onplaying =  t.step_func(function() {t.done()});
On 2017/07/07 09:59:28, Srirama wrote:
> use t_step_func_done();

Done.

[George Joseph, 2017-07-10 06:58:46]

Hello Srirama,
  Thanks for reviewing the commit. I have made the changes as you have
suggested. Please do review. I did try not using a boolean - however, this means
checking a combination of states in the other variables - I did not have much
luck with this.

Do let me know your feedback,

Thanks,
George.

[Srirama, 2017-07-10 07:03:49]

On 2017/07/10 06:58:46, George Joseph wrote:
> Hello Srirama,
>   Thanks for reviewing the commit. I have made the changes as you have
> suggested. Please do review. I did try not using a boolean - however, this
means
> checking a combination of states in the other variables - I did not have much
> luck with this.
> 
> Do let me know your feedback,
> 
> Thanks,
> George.

As discussed in the bug, the work on microtask changes have some dependency and
i don't think that will be done anytime soon.
So i am ok to land this change for now.

@Mounir, WDYT?

[George Joseph, 2017-08-09 13:49:47]

On 2017/07/10 07:03:49, Srirama wrote:
> On 2017/07/10 06:58:46, George Joseph wrote:
> > Hello Srirama,
> >   Thanks for reviewing the commit. I have made the changes as you have
> > suggested. Please do review. I did try not using a boolean - however, this
> means
> > checking a combination of states in the other variables - I did not have
much
> > luck with this.
> > 
> > Do let me know your feedback,
> > 
> > Thanks,
> > George.
> 
> As discussed in the bug, the work on microtask changes have some dependency
and
> i don't think that will be done anytime soon.
> So i am ok to land this change for now.
> 
> @Mounir, WDYT?

Hello Sriaram and Mounir, 

  Any luck with reviewing this issue. Do let me know your thoughts.

Thanks,

[Srirama, 2017-08-17 04:53:02]

@Mounir, Please have a look at it.