Fix TimeRanges::nearest() to actually calculate nearest time.

Fix TimeRanges::nearest() to actually calculate nearest time.

The existing implementation instead couldn't decide if it wanted to
calculate the nearest time or how close a given time was to a range.

The new implementation follows the HTML spec for TimeRanges where
nearest() returns a time within the range closest to the desired
time. Equidistant matches are resolved by distance from the current
time.

BUG=412562
TEST=new unittest.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181859

[DaleCurtis, 2014-09-10 23:08:41]

dalecurtis@chromium.org changed reviewers:
+ acolwell@chromium.org, philipj@opera.com

[philipj_slow, 2014-09-11 11:08:02]

lgtm % nit

https://codereview.chromium.org/559993004/diff/1/Source/core/html/TimeRangesT...
File Source/core/html/TimeRangesTest.cpp (right):

https://codereview.chromium.org/559993004/diff/1/Source/core/html/TimeRangesT...
Source/core/html/TimeRangesTest.cpp:317: ASSERT_EQ(7, ranges->nearest(8, 6));
Can you also ASSERT_EQ(7, ranges->nearest(8, 8)), or ASSERT_EQ(9,
ranges->nearest(8, 8)) if you want to change the code to do that instead? Either
is fine.

[DaleCurtis, 2014-09-11 19:34:10]

https://codereview.chromium.org/559993004/diff/1/Source/core/html/TimeRangesT...
File Source/core/html/TimeRangesTest.cpp (right):

https://codereview.chromium.org/559993004/diff/1/Source/core/html/TimeRangesT...
Source/core/html/TimeRangesTest.cpp:317: ASSERT_EQ(7, ranges->nearest(8, 6));
On 2014/09/11 11:08:02, philipj wrote:
> Can you also ASSERT_EQ(7, ranges->nearest(8, 8)), or ASSERT_EQ(9,
> ranges->nearest(8, 8)) if you want to change the code to do that instead?
Either
> is fine.

Lets go with the first one. Done.

[acolwell GONE FROM CHROMIUM, 2014-09-11 22:24:39]

lgtm % nit

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
File Source/core/html/TimeRanges.cpp (right):

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
Source/core/html/TimeRanges.cpp:185: double TimeRanges::nearest(double time,
double now) const
ditto.

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
File Source/core/html/TimeRanges.h (right):

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
Source/core/html/TimeRanges.h:112: double nearest(double time, double now)
const;
nit: How about s/time/newPlaybackPosition/ and s/now/currentPlaybackPosition/ so
the names match the spec text?

[DaleCurtis, 2014-09-11 22:40:01]

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
File Source/core/html/TimeRanges.cpp (right):

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
Source/core/html/TimeRanges.cpp:185: double TimeRanges::nearest(double time,
double now) const
On 2014/09/11 22:24:39, acolwell_leaving_chromium_9-23 wrote:
> ditto.

Done.

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
File Source/core/html/TimeRanges.h (right):

https://codereview.chromium.org/559993004/diff/20001/Source/core/html/TimeRan...
Source/core/html/TimeRanges.h:112: double nearest(double time, double now)
const;
On 2014/09/11 22:24:39, acolwell_leaving_chromium_9-23 wrote:
> nit: How about s/time/newPlaybackPosition/ and s/now/currentPlaybackPosition/
so
> the names match the spec text?

I think you must like Java names, but done. :p

[DaleCurtis, 2014-09-11 23:07:49]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-09-11 23:08:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/559993004/40001

[commit-bot: I haz the power, 2014-09-12 00:09:54]

Committed patchset #3 (id:40001) as 181859