Media Controls: Fix seek to end when max attr is rounded up

Media Controls: Fix seek to end when max attr is rounded up

When seeking to the end of an audio/video element, by clicking slightly
past the end of the track, sometimes the scrubber would move but the
current playing time wouldn't be updated.

This happened because https://codereview.chromium.org/614263002
(https://crbug.com/412562) made the scrubber only call
mediaElement().setCurrentTime if the |value| of the underlying
<input type="range"> is contained within the mediaElement().seekable()
TimeRanges.

But the |max| attribute of the <input type="range"> can sometimes be
rounded up slightly, for example a duration of 596.586667 results in a
|max| attribute of 596.587, and so seeking to this |max| value fails to
update currentTime.

This patch updates the seekable() contains check to take this floating
point error into account.

BUG=695065
Review-Url: https://codereview.chromium.org/2707043004
Cr-Commit-Position: refs/heads/master@{#453689}
Committed: https://chromium.googlesource.com/chromium/src/+/6d7ae217f9f1a7772046c5447f73bc8cdff8cbed

[johnme, 2017-02-22 17:57:47]

johnme@chromium.org changed reviewers:
+ mlamouri@chromium.org

[mlamouri (slow - plz ping), 2017-02-27 17:48:12]

https://codereview.chromium.org/2707043004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
(right):

https://codereview.chromium.org/2707043004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp:779: time =
duration;
I do not understand why this isn't:
```
if (duration < time)
  time = duration;
```
or said otherwise: `time = std:min(time, duration);`

I bet there is a reason. I can actually guess why we do `time <= maxAttribute`
even though that sounds like we could drop the check. However, I don't
understand why `+0.5`.

[johnme, 2017-02-28 18:02:56]

Addressed review comment - thanks!

https://codereview.chromium.org/2707043004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
(right):

https://codereview.chromium.org/2707043004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp:779: time =
duration;
On 2017/02/27 17:48:12, mlamouri wrote:
> I do not understand why this isn't:
> ```
> if (duration < time)
>   time = duration;
> ```
> or said otherwise: `time = std:min(time, duration);`
> 
> I bet there is a reason. I can actually guess why we do `time <= maxAttribute`
> even though that sounds like we could drop the check. However, I don't
> understand why `+0.5`.

Done.

I was guarding against the case where mediaElement().duration() and and
getFloatingPointAttribute(maxAttr) are significantly out of sync (not just by
rounding error), and trying not to change behaviour in that case. It seems that
can only happen if the webpage calls event.stopImmediatePropagation() in a
capturing durationchange event listener, and since that's such an edge case,
setting time to duration in that case is probably fine too (in fact, it probably
makes more sense).

(The 0.5 was to allow the seconds value to have been rounded, which increases it
by at most 0.5 seconds; though FWIW it seems that it's the milliseconds that get
rounded)

[mlamouri (slow - plz ping), 2017-02-28 18:06:04]

lgtm

[johnme, 2017-02-28 18:30:17]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2017-02-28 18:30:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 20:34:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488306617512150,
"parent_rev": "ddafc6c502773a7f87355d1956e40945b15ca9e5", "commit_rev":
"6d7ae217f9f1a7772046c5447f73bc8cdff8cbed"}

[commit-bot: I haz the power, 2017-02-28 20:34:57]

Description was changed from

==========
Media Controls: Fix seek to end when max attr is rounded up

When seeking to the end of an audio/video element, by clicking slightly
past the end of the track, sometimes the scrubber would move but the
current playing time wouldn't be updated.

This happened because https://codereview.chromium.org/614263002
(https://crbug.com/412562) made the scrubber only call
mediaElement().setCurrentTime if the |value| of the underlying
<input type="range"> is contained within the mediaElement().seekable()
TimeRanges.

But the |max| attribute of the <input type="range"> can sometimes be
rounded up slightly, for example a duration of 596.586667 results in a
|max| attribute of 596.587, and so seeking to this |max| value fails to
update currentTime.

This patch updates the seekable() contains check to take this floating
point error into account.

BUG=695065
==========

to

==========
Media Controls: Fix seek to end when max attr is rounded up

When seeking to the end of an audio/video element, by clicking slightly
past the end of the track, sometimes the scrubber would move but the
current playing time wouldn't be updated.

This happened because https://codereview.chromium.org/614263002
(https://crbug.com/412562) made the scrubber only call
mediaElement().setCurrentTime if the |value| of the underlying
<input type="range"> is contained within the mediaElement().seekable()
TimeRanges.

But the |max| attribute of the <input type="range"> can sometimes be
rounded up slightly, for example a duration of 596.586667 results in a
|max| attribute of 596.587, and so seeking to this |max| value fails to
update currentTime.

This patch updates the seekable() contains check to take this floating
point error into account.

BUG=695065

Review-Url: https://codereview.chromium.org/2707043004
Cr-Commit-Position: refs/heads/master@{#453689}
Committed:
https://chromium.googlesource.com/chromium/src/+/6d7ae217f9f1a7772046c5447f73...
==========

[commit-bot: I haz the power, 2017-02-28 20:35:00]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/6d7ae217f9f1a7772046c5447f73...