Ian Hickson | 2 Aug 11:17 2010
Picon

Re: Race condition in media load algorithm

On Mon, 2 Aug 2010, Chris Pearce wrote:
>
> There's a race condition in the media load algorithm. When the resource 
> selection algorithm begins, it sets a task to complete the rest of the 
> resource selection algorithm asynchronously.

Not quite. It awaits a stable state and then runs a synchronous section, 
which means that it will run the subsequent steps as soon as the current 
task has finished, before anything else that is queued.

Thus:

> In the asynchronous task, we set the delaying-the-load-event flag to 
> true at step 4. But between the resource selection algorithm setting the 
> task to asynchronously continue the algorithm, and the task actually 
> running, the load event could fire

...the load event couldn't fire, because the load event is queued as a 
task, which will execute at the earliest _after_ the synchronous section 
has run (unless the task was queued before the task in which the resource 
selection algorithm was invoked started, of course). I don't think there's 
anything defined that delays the load event which can finish without a 
task executing. (And even if there was, moving this particular step around 
wouldn't change things other than theoretically making the race condition 
window a bit smaller.)

--

-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'
(Continue reading)

Boris Zbarsky | 2 Aug 17:10 2010
Picon

Re: Race condition in media load algorithm

On 8/2/10 5:17 AM, Ian Hickson wrote:
>> There's a race condition in the media load algorithm. When the resource
>> selection algorithm begins, it sets a task to complete the rest of the
>> resource selection algorithm asynchronously.
>
> Not quite. It awaits a stable state and then runs a synchronous section,
> which means that it will run the subsequent steps as soon as the current
> task has finished, before anything else that is queued.

So the model is that there are asynchronous tasks but there are also 
things that run after the "current asynchronous task" finishes?

How are we defining "current asynchronous task" and "finish"?  How does 
this setup handle cases when the "current asynchronous task" spins the 
event loop, if at all?

-Boris

Ian Hickson | 2 Aug 21:11 2010
Picon

Re: Race condition in media load algorithm

On Mon, 2 Aug 2010, Boris Zbarsky wrote:
> 
> So the model is that there are asynchronous tasks but there are also 
> things that run after the "current asynchronous task" finishes?

It's a little more detailed than that, but yes, that describes the event 
loop model.

> How are we defining "current asynchronous task" and "finish"?

In terms of the event loop algorithm:

   http://www.whatwg.org/specs/web-apps/current-work/complete/webappapis.html#processing-model-2

> How does this setup handle cases when the "current asynchronous task" 
> spins the event loop, if at all?

The spinning of the event loop is also defined in terms of the event loop 
in a manner that strictly defines this:

   http://www.whatwg.org/specs/web-apps/current-work/complete/webappapis.html#spin-the-event-loop

--

-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'

Boris Zbarsky | 2 Aug 21:39 2010
Picon

Re: Race condition in media load algorithm

On 8/2/10 3:11 PM, Ian Hickson wrote:
>> How are we defining "current asynchronous task" and "finish"?
>
> In terms of the event loop algorithm:
>
>     http://www.whatwg.org/specs/web-apps/current-work/complete/webappapis.html#processing-model-2

Ah, I see.  Is the note there about synchronous sections having no 
side-effects something that is true by default due to the way 
synchronous sections are used in the specification?

>> How does this setup handle cases when the "current asynchronous task"
>> spins the event loop, if at all?
>
> The spinning of the event loop is also defined in terms of the event loop
> in a manner that strictly defines this:
>
>     http://www.whatwg.org/specs/web-apps/current-work/complete/webappapis.html#spin-the-event-loop

I'm not sure I follow the steps here, actually.  Just to make sure I do 
understand....

Say I have a task T in the event queue.  Task T begins some algorithm 
that has a synchronous section, then spins the event loop.  If I 
understand the steps in #processing-model-2 correctly, the synchronous 
sections would run after task T completes, not while task T is spinning 
the event loop?  Or does "stop the currently running task" in 
#spin-the-event-loop imply a jump to step 2 of the algorithm under 
#processing-model2?

(Continue reading)

Ian Hickson | 2 Aug 23:20 2010
Picon

Re: Race condition in media load algorithm

On Mon, 2 Aug 2010, Boris Zbarsky wrote:
> On 8/2/10 3:11 PM, Ian Hickson wrote:
> > > How are we defining "current asynchronous task" and "finish"?
> > 
> > In terms of the event loop algorithm:
> > 
> >     http://www.whatwg.org/specs/web-apps/current-work/complete/webappapis.html#processing-model-2
> 
> Ah, I see.  Is the note there about synchronous sections having no 
> side-effects something that is true by default due to the way 
> synchronous sections are used in the specification?

Yeah. It's intended to help keep the ways to implement this somewhat open.

> > > How does this setup handle cases when the "current asynchronous task"
> > > spins the event loop, if at all?
> > 
> > The spinning of the event loop is also defined in terms of the event loop
> > in a manner that strictly defines this:
> > 
> >     http://www.whatwg.org/specs/web-apps/current-work/complete/webappapis.html#spin-the-event-loop
> 
> I'm not sure I follow the steps here, actually.  Just to make sure I do
> understand....
> 
> Say I have a task T in the event queue.  Task T begins some algorithm that has
> a synchronous section, then spins the event loop. If I understand the steps
> in #processing-model-2 correctly, the synchronous sections would run after
> task T completes, not while task T is spinning the event loop?

(Continue reading)

Boris Zbarsky | 3 Aug 06:13 2010
Picon

Re: Race condition in media load algorithm

On 8/2/10 5:20 PM, Ian Hickson wrote:
>> Or does "stop the currently running task" in #spin-the-event-loop imply
>> a jump to step 2 of the algorithm under #processing-model2?
>
> Yes.

OK, that might be worth clarifying.

