Bug 106587

Summary: HTMLMediaElement::resume() should schedule a load rather than load immediately
Product: WebKit Reporter: Yong Li <yong.li.webkit>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the proposed patch none

Description Yong Li 2013-01-10 11:44:13 PST
Generally, those resume() methods should only schedule the resumed jobs by timers. This is because:

1) It is not always safe to start the jobs at the time resume() is called. That is why the jobs are suspended.

For example, when ActiveDOMObjects are suspended for nested event loop, the sequence is like this:

- The stack is on top of document parsing, javascript execution, so it is not safe to run some jobs in the nested event loop.
- suspend all scheduled jobs so they won't run in the event loop
- run the nested event loop
- resume (re-schedule those jobs). However, it is still not safe at this point to run those jobs!
...
- Stack unwinds back to main event loop
....
- Timers fire and scheduled jobs get done.

Another case is PageCache. ActiveDOMObjects are suspended while a document is in the page cache. When it is being restored, resume() is called upon every ActiveDOMObject. However I don't think the page is guaranteed ready at this point to start the resumed jobs immediately.

2) It is also good for performance, as the objects may be suspended again after being resumed.
Comment 1 Yong Li 2013-01-10 12:00:37 PST
Created attachment 182186 [details]
the proposed patch
Comment 2 Yong Li 2013-01-10 12:03:22 PST
+Eric Carlson

Does the patch make sense?
Comment 3 Eric Carlson 2013-01-10 12:10:24 PST
Comment on attachment 182186 [details]
the proposed patch

This change looks good, but please wait until the bots have had a chance to exercise the change.
Comment 4 Yong Li 2013-01-10 12:10:43 PST
(In reply to comment #3)
> (From update of attachment 182186 [details])
> This change looks good, but please wait until the bots have had a chance to exercise the change.

Sure. thanks!
Comment 5 WebKit Review Bot 2013-01-10 14:33:14 PST
Comment on attachment 182186 [details]
the proposed patch

Clearing flags on attachment: 182186

Committed r139371: <http://trac.webkit.org/changeset/139371>
Comment 6 WebKit Review Bot 2013-01-10 14:33:18 PST
All reviewed patches have been landed.  Closing bug.