Bug 128976

Summary: Do not cache media time until media engine returns a non-zero value
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, gyuyoung.kim, jer.noble, philipj, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 128989    
Bug Blocks:    
Attachments:
Description Flags
Proposed patch. none

Description Eric Carlson 2014-02-18 08:40:40 PST
It can sometimes take a media engine a non-insignificant amount of time to begin playing after receiving a play command, so do not cache current time until the media engine returns a non-zero value.
Comment 1 Eric Carlson 2014-02-18 08:50:29 PST
Created attachment 224518 [details]
Proposed patch.
Comment 2 Jer Noble 2014-02-18 09:10:45 PST
Comment on attachment 224518 [details]
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=224518&action=review

r=me, with nits.

> Source/WebCore/ChangeLog:4
> +        Do not cache media time until media engine returns a non-zero value
> +        https://bugs.webkit.org/show_bug.cgi?id=128976

This doesn't say _why_ we shouldn't cache the media time in this case.

> Source/WebCore/html/HTMLMediaElement.cpp:2446
> +    if (m_cachedTime) {
> +        LOG(Media, "HTMLMediaElement::refreshCachedTime - caching time %f", m_cachedTime);
> +        m_clockTimeAtLastCachedTimeUpdate = monotonicallyIncreasingTime();
> +    } else
> +        invalidateCachedTime();

I would structure this a little differently, to be a bit more clear about what's going on:

if (!m_cachedTime) {
    // Short comment about why not caching a 0-currentTime is the right thing to do.
    invalidateCachedTime();
    return;
}

LOG(Media, "HTMLMediaElement::refreshCachedTime - caching time %f", m_cachedTime);
m_clockTimeAtLastCachedTimeUpdate = monotonicallyIncreasingTime();
Comment 3 Eric Carlson 2014-02-18 10:39:05 PST
Committed r164296: https://trac.webkit.org/r164296
Comment 4 WebKit Commit Bot 2014-02-18 11:56:38 PST
Re-opened since this is blocked by bug 128989
Comment 5 Eric Carlson 2014-02-18 14:19:46 PST
Resubmitted as https://trac.webkit.org/r164318.