>> (Note: I still have a problem with the way "pause" is defined here, but
>> I've raised that before, I believe.)
>
> I think we all have a problem with "pause", but I don't know what we can
> do about it. I don't have any pending feedback from you on this topic, as
> far as I can tell.

Odd.  I definitely sent something about it (in particular that it 
doesn't seem very easily implementable in terms of network event 
behavior, if the pause is waiting on a network event and other network 
events need to not be delivered in the meantime).

-Boris

Chris Pearce | 4 Aug 01:18 2010
Picon

Re: Race condition in media load algorithm

  On 3/08/2010 9:20 a.m., Ian Hickson wrote:
>
> The synchronous section would run as soon as the task span the event loop.
> Spinning the event loop is defined essentially as being equivalent to
> breaking the original task in two, one that does everything up to spinning
> the event loop, and one that does everything after spinning the event
> loop. You are effectively waiting for some condition to become true and
> then queueing a task to run the continuation of the algorithm. It's just
> more convenient to write the algorithms as one long set of steps rather
> than having split them up into multiple algorithms that invoke each other
> and pass state around.
>
>> Or does "stop the currently running task" in #spin-the-event-loop imply
>> a jump to step 2 of the algorithm under #processing-model2?
> Yes.
>

So the task spinning the event loop has run up to the point of spinning 
the event loop, then we run the synchronous section, and then the task 
which was spinning the event loop will resume after its goal is met at 
some later time?

In the case of a tasks which invokes an algorithm which has a 
synchronous section, and then pauses the event loop (such calling 
window.alert()), we should not run the synchronous section until the 
event loop pause has completed?

Thanks,
Chris Pearce.

(Continue reading)

Ian Hickson | 4 Aug 01:32 2010
Picon

Re: Race condition in media load algorithm

On Wed, 4 Aug 2010, Chris Pearce wrote:
> 
> So the task spinning the event loop has run up to the point of spinning 
> the event loop, then we run the synchronous section, and then the task 
> which was spinning the event loop will resume after its goal is met at 
> some later time?

Sounds right.

> In the case of a tasks which invokes an algorithm which has a 
> synchronous section, and then pauses the event loop (such calling 
> window.alert()), we should not run the synchronous section until the 
> event loop pause has completed?

Currently, yeah. We might want to make "pause" also trigger synchronous 
sections, if that's a problem.

--

-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'

Chris Pearce | 4 Aug 01:52 2010
Picon

Re: Race condition in media load algorithm

  On 4/08/2010 11:32 a.m., Ian Hickson wrote:
>
>> In the case of a tasks which invokes an algorithm which has a
>> synchronous section, and then pauses the event loop (such calling
>> window.alert()), we should not run the synchronous section until the
>> event loop pause has completed?
> Currently, yeah. We might want to make "pause" also trigger synchronous
> sections, if that's a problem.
>

Having "pause" trigger synchronous sections is definitely easier for us 
to implement, and would make pausing the event loop consistent with 
spinning the event loop WRT synchronous sections.

Chris P.

Chris Pearce | 3 Aug 00:40 2010
Picon

Re: Race condition in media load algorithm

  On 2/08/2010 9:17 p.m., Ian Hickson wrote:
> On Mon, 2 Aug 2010, Chris Pearce wrote:
>> There's a race condition in the media load algorithm. When the resource
>> selection algorithm begins, it sets a task to complete the rest of the
>> resource selection algorithm asynchronously.
> Not quite. It awaits a stable state and then runs a synchronous section,
> which means that it will run the subsequent steps as soon as the current
> task has finished, before anything else that is queued.
>

Ah, I see. My understanding of the processing model was incomplete, 
thanks for clarifying!

All the best,
Chris Pearce.

Philip Jägenstedt | 3 Aug 10:27 2010
Picon

Re: Race condition in media load algorithm

On Tue, 03 Aug 2010 00:40:26 +0200, Chris Pearce <chris@...>  
wrote:

>   On 2/08/2010 9:17 p.m., Ian Hickson wrote:
>> On Mon, 2 Aug 2010, Chris Pearce wrote:
>>> There's a race condition in the media load algorithm. When the resource
>>> selection algorithm begins, it sets a task to complete the rest of the
>>> resource selection algorithm asynchronously.
>> Not quite. It awaits a stable state and then runs a synchronous section,
>> which means that it will run the subsequent steps as soon as the current
>> task has finished, before anything else that is queued.
>>
>
> Ah, I see. My understanding of the processing model was incomplete,  
> thanks for clarifying!

For the record, here's how I interpreted "await a stable state":

The only state that is not stable is a running script. The only step in  
any video-related algorithm one can reach from a script is step 2 of the  
resource selection algorithm. Therefore, when reaching that step, if the  
resource selection algorithm was triggered by a script, wait until that  
script has finished and then continue. The only somewhat tricky part is  
that if we are in an event handler triggered by script, we should wait  
until the script that triggered the event handler has finished. The only  
way I know of triggering this corner case is by invoking a synchronous  
event handler from script, e.g. by calling click().

All other occurrences of "await a stable state" I've ignored as we can't  
not be in a stable state when reaching them.
(Continue reading)

Boris Zbarsky | 3 Aug 17:40 2010
Picon

Re: Race condition in media load algorithm

On 8/3/10 4:27 AM, Philip Jägenstedt wrote:
> For the record, here's how I interpreted "await a stable state":
>
> The only state that is not stable is a running script.

I don't think that's true; for example you could be in an unstable state 
if you're in the middle of the parser inserting some nodes into the DOM.

> Therefore, when reaching that step, if the
> resource selection algorithm was triggered by a script, wait until that
> script has finished and then continue.

Per spec as currently written, this will give the wrong behavior.  For 
example, if the script in question calls showModalDialog, this is 
supposed to interrupt the script task and spin the event loop, and so 
your synchronous section would need to run while the modal dialog is up. 
  It doesn't sound like your implementation does that.

-Boris

Philip Jägenstedt | 4 Aug 10:29 2010
Picon

Re: Race condition in media load algorithm

On Tue, 03 Aug 2010 17:40:33 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/3/10 4:27 AM, Philip Jägenstedt wrote:
>> For the record, here's how I interpreted "await a stable state":
>>
>> The only state that is not stable is a running script.
>
> I don't think that's true; for example you could be in an unstable state  
> if you're in the middle of the parser inserting some nodes into the DOM.

If the parser running in considered an unstable state, then we would have  
to wait until the whole document has finished parsing before running the  
resource selection algorithm. Thus, <video> would be unnecessarily delayed  
in a way that <img> isn't. The spec is certainly written in a way that  
makes good sense when triggering the resource selection algorithm when the  
parser first sets a src attribute or appends a child source element and  
then waiting in NETWORK_NO_SOURCE for more source elements. If this isn't  
the intention, I have really completely misunderstood what the spec is  
trying to do. Whatever the language used, I suggest letting the parser  
trigger the resource selection algorithm synchronously.

>> Therefore, when reaching that step, if the
>> resource selection algorithm was triggered by a script, wait until that
>> script has finished and then continue.
>
> Per spec as currently written, this will give the wrong behavior.  For  
> example, if the script in question calls showModalDialog, this is  
> supposed to interrupt the script task and spin the event loop, and so  
> your synchronous section would need to run while the modal dialog is up.  
>   It doesn't sound like your implementation does that.
(Continue reading)

Boris Zbarsky | 4 Aug 11:32 2010
Picon

Re: Race condition in media load algorithm

On 8/4/10 4:29 AM, Philip Jägenstedt wrote:
> That could be, but is this behavior actually useful for anything? It's
> certainly simpler to implement and more predictable for authors to
> always wait until the current script has finished executing.

1)  That requires defining "current script".
2)  Who said it will ever finish executing?

