147892 – Don't short circuit seeking

Bug 147892 - Don't short circuit seeking

Summary: Don't short circuit seeking

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Media (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Eric Carlson

URL:
Keywords:	InRadar

Depends on:
Blocks:

Reported:	2015-08-11 12:27 PDT by Eric Carlson
Modified:	2015-09-04 03:58 PDT (History)
CC List:	6 users (show)

See Also:

Attachments
Patch to preview, tests will follow. (10.70 KB, text/plain) 2015-08-11 14:18 PDT, Eric Carlson	buildbot: commit-queue-	Details
Archive of layout-test-results from ews102 for mac-mavericks (688.66 KB, application/zip) 2015-08-11 14:41 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (748.24 KB, application/zip) 2015-08-11 14:46 PDT, Build Bot	no flags	Details
Updated patch (22.93 KB, patch) 2015-08-12 13:08 PDT, Eric Carlson	jer.noble: review+	Details \| Formatted Diff \| Diff
Patch for landing (22.35 KB, text/plain) 2015-08-12 18:44 PDT, Eric Carlson	no flags	Details
Patch for landing (23.05 KB, patch) 2015-08-12 18:47 PDT, Eric Carlson	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews102 for mac-mavericks (552.01 KB, application/zip) 2015-08-12 19:22 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (603.92 KB, application/zip) 2015-08-12 19:27 PDT, Build Bot	no flags	Details
Updated patch for landing (18.79 KB, patch) 2015-08-13 08:14 PDT, Eric Carlson	eric.carlson: commit-queue+	Details \| Formatted Diff \| Diff
Another updated patch for landing (22.75 KB, patch) 2015-08-13 10:37 PDT, Eric Carlson	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from ews102 for mac-mavericks (654.53 KB, application/zip) 2015-08-13 11:15 PDT, Build Bot	no flags	Details
YAUPFL (18.49 KB, patch) 2015-08-13 11:23 PDT, Eric Carlson	no flags	Details \| Formatted Diff \| Diff
Follow up patch (1.85 KB, patch) 2015-08-13 15:40 PDT, Jer Noble	no flags	Details \| Formatted Diff \| Diff
Follow up patch (3.40 KB, patch) 2015-08-13 15:50 PDT, Jer Noble	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (11) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Eric Carlson 2015-08-11 12:27:03 PDT

HTMLMediaElement does nothing when asked to seek to the time of a pending seek. This can result in a seek to the wrong time when the pending seek was caused by a call to fastSeek and the seek that is ignored is an exact seek.

Comment 1 Eric Carlson 2015-08-11 12:34:17 PDT

<rdar://problem/21367467>

Comment 2 Eric Carlson 2015-08-11 14:18:00 PDT

Created attachment 258755 [details]
Patch to preview, tests will follow.

Comment 3 WebKit Commit Bot 2015-08-11 14:20:10 PDT

Attachment 258755 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 4 Build Bot 2015-08-11 14:41:01 PDT

Comment on attachment 258755 [details]
Patch to preview, tests will follow.

Attachment 258755 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/46021

New failing tests:
media/track/track-cues-seeking.html
media/track/track-cue-rendering-rtl.html
media/track/track-css-cue-lifetime.html
media/media-controller-playbackrate.html
media/track/track-in-band-style.html
media/track/track-cue-rendering-snap-to-lines-not-set.html
media/track/track-cue-nothing-to-render.html
media/track/regions-webvtt/vtt-region-display.html
media/track/track-css-matching.html
media/track/track-css-matching-timestamps.html
media/track/track-cue-overlap-snap-to-lines-not-set.html

Comment 5 Build Bot 2015-08-11 14:41:03 PDT

Created attachment 258756 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

Comment 6 Build Bot 2015-08-11 14:46:23 PDT

Comment on attachment 258755 [details]
Patch to preview, tests will follow.

Attachment 258755 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/46042

New failing tests:
media/track/track-cues-seeking.html
media/track/track-cue-rendering-rtl.html
media/track/track-css-cue-lifetime.html
media/media-controller-playbackrate.html
media/track/track-in-band-style.html
media/track/track-cue-rendering-snap-to-lines-not-set.html
media/track/track-cue-nothing-to-render.html
media/track/regions-webvtt/vtt-region-display.html
media/track/track-css-matching.html
media/track/track-css-matching-timestamps.html
media/track/track-cue-overlap-snap-to-lines-not-set.html

Comment 7 Build Bot 2015-08-11 14:46:25 PDT

Created attachment 258758 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

Comment 8 Eric Carlson 2015-08-12 13:08:16 PDT

Created attachment 258840 [details]
Updated patch

Comment 9 Jer Noble 2015-08-12 13:48:58 PDT

Comment on attachment 258840 [details]
Updated patch

Nit, the patch doesn't apply to ToT.

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1315
> -        callOnMainThread([weakThis, finished] {
> +        double currentTime = CMTimeGetSeconds([m_avPlayerItem currentTime]);
> +        callOnMainThread([weakThis, finished, currentTime] {
>              auto _this = weakThis.get();
> +            LOG(Media, "MediaPlayerPrivateAVFoundationObjC::seekToTime(%p) - completion handler called, currentTime = %f", _this, currentTime);
>              if (!_this)

I think this is going to result in a "unused parameter warning/error" on non-logging builds; you may want to add UNUSED_PARAM() inside the block.

Comment 10 Eric Carlson 2015-08-12 18:44:19 PDT

Created attachment 258870 [details]
Patch for landing

Comment 11 Eric Carlson 2015-08-12 18:47:17 PDT

Created attachment 258871 [details]
Patch for landing

Comment 12 Build Bot 2015-08-12 19:22:56 PDT

Comment on attachment 258871 [details]
Patch for landing

Attachment 258871 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/51185

New failing tests:
media/video-played-ranges-1.html
media/video-played-collapse.html
media/video-pause-immediately.html

Comment 13 Build Bot 2015-08-12 19:22:58 PDT

Created attachment 258874 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

Comment 14 Build Bot 2015-08-12 19:27:53 PDT

Comment on attachment 258871 [details]
Patch for landing

Attachment 258871 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/51196

New failing tests:
media/video-played-ranges-1.html
media/video-pause-immediately.html

Comment 15 Build Bot 2015-08-12 19:27:57 PDT

Created attachment 258875 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

Comment 16 Eric Carlson 2015-08-13 08:14:24 PDT

Created attachment 258889 [details]
Updated patch for landing

Comment 17 Eric Carlson 2015-08-13 10:37:36 PDT

Created attachment 258898 [details]
Another updated patch for landing

Comment 18 Build Bot 2015-08-13 11:15:45 PDT

Comment on attachment 258898 [details]
Another updated patch for landing

Attachment 258898 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/53601

New failing tests:
media/video-played-ranges-1.html
media/video-played-collapse.html
media/video-pause-immediately.html

Comment 19 Build Bot 2015-08-13 11:15:47 PDT

Created attachment 258902 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

Comment 20 Eric Carlson 2015-08-13 11:23:53 PDT

Created attachment 258905 [details]
YAUPFL

Comment 21 WebKit Commit Bot 2015-08-13 12:16:32 PDT

Comment on attachment 258905 [details]
YAUPFL

Clearing flags on attachment: 258905

Committed r188390: <http://trac.webkit.org/changeset/188390>

Comment 22 Jer Noble 2015-08-13 15:40:20 PDT

Created attachment 258941 [details]
Follow up patch

Pre-reviewed by Eric Carlson in person.

Comment 23 Jer Noble 2015-08-13 15:50:08 PDT

Created attachment 258943 [details]
Follow up patch

Pre-reviewed by Eric Carlson in person.

Comment 24 WebKit Commit Bot 2015-08-13 16:39:39 PDT

Comment on attachment 258943 [details]
Follow up patch

Clearing flags on attachment: 258943

Committed r188416: <http://trac.webkit.org/changeset/188416>

Comment 25 Csaba Osztrogonác 2015-09-04 03:58:51 PDT

Already landed, let's close it.