Bug 134116

Summary: HTMLMediaElement seek algorithm should allow cancelling previous seeks.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, calvaris, commit-queue, eric.carlson, esprehn+autocc, glenn, gyuyoung.kim, ossy, philipj, rniwa, sergio, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=224363
Bug Depends on:    
Bug Blocks: 134253    
Attachments:
Description Flags
Patch
none
Patch
eric.carlson: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
none
Patch for landing.
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 none

Description Jer Noble 2014-06-20 13:25:24 PDT
HTMLMediaElement seek algorithm should allow cancelling previous seeks.
Comment 1 Jer Noble 2014-06-20 14:20:17 PDT
Created attachment 233454 [details]
Patch
Comment 2 WebKit Commit Bot 2014-06-20 14:21:17 PDT
Attachment 233454 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:429:  Missing space before {  [whitespace/braces] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Jer Noble 2014-06-20 14:26:39 PDT
Created attachment 233455 [details]
Patch
Comment 4 WebKit Commit Bot 2014-06-20 14:28:14 PDT
Attachment 233455 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:432:  Missing space before {  [whitespace/braces] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Eric Carlson 2014-06-20 14:29:21 PDT
Comment on attachment 233455 [details]
Patch

> Source/WebCore/html/HTMLMediaElement.h:736
> +    std::unique_ptr<PendingSeek> m_pendingSeek;

Since there can only be one of these, why not have "PendingSeek m_pendingSeek" so it doesn't need to be allocated each time?
Comment 6 Jer Noble 2014-06-20 14:52:10 PDT
Comment on attachment 233455 [details]
Patch

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

>> Source/WebCore/html/HTMLMediaElement.h:736
>> +    std::unique_ptr<PendingSeek> m_pendingSeek;
> 
> Since there can only be one of these, why not have "PendingSeek m_pendingSeek" so it doesn't need to be allocated each time?

Because there needs to be a way to distinguish between "I have a pending seek" and "I have no pending seeks". Otherwise, the ASSERT() in seekTimerFired wouldn't catch when an unexpected seek happened.
Comment 7 Build Bot 2014-06-20 16:04:42 PDT
Comment on attachment 233455 [details]
Patch

Attachment 233455 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6109625321521152

New failing tests:
media/video-seek-with-negative-playback.html
media/video-currentTime-set2.html
media/media-controller-playbackrate.html
media/video-played-ranges-1.html
media/video-fast-seek.html
media/video-ended-event-negative-playback.html
Comment 8 Build Bot 2014-06-20 16:04:46 PDT
Created attachment 233474 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 9 Build Bot 2014-06-20 16:16:29 PDT
Comment on attachment 233455 [details]
Patch

Attachment 233455 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5091649352892416

New failing tests:
media/video-seek-with-negative-playback.html
media/video-currentTime-set2.html
media/media-controller-playbackrate.html
media/video-played-ranges-1.html
media/video-fast-seek.html
media/video-ended-event-negative-playback.html
Comment 10 Build Bot 2014-06-20 16:16:34 PDT
Created attachment 233479 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 11 Build Bot 2014-06-21 00:51:28 PDT
Comment on attachment 233455 [details]
Patch

Attachment 233455 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4552547230547968

New failing tests:
media/video-seek-with-negative-playback.html
media/video-currentTime-set2.html
media/media-controller-playbackrate.html
media/video-played-ranges-1.html
media/video-fast-seek.html
media/video-ended-event-negative-playback.html
Comment 12 Build Bot 2014-06-21 00:51:32 PDT
Created attachment 233516 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 13 Jer Noble 2014-06-23 12:54:13 PDT
Created attachment 233629 [details]
Patch for landing.
Comment 14 Build Bot 2014-06-23 15:07:28 PDT
Comment on attachment 233629 [details]
Patch for landing.

Attachment 233629 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6743095953588224

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
Comment 15 Build Bot 2014-06-23 15:07:35 PDT
Created attachment 233638 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 16 Jer Noble 2014-06-23 17:09:17 PDT
Committed r170336: <http://trac.webkit.org/changeset/170336>
Comment 17 Csaba Osztrogonác 2014-06-24 04:22:43 PDT
(In reply to comment #16)
> Committed r170336: <http://trac.webkit.org/changeset/170336>

It made fullscreen/video-cursor-auto-hide.html crash on the Apple Mac bots.
Comment 20 Jer Noble 2014-06-24 10:05:05 PDT
Tracking the crash in <https://bugs.webkit.org/show_bug.cgi?id=134253>