-Boris

Philip Jägenstedt | 4 Aug 12:56 2010
Picon

Re: Race condition in media load algorithm

On Wed, 04 Aug 2010 11:32:51 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/4/10 4:29 AM, Philip Jägenstedt wrote:
>> That could be, but is this behavior actually useful for anything? It's
>> certainly simpler to implement and more predictable for authors to
>> always wait until the current script has finished executing.
>
> 1)  That requires defining "current script".

OK, but that's just a spec problem. It's trivial in implementation because  
when the resource selection algorithm was is triggered by a script, you  
can just pass along a reference to that very script.

> 2)  Who said it will ever finish executing?

If it doesn't, just don't ever continue with the synchronous section. Is  
there any valid case for a script never finishing? It would block all  
event handlers from running too, so in any case it's a broken page.

--

-- 
Philip Jägenstedt
Core Developer
Opera Software

Boris Zbarsky | 4 Aug 19:19 2010
Picon

Re: Race condition in media load algorithm

On 8/4/10 6:56 AM, Philip Jägenstedt wrote:
> On Wed, 04 Aug 2010 11:32:51 +0200, Boris Zbarsky <bzbarsky@...> wrote:
>
>> On 8/4/10 4:29 AM, Philip Jägenstedt wrote:
>>> That could be, but is this behavior actually useful for anything? It's
>>> certainly simpler to implement and more predictable for authors to
>>> always wait until the current script has finished executing.
>>
>> 1) That requires defining "current script".
>
> OK, but that's just a spec problem. It's trivial in implementation
> because when the resource selection algorithm was is triggered by a
> script, you can just pass along a reference to that very script.

It's not, in fact, trivial in implementation.  You're making assumptions 
about how implementations work that don't seem warranted (e.g. the 
concept of "reference to that very script" is not well-defined in some 
implementations).  In particular, what you're proposing is not at all 
trivial in Gecko.

>> 2) Who said it will ever finish executing?
>
> If it doesn't, just don't ever continue with the synchronous section.

I don't think that's reasonable.

> Is there any valid case for a script never finishing?

Yes, it could showModalDialog and the user could spend several hours 
interacting with it.
(Continue reading)

Philip Jägenstedt | 5 Aug 11:14 2010
Picon

Re: Race condition in media load algorithm

On Wed, 04 Aug 2010 19:19:35 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/4/10 6:56 AM, Philip Jägenstedt wrote:
>> On Wed, 04 Aug 2010 11:32:51 +0200, Boris Zbarsky <bzbarsky@...>  
>> wrote:
>>
>>> On 8/4/10 4:29 AM, Philip Jägenstedt wrote:
>>>> That could be, but is this behavior actually useful for anything? It's
>>>> certainly simpler to implement and more predictable for authors to
>>>> always wait until the current script has finished executing.
>>>
>>> 1) That requires defining "current script".
>>
>> OK, but that's just a spec problem. It's trivial in implementation
>> because when the resource selection algorithm was is triggered by a
>> script, you can just pass along a reference to that very script.
>
> It's not, in fact, trivial in implementation.  You're making assumptions  
> about how implementations work that don't seem warranted (e.g. the  
> concept of "reference to that very script" is not well-defined in some  
> implementations).  In particular, what you're proposing is not at all  
> trivial in Gecko.

 From <https://bugzilla.mozilla.org/show_bug.cgi?id=485288#c7> it sounds  
like Gecko actually has pretty much the same tools to work with as Opera:  
a nsIThreadObserver that allows you to wait until you've "finished the  
task in which some JS called load()". This is exactly what Opera does.

(It's difficult for Opera to rely on the message loop to resume the  
synchronous section, because our script engine can suspend and continue  
(Continue reading)

Boris Zbarsky | 5 Aug 17:22 2010
Picon

Re: Race condition in media load algorithm

On 8/5/10 5:14 AM, Philip Jägenstedt wrote:
>> It's not, in fact, trivial in implementation. You're making
>> assumptions about how implementations work that don't seem warranted
>> (e.g. the concept of "reference to that very script" is not
>> well-defined in some implementations). In particular, what you're
>> proposing is not at all trivial in Gecko.
>
>  From <https://bugzilla.mozilla.org/show_bug.cgi?id=485288#c7> it sounds
> like Gecko actually has pretty much the same tools to work with as
> Opera: a nsIThreadObserver that allows you to wait until you've
> "finished the task in which some JS called load()". This is exactly what
> Opera does.

No, what Gecko has is an nsIThreadObserver which is notified when an 
event is processed.  This as nothing with any particular event 
finishing, and will get called in the "pause" and "spin event loop" 
states that are defined in the spec.  It does not allow you to wait for 
a particular task to finish.

Furthemore, tasks do not map to "scripts" very well, either in Gecko or 
in general.

> (It's difficult for Opera to rely on the message loop to resume the
> synchronous section, because our script engine can suspend and continue
> executing on a later message. Therefore, waiting for a script to finish
> executing is the best solution.)

See, this concept of "a script" is a funny one, given that scripts are 
reentrant, and that multiple different scripts can all be running at the 
same time, possibly with event loops nested in between on the conceptual 
(Continue reading)

Eric Carlson | 5 Aug 20:24 2010
Picon

Re: Race condition in media load algorithm


On Aug 5, 2010, at 8:22 AM, Boris Zbarsky wrote:

> In practice, what Gecko would likely do here is to treat "stable state" as "the event loop is spinning",
just like we would for the other case.  This means that while a modal dialog is up, or a sync XHR is running or
whatnot is a stable state.
> 

  FWIW this is what WebKit does now, assuming you mean *asynch* XHR.

eric

Boris Zbarsky | 5 Aug 20:34 2010
Picon

Re: Race condition in media load algorithm

On 8/5/10 2:24 PM, Eric Carlson wrote:
>
> On Aug 5, 2010, at 8:22 AM, Boris Zbarsky wrote:
>
>> In practice, what Gecko would likely do here is to treat "stable state" as "the event loop is spinning",
just like we would for the other case.  This means that while a modal dialog is up, or a sync XHR is running or
whatnot is a stable state.
>>
>
>    FWIW this is what WebKit does now, assuming you mean *asynch* XHR.

No, I meant what I said.  Async XHR is clearly going to go back to the 
event loop, etc.  In Gecko, sync XHR does as well, by manually spinning 
the event loop under the Send() call (and blocking delivery of certain 
classes of events to the web page that triggered the send).

This isn't at all like the "pause" state the spec describes, which I 
also mentioned.

On the other hand, I think the spec's "pause" state can have some really 
undesirable interactions whereby a sync XHR on one page can affect 
behavior of other pages, which is why I have problems with it...

-Boris

Philip Jägenstedt | 6 Aug 10:04 2010
Picon

Re: Race condition in media load algorithm

On Thu, 05 Aug 2010 17:22:17 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/5/10 5:14 AM, Philip Jägenstedt wrote:
>>> It's not, in fact, trivial in implementation. You're making
>>> assumptions about how implementations work that don't seem warranted
>>> (e.g. the concept of "reference to that very script" is not
>>> well-defined in some implementations). In particular, what you're
>>> proposing is not at all trivial in Gecko.
>>
>>  From <https://bugzilla.mozilla.org/show_bug.cgi?id=485288#c7> it sounds
>> like Gecko actually has pretty much the same tools to work with as
>> Opera: a nsIThreadObserver that allows you to wait until you've
>> "finished the task in which some JS called load()". This is exactly what
>> Opera does.
>
> No, what Gecko has is an nsIThreadObserver which is notified when an  
> event is processed.  This as nothing with any particular event  
> finishing, and will get called in the "pause" and "spin event loop"  
> states that are defined in the spec.  It does not allow you to wait for  
> a particular task to finish.
>
> Furthemore, tasks do not map to "scripts" very well, either in Gecko or  
> in general.

OK, fair enough.

>> (It's difficult for Opera to rely on the message loop to resume the
>> synchronous section, because our script engine can suspend and continue
>> executing on a later message. Therefore, waiting for a script to finish
>> executing is the best solution.)
(Continue reading)

Boris Zbarsky | 6 Aug 15:30 2010
Picon

Re: Race condition in media load algorithm

On 8/6/10 4:04 AM, Philip Jägenstedt wrote:
>> See, this concept of "a script" is a funny one, given that scripts are
>> reentrant, and that multiple different scripts can all be running at
>> the same time, possibly with event loops nested in between on the
>> conceptual callstack
>
> Well, what we really look at is an execution thread. When a script
> triggers a synchronous event handler or is otherwise suspended while
> waiting for another thread to finish, we will wait for the "outermost"
> suspended thread to finish executing. So, yeah, it's not completely
> trivial :)

That really doesn't sound like it maps well to the proposed HTML5 spec, 
for what it's worth...  Of course it sounds like you're suggesting the 
spec should be changed (without actually describing your changes in 
speccable non-implementation-specific terms).

>> Who said anything about infinite loops?
>
> It's one way of a script not finishing.

OK, sure.  But if a script s busy-looping, I think we all agree that's 
not considered a time when synchronous sections can be run.

>>> If a script is showing a modal dialog, not loading a video in the
>>> background seems fine.
>>
>> Why? It doesn't seem fine to me at all.
>
> My point is that no one uses modal dialogues
(Continue reading)

Philip Jägenstedt | 9 Aug 12:28 2010
Picon

Re: Race condition in media load algorithm

On Fri, 06 Aug 2010 15:30:42 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/6/10 4:04 AM, Philip Jägenstedt wrote:
>>> See, this concept of "a script" is a funny one, given that scripts are
>>> reentrant, and that multiple different scripts can all be running at
>>> the same time, possibly with event loops nested in between on the
>>> conceptual callstack
>>
>> Well, what we really look at is an execution thread. When a script
>> triggers a synchronous event handler or is otherwise suspended while
>> waiting for another thread to finish, we will wait for the "outermost"
>> suspended thread to finish executing. So, yeah, it's not completely
>> trivial :)
>
> That really doesn't sound like it maps well to the proposed HTML5 spec,  
> for what it's worth...  Of course it sounds like you're suggesting the  
> spec should be changed (without actually describing your changes in  
> speccable non-implementation-specific terms).

The "await a stable state" concept was introduced in  
<http://html5.org/tools/web-apps-tracker?from=3461&to=3462> and is used  
only for media elements. I can't remember any discussion about the change  
in async behavior at the time, so didn't think much of it. It's because  
Mozilla are implementing this now that the issue has surfaced, so let's  
try to find a solution that's implementable in all browsers. Let's focus  
on how this interacts with the parser, because that probably changes  
everything else to fall into place, see below.

>>> It's possible that it can't. It would depend on the exact steps that
>>> run in the synchronous section, but since synchronous sections can't
(Continue reading)

Boris Zbarsky | 9 Aug 14:16 2010
Picon

Re: Race condition in media load algorithm

On 8/9/10 6:28 AM, Philip Jägenstedt wrote:
> Let's focus on how this interacts with the parser, because that probably
> changes everything else to fall into place, see below.

OK.

> In the case of a script triggering it, it's important that the
> synchronous section not be run until the script has finished modifying
> the DOM.

Why?  Maybe if I understood what we're trying to accomplish with the 
synchronous section bit here I'd have a better idea of how it should work...

> If that's accomplished by waiting until the thread has finished
> or by means of the event loop isn't important as long as it's
> implementable and the result is 100% predictable.

Agreed so far.

> Modulo the effects of modal dialogs and other corner-cases, networkState should remain at
> NETWORK_NO_SOURCE for the remainder of the script that triggered
> resource selection.

I'd like to pin down why this is the case before even starting to sort 
out the parser behavior, since that depends on the exact goal we're 
trying to achieve.

-Boris

(Continue reading)

Philip Jägenstedt | 9 Aug 18:14 2010
Picon

<video> "await a stable state" in resource selection (Was: Race condition in media load algorithm)

This goes quite a bit from the original topic, so renaming accordingly.  
The issue at hand is the "await a stable state" concept in the resource  
selection algorithm:  
<http://www.whatwg.org/specs/web-apps/current-work/multipage/video.html#concept-media-load-algorithm>

On Mon, 09 Aug 2010 14:16:04 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/9/10 6:28 AM, Philip Jägenstedt wrote:
>> Let's focus on how this interacts with the parser, because that probably
>> changes everything else to fall into place, see below.
>
> OK.
>
>> In the case of a script triggering it, it's important that the
>> synchronous section not be run until the script has finished modifying
>> the DOM.
>
> Why?  Maybe if I understood what we're trying to accomplish with the  
> synchronous section bit here I'd have a better idea of how it should  
> work...

The general idea of waiting is that since the following steps of the  
resource selection algorithm inspects video elements src attribute and  
source element children, those should be in a consistent state before the  
checking is done. In particular, if a script is removing and inserting  
source elements, then after any given modification the DOM might not be in  
a state where the "right" source will be selected. For example:

<body>
<script>
(Continue reading)

Boris Zbarsky | 9 Aug 18:35 2010
Picon

Re: <video> "await a stable state" in resource selection (Was: Race condition in media load algorithm)

On 8/9/10 12:14 PM, Philip Jägenstedt wrote:
>> Why? Maybe if I understood what we're trying to accomplish with the
>> synchronous section bit here I'd have a better idea of how it should
>> work...
>
> The general idea of waiting is that since the following steps of the
> resource selection algorithm inspects video elements src attribute and
> source element children, those should be in a consistent state before
> the checking is done.

OK, but then why are we not imposing such a requirement for the case 
when the <video> is being created by the parser?

> <body>
> <script>
> var v = document.createElement('video');
> var exts=["webm", "mp4"];
> exts.forEach(function(ext) {
> var s = document.createElement('source');
> v.appendChild(s);
> s.src = "foo."+ext;
> s.type = "video/"+ext;
> document.body.appendChild(v);
> });
> </script>
>
> Unless we wait until the script has finished before running the
> synchronous section, no source at all will be selected

Because changes to the set of <source> elements do not restart the 
(Continue reading)

Philip Jägenstedt | 10 Aug 10:40 2010
Picon

Re: <video> "await a stable state" in resource selection (Was: Race condition in media load algorithm)

On Mon, 09 Aug 2010 18:35:47 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/9/10 12:14 PM, Philip Jägenstedt wrote:
>>> Why? Maybe if I understood what we're trying to accomplish with the
>>> synchronous section bit here I'd have a better idea of how it should
>>> work...
>>
>> The general idea of waiting is that since the following steps of the
>> resource selection algorithm inspects video elements src attribute and
>> source element children, those should be in a consistent state before
>> the checking is done.
>
> OK, but then why are we not imposing such a requirement for the case  
> when the <video> is being created by the parser?

Because the parser can't create a state which the algorithm doesn't  
handle. It always first inserts the video element, then the source  
elements in the order they should be evaluated. The algorithm is written  
in such a way that the overall result is the same regardless of whether it  
is invoked/continued on each inserted source element or after the video  
element is closed. However, scripts can see the state at any point, which  
is why it needs to be the same in all browsers.

>> <body>
>> <script>
>> var v = document.createElement('video');
>> var exts=["webm", "mp4"];
>> exts.forEach(function(ext) {
>> var s = document.createElement('source');
>> v.appendChild(s);
(Continue reading)

Boris Zbarsky | 10 Aug 18:39 2010
Picon

Re: <video> "await a stable state" in resource selection (Was: Race condition in media load algorithm)

On 8/10/10 4:40 AM, Philip Jägenstedt wrote:
> Because the parser can't create a state which the algorithm doesn't
> handle. It always first inserts the video element, then the source
> elements in the order they should be evaluated. The algorithm is written
> in such a way that the overall result is the same regardless of whether
> it is invoked/continued on each inserted source element or after the
> video element is closed.

Ah, the waiting state, etc?

Why does the algorithm not just reevaluate any sources after the 
newly-inserted source instead?

> However, scripts can see the state at any point, which is why it needs to be the same in all browsers.

I'm not sure which "the state" you mean here.

>> Because changes to the set of <source> elements do not restart the
>> resource selection algorithm, right? Why don't they, exactly? That
>> seems broken to me, from the POV of how the rest of the DOM generally
>> works (except as required by backward compatibility considerations)...
>
> The resource selection is only started once, typically when the src
> attribute is set (by parser or script) or when the first source element
> is inserted. If it ends up in step 21 waiting, inserting another source
> element may cause it to continue at step 22.

Right, ok.

> Restarting the algorithm on any modification of source elements would
(Continue reading)

Philip Jägenstedt | 11 Aug 11:13 2010
Picon

Re: <video> "await a stable state" in resource selection (Was: Race condition in media load algorithm)

CC Hixie, question below.

On Tue, 10 Aug 2010 18:39:04 +0200, Boris Zbarsky <bzbarsky@...> wrote:

> On 8/10/10 4:40 AM, Philip Jägenstedt wrote:
>> Because the parser can't create a state which the algorithm doesn't
>> handle. It always first inserts the video element, then the source
>> elements in the order they should be evaluated. The algorithm is written
>> in such a way that the overall result is the same regardless of whether
>> it is invoked/continued on each inserted source element or after the
>> video element is closed.
>
> Ah, the waiting state, etc?

Yes, in the case of the parser inserting source elements that fail one of  
the tests (no src, wrong type, wrong media) the algorithm will end up at  
step 6.21 waiting. It doesn't matter if all sources are available when the  
algorithm is first invoked or if they "trickle in", be that from the  
parser or from scripts.

> Why does the algorithm not just reevaluate any sources after the  
> newly-inserted source instead?

Because if a source failed after network access (404, wrong MIME, etc)  
then we'd have to perform that network access again and again for each  
modification. More on that below.

>> However, scripts can see the state at any point, which is why it needs  
>> to be the same in all browsers.
>
(Continue reading)

Boris Zbarsky | 11 Aug 21:14 2010
Picon

Re: <video> "await a stable state" in resource selection (Was: Race condition in media load algorithm)

On 8/11/10 5:13 AM, Philip Jägenstedt wrote:
> Yes, in the case of the parser inserting source elements that fail one
> of the tests (no src, wrong type, wrong media) the algorithm will end up
> at step 6.21 waiting. It doesn't matter if all sources are available
> when the algorithm is first invoked or if they "trickle in", be that
> from the parser or from scripts.

Right, ok.  Thanks for bearing with me on this!

> It's quite serious because NETWORK_EMPTY is used as a condition in many
> places of the spec, so this absolutely must be consistent between browsers.

OK, gotcha.

> Otherwise the pointer could potentially reach the same source element
> twice, with the aforementioned problems with failing after network access.

But this would only happen in a rare case, right?  Specifically, that of 
source elements being inserted out of order...  And if the update to new 
source elements being inserted were fully async, it would only happen if 
you trickle the source elements in out of order.

But yes, your idea of flagging a source when it fails would also address 
this just fine.

> OK, perhaps I should take this more seriously. Making the whole
> algorithm synchronous probably isn't a brilliant idea unless we can also
> do away with all of the state it keeps (i.e. hysteresis).

State is not necessarily a problem if it's invalidated as needed....
(Continue reading)

Ian Hickson | 24 Aug 10:24 2010
Picon

<video> loading algorithms

Silvia wrote:
>
> [...] All other browsers set the  <at> networkState to NETWORK_EMPTY at the 
> start of loading a new media resource (Firefox, Safari and Chrome). I 
> was going to discuss this with you, since Opera interprets the spec 
> different here - which is understandable, since the first step in the 
> resource selection algorithm says to:
>
> 1. Set the networkState to NETWORK_NO_SOURCE.

The other browsers implemented the first draft, which was then changed 
based on implementation feedback. Hopefully the other browsers will update 
their implementation now to fix the bugs that were present in the original 
version of the spec, leading to everyone doing the same thing. :-)

> OTOH however, the description for the [NETWORK_EMPTY] state is as 
> follows:
>
> NETWORK_EMPTY (numeric value 0)
> The element has not yet been initialized. All attributes are in their 
> initial states.
>
> and for NETWORK_NO_SOURCE is:
>
> NETWORK_NO_SOURCE (numeric value 3)
> The element's resource selection algorithm is active, but it has failed 
> to find a resource to use.
> 
> Thus, I believe this may be a spec bug and really the first step in the 
> resource selection algorithm should say
(Continue reading)

Philip Jägenstedt | 24 Aug 11:43 2010
Picon

Re: <video> loading algorithms

On Tue, 24 Aug 2010 10:24:35 +0200, Ian Hickson <ian@...> wrote:

> On Mon, 26 Jul 2010, Philip Jägenstedt wrote:
>>
>> Looking again at the resource selection algorithm, the second step is to
>> await a stable state, i.e. wait until the current script has finished.
>
> Not necessarily finished. The idea is just that the resource selection
> algorithm might run on another thread, and the way the spec is written
> allows the UA to synchronise at very predictable points (e.g. when
> showModalDialag() is invoked, or when the HTML parser isn't inserting
> elements, or while waiting for a style sheet to load so that a <script>
> can be run).

OK, I hadn't really understood "await a stable state".

>> Given that, it wouldn't be a big problem to let modification of src
>> attributes on source elements trigger resource selection, you won't get
>> the 3-2-1 problem I mentioned earlier.
>
> I don't really understand what use case this would solve.

Neither do I, I'm just saying that it's easy to do if necessary.

> On Tue, 3 Aug 2010, Philip Jägenstedt wrote:
>>
>> For the record, here's how I interpreted "await a stable state":
>>
>> The only state that is not stable is a running script.
>
(Continue reading)

Silvia Pfeiffer | 25 Aug 16:47 2010
Picon

Re: <video> loading algorithms

On Tue, Aug 24, 2010 at 7:43 PM, Philip Jägenstedt <philipj-4vTceeIZeJ0AvxtiuMwx3w@public.gmane.org> wrote:

On Tue, 24 Aug 2010 10:24:35 +0200, Ian Hickson <ian-Y1JINVRCvcs@public.gmane.org> wrote:


Given that, it wouldn't be a big problem to let modification of src
attributes on source elements trigger resource selection, you won't get
the 3-2-1 problem I mentioned earlier.

I don't really understand what use case this would solve.

Neither do I, I'm just saying that it's easy to do if necessary.

I was running through all the properties that can possibly be changed in JavaScript and checking which effects they have in all browsers, i.e. what events they call etc. I wasn't aware that authors are discouraged from changing <at> src on <source>.


On Tue, Aug 24, 2010 at 6:24 PM, Ian Hickson <ian-Y1JINVRCvcs@public.gmane.org> wrote:

On Sat, 24 Jul 2010, Silvia Pfeiffer wrote:
>
> There is definitely a spec bug, because different locations of the spec
> say diverging things.

For the record, when the spec contradicts itself, and one part is
normative (such as the algorithm here) and another is non-normative (such
as the definitions here) then always assume the normative part is wrong. I
often forget to update the non-normative parts.

You mean: the normative part is right, I assume?
Thanks for this clarification - I was indeed unaware of the history of changes.
 

On Mon, 26 Jul 2010, Silvia Pfeiffer wrote:
>
> I now wonder about the intention of play() (and also of pause()). As I
> understood it, they are both meant to reload the media resource if
> <at> currentSrc has changed, similar to what load() is supposed to do.

I do not believe that has ever been the intent.


Both descriptions of play() and pause() have a "loading the media resource" in them.
media . play()

Sets the paused attribute to false, loading the media resource and beginning playback if necessary. If the playback had ended, will restart it from the start.

media . pause()

Sets the paused attribute to true, loading the media resource if necessary.

I assumed that if the <at> currentSrc had changed, that would trigger loading the new media resource, too. If that is not the intention, maybe the non-normative description should point out that it only triggers loading the media resource when the <at> src attribute is being set for the very first time. Though, to be honest, I don't really see a difference between setting <at> src through JavaScript for the first time (which IIUC also has a NETWORK_EMPTY state) and re-setting it again a subsequent time - IMHO both should consistently either include the media resource loading or exclude it.


On Tue, 27 Jul 2010, Silvia Pfeiffer wrote:
>
> Sure, but this is only a snippet of an actual application. If, e.g., you
> want to step through a list of videos (maybe an automated playlist)
> using script and you need to provide at least two different formats with
> <source>, you'd want to run this algorithm frequently.

Just have a bunch of <video>s in the markup, and when one ends, hide it
and show the next one. Don't start dynamically manipulating <source>
elements, that's just asking for pain.

If you really must do it all using script, just use canPlayType and the
<video src=""> attribute, don't mess around with <source>.

Thanks for adding that advice. I think it's important to point that out.

Cheers,
Silvia.

Ian Hickson | 8 Dec 23:29 2010
Picon

Re: <video> loading algorithms

On Tue, 3 Aug 2010, Boris Zbarsky wrote:
> On 8/2/10 5:20 PM, Ian Hickson wrote:
> > > Or does "stop the currently running task" in #spin-the-event-loop 
> > > imply a jump to step 2 of the algorithm under #processing-model2?
> > 
> > Yes.
> 
> OK, that might be worth clarifying.

Done.

> > > (Note: I still have a problem with the way "pause" is defined here, 
> > > but I've raised that before, I believe.)
> > 
> > I think we all have a problem with "pause", but I don't know what we 
> > can do about it. I don't have any pending feedback from you on this 
> > topic, as far as I can tell.
> 
> Odd.  I definitely sent something about it (in particular that it 
> doesn't seem very easily implementable in terms of network event 
> behavior, if the pause is waiting on a network event and other network 
> events need to not be delivered in the meantime).

The "pause" thing is never invoked when waiting for network behaviour on 
the same event loop. (That wouldn't work, as you point out.)

On Wed, 4 Aug 2010, Chris Pearce wrote:
> On 4/08/2010 11:32 a.m., Ian Hickson wrote:
> > 
> > > In the case of a tasks which invokes an algorithm which has a 
> > > synchronous section, and then pauses the event loop (such calling 
> > > window.alert()), we should not run the synchronous section until the 
> > > event loop pause has completed?
> >
> > Currently, yeah. We might want to make "pause" also trigger 
> > synchronous sections, if that's a problem.
> 
> Having "pause" trigger synchronous sections is definitely easier for us 
> to implement, and would make pausing the event loop consistent with 
> spinning the event loop WRT synchronous sections.

Ok, done.

On Wed, 4 Aug 2010, Philip Jägenstedt wrote:
> On Tue, 03 Aug 2010 17:40:33 +0200, Boris Zbarsky <bzbarsky@...> 
> wrote:
> > On 8/3/10 4:27 AM, Philip Jägenstedt wrote:
> > > For the record, here's how I interpreted "await a stable state":
> > > 
> > > The only state that is not stable is a running script.
> > 
> > I don't think that's true; for example you could be in an unstable 
> > state if you're in the middle of the parser inserting some nodes into 
> > the DOM.
> 
> If the parser running in considered an unstable state, then we would 
> have to wait until the whole document has finished parsing before 
> running the resource selection algorithm.

No, the parser is defined in terms of tasks and its interaction with the 
event loop (including what is a stable state) is defined.

> Thus, <video> would be unnecessarily delayed in a way that <img> isn't.

Actually <img> waits for much the same thing. :-) (Its rendering is 
updated when the event loop spins, which is around the same time as what 
is called a "stable state".)

> > > Therefore, when reaching that step, if the resource selection 
> > > algorithm was triggered by a script, wait until that script has 
> > > finished and then continue.
> > 
> > Per spec as currently written, this will give the wrong behavior.  
> > For example, if the script in question calls showModalDialog, this is 
> > supposed to interrupt the script task and spin the event loop, and so 
> > your synchronous section would need to run while the modal dialog is 
> > up.  It doesn't sound like your implementation does that.
> 
> That could be, but is this behavior actually useful for anything? It's 
> certainly simpler to implement and more predictable for authors to 
> always wait until the current script has finished executing.

Consider trying to play a sound in the background while the dialog is up, 
for instance.

On Tue, 24 Aug 2010, Philip Jägenstedt wrote:
>
> [Step 3 of the Otherwise clause at the end of the "An end tag whose tag 
> name is "script"" section of The "text" insertion mode.]
> 
> Should this step be read as
> 
> while (there is no style sheet blocking scripts and the script's "ready to be
> parser-executed" flag is set) {
>  Spin the event loop
> }
> 
> or
> 
> do {
>  Spin the event loop
> } while (there is no style sheet blocking scripts and the script's "ready to
> be parser-executed" flag is set)

It was meant to be the former. I've fixed it.

> In other words, will the synchronous section always be executed?

Not in between one <script> and a <script> that was document.write()ten by 
that script, no, not always.

> I think I meant to say that it if the synchronous section has run 
> depends on "whether or not the parser happened to return to the event 
> loop before the script". In other words, networkState could be 
> NETWORK_NO_SOURCE, NETWORK_EMPTY or NETWORK_LOADING here. Hopefully all 
> my conclusions are incorrect and there's actually a guarantee that the 
> synchronous sections runs before any scripts execute, see above.

There is not such a guarantee.

On Thu, 26 Aug 2010, Silvia Pfeiffer wrote:
> On Tue, Aug 24, 2010 at 6:24 PM, Ian Hickson <ian@...> wrote:
> > On Sat, 24 Jul 2010, Silvia Pfeiffer wrote:
> > >
> > > There is definitely a spec bug, because different locations of the 
> > > spec say diverging things.
> >
> > For the record, when the spec contradicts itself, and one part is 
> > normative (such as the algorithm here) and another is non-normative 
> > (such as the definitions here) then always assume the normative part 
> > is wrong. I often forget to update the non-normative parts.
> 
> You mean: the normative part is right, I assume?

Er, yes! Oops.

> On Mon, 26 Jul 2010, Silvia Pfeiffer wrote:
> > >
> > > I now wonder about the intention of play() (and also of pause()). As 
> > > I understood it, they are both meant to reload the media resource if 
> > >  <at> currentSrc has changed, similar to what load() is supposed to do.
> >
> > I do not believe that has ever been the intent.
> 
> Both descriptions of play() and pause() have a "loading the media 
> resource" in them.

Yes, but not for the purpose of reloading, only for the purpose of loading 
the resource in the first place.

> I assumed that if the  <at> currentSrc had changed, that would trigger 
> loading the new media resource, too. If that is not the intention, maybe 
> the non-normative description should point out that it only triggers 
> loading the media resource when the  <at> src attribute is being set for the 
> very first time.

Well that's not quite accurate either... it does it if the networkState is 
NETWORK_EMPTY, which can happen in a variety of conditions. I'm open to 
changing this text (from "if necessary" which is what it says now) but I 
really don't know what to change it to that would be both accurate and 
helpful.

> Though, to be honest, I don't really see a difference between setting 
>  <at> src through JavaScript for the first time (which IIUC also has a 
> NETWORK_EMPTY state) and re-setting it again a subsequent time - IMHO 
> both should consistently either include the media resource loading or 
> exclude it.

The difference is that the first time there's nothing loaded, so to play() 
anything you have to load it, whereas the second time there's already 
something loaded, which play() can just play straight away without having 
to load anything first.

> On Tue, 27 Jul 2010, Silvia Pfeiffer wrote:
> > >
> > > Sure, but this is only a snippet of an actual application. If, e.g., 
> > > you want to step through a list of videos (maybe an automated 
> > > playlist) using script and you need to provide at least two 
> > > different formats with <source>, you'd want to run this algorithm 
> > > frequently.
> >
> > Just have a bunch of <video>s in the markup, and when one ends, hide 
> > it and show the next one. Don't start dynamically manipulating 
> > <source> elements, that's just asking for pain.
> >
> > If you really must do it all using script, just use canPlayType and 
> > the <video src=""> attribute, don't mess around with <source>.
> 
> Thanks for adding that advice. I think it's important to point that out.

I can add it to the spec too if you think that would help. Where would a 
good place for it be?

--

-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'
Silvia Pfeiffer | 9 Dec 01:12 2010
Picon

Re: <video> loading algorithms

>> On Mon, 26 Jul 2010, Silvia Pfeiffer wrote:
>> > >
>> > > I now wonder about the intention of play() (and also of pause()). As
>> > > I understood it, they are both meant to reload the media resource if
>> > >  <at> currentSrc has changed, similar to what load() is supposed to do.
>> >
>> > I do not believe that has ever been the intent.
>>
>> Both descriptions of play() and pause() have a "loading the media
>> resource" in them.
>
> Yes, but not for the purpose of reloading, only for the purpose of loading
> the resource in the first place.
>
>
>> I assumed that if the  <at> currentSrc had changed, that would trigger
>> loading the new media resource, too. If that is not the intention, maybe
>> the non-normative description should point out that it only triggers
>> loading the media resource when the  <at> src attribute is being set for the
>> very first time.
>
> Well that's not quite accurate either... it does it if the networkState is
> NETWORK_EMPTY, which can happen in a variety of conditions. I'm open to
> changing this text (from "if necessary" which is what it says now) but I
> really don't know what to change it to that would be both accurate and
> helpful.
>
>
>> Though, to be honest, I don't really see a difference between setting
>>  <at> src through JavaScript for the first time (which IIUC also has a
>> NETWORK_EMPTY state) and re-setting it again a subsequent time - IMHO
>> both should consistently either include the media resource loading or
>> exclude it.
>
> The difference is that the first time there's nothing loaded, so to play()
> anything you have to load it, whereas the second time there's already
> something loaded, which play() can just play straight away without having
> to load anything first.
>
>
>> On Tue, 27 Jul 2010, Silvia Pfeiffer wrote:
>> > >
>> > > Sure, but this is only a snippet of an actual application. If, e.g.,
>> > > you want to step through a list of videos (maybe an automated
>> > > playlist) using script and you need to provide at least two
>> > > different formats with <source>, you'd want to run this algorithm
>> > > frequently.
>> >
>> > Just have a bunch of <video>s in the markup, and when one ends, hide
>> > it and show the next one. Don't start dynamically manipulating
>> > <source> elements, that's just asking for pain.
>> >
>> > If you really must do it all using script, just use canPlayType and
>> > the <video src=""> attribute, don't mess around with <source>.
>>
>> Thanks for adding that advice. I think it's important to point that out.
>
> I can add it to the spec too if you think that would help. Where would a
> good place for it be?

There is a note in the <source> element section that reads as follows:
"Dynamically modifying a source element and its attribute when the
element is already inserted in a video or audio element will have no
effect. To change what is playing, either just use the src attribute
on the media element directly, or call the load() method on the media
element after manipulating the source elements."

Maybe you can add some advice there to use canPlayType to identify
what type of resource to add in the  <at> src attribute on the media
element. Also, you should remove the last half of the second sentence
in this note if that is not something we'd like to encourage.

Cheers,
Silvia.


Gmane