Change original WebMediaPlayer signatures from float to double.

Change original WebMediaPlayer signatures from float to double.

This change puts the final signatures containing doubles in place so that
they can be used by Blink code. After this change lands Blink will be updated
to call these methods instead of the temporary xxxFloat() methods.

BUG=227156
TEST=All existing tests still pass. No user visible behavior has changed.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194624

[acolwell GONE FROM CHROMIUM, 2013-04-16 00:07:58]

Trivial patch that changes the original WebMediaPlayer float signatures to
doubles. This is part of a multistage patch sequence to get HTMLMediaElement
updated to use doubles like the spec mandates. Currently Blink calls the
xxxFloat() versions of these functions so that this signature change doesn't
cause the build to break.

Here are previous patches in the sequence that have landed for reference.
Step 1: https://codereview.chromium.org/13431009/
Step 2: https://codereview.chromium.org/13454026/

[DaleCurtis, 2013-04-16 01:19:48]

Mostly nits and things I may not understand, so lgtm % Qs and nits.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
File webkit/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:138: // TODO(qinmin): Remove this
from WebKit::WebMediaPlayer as it is never used.
May lead to some unexpected breakage if this isn't true...

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:146: NOTIMPLEMENTED();
Ditto.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:154: NOTIMPLEMENTED();
Ditto.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:202: return
static_cast<float>(duration());
static cast is unnecessary?

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:292: return
ConvertSecondsToTimestamp(timeValue).InSecondsF();
The upconversion to double here could cause a user visible difference, right?

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
File webkit/media/android/webmediaplayer_impl_android.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_impl_android.cc:94: current_time_ =
current_time.InSecondsF();
Should just store the TimeDelta?

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
File webkit/media/webmediaplayer_impl.cc (left):

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
webkit/media/webmediaplayer_impl.cc:589: return maxTimeSeekable();
How come?

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
File webkit/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
webkit/media/webmediaplayer_impl.cc:561: return paused_time_.InSecondsF();
If you're so inclined: 
return (paused_ ? paused_time_ : pipeline_->GetMediaTime()).InSecondsF()

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_u...
File webkit/media/webmediaplayer_util.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_u...
webkit/media/webmediaplayer_util.cc:11: base::TimeDelta
ConvertSecondsToTimestamp(double seconds) {
This is a lot simpler written as:

const double microseconds = seconds * base::Time::kMicrosecondsPerSecond;
return base::TimeDelta::FromMicroseconds(
    microseconds > 0 ? microseconds + 0.5 : ceil(microseconds - 0.5));

[acolwell GONE FROM CHROMIUM, 2013-04-16 16:58:15]

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
File webkit/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:138: // TODO(qinmin): Remove this
from WebKit::WebMediaPlayer as it is never used.
On 2013/04/16 01:19:48, DaleCurtis wrote:
> May lead to some unexpected breakage if this isn't true...

This isn't called. I plan on removing this particular method in a separate CL.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:146: NOTIMPLEMENTED();
On 2013/04/16 01:19:48, DaleCurtis wrote:
> Ditto.

Right now I'm just preserving existing behavior. I'm not sure why Android
doesn't support setRate() and setVolume() at this point, but I don't think
changing this is in line w/ this particular CL.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:154: NOTIMPLEMENTED();
On 2013/04/16 01:19:48, DaleCurtis wrote:
> Ditto.
Just preserving existing behavior here.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:202: return
static_cast<float>(duration());
On 2013/04/16 01:19:48, DaleCurtis wrote:
> static cast is unnecessary?

Done.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_android.cc:292: return
ConvertSecondsToTimestamp(timeValue).InSecondsF();
On 2013/04/16 01:19:48, DaleCurtis wrote:
> The upconversion to double here could cause a user visible difference, right?
Only for really large time values that no one is likely to encounter at the
moment. Tiny values are truncated to microsecond resolution so there should be
no difference on the small end of things. I think this should be safe for the
brief period between this patch landing and the followup Blink patch.

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
File webkit/media/android/webmediaplayer_impl_android.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/android/webmedia...
webkit/media/android/webmediaplayer_impl_android.cc:94: current_time_ =
current_time.InSecondsF();
On 2013/04/16 01:19:48, DaleCurtis wrote:
> Should just store the TimeDelta?
I was trying to avoid unrelated changes such as this. If I do this though, it
would mean that conversion would happen on every GetCurrentTimeInternal() call,
which I'm assuming happens way more often then OnTimeUpdate().

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
File webkit/media/webmediaplayer_impl.cc (left):

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
webkit/media/webmediaplayer_impl.cc:589: return maxTimeSeekable();
On 2013/04/16 01:19:48, DaleCurtis wrote:
> How come?
durationFloat() needs to be used in this version now that the truncation of the
double range to floats only exists in that function. I just duplicated the
function body for right now and changed the duration() call at the end to
durationFloat().

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
File webkit/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_i...
webkit/media/webmediaplayer_impl.cc:561: return paused_time_.InSecondsF();
On 2013/04/16 01:19:48, DaleCurtis wrote:
> If you're so inclined: 
> return (paused_ ? paused_time_ : pipeline_->GetMediaTime()).InSecondsF()

Done.

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_u...
File webkit/media/webmediaplayer_util.cc (right):

https://codereview.chromium.org/13866046/diff/1/webkit/media/webmediaplayer_u...
webkit/media/webmediaplayer_util.cc:11: base::TimeDelta
ConvertSecondsToTimestamp(double seconds) {
On 2013/04/16 01:19:48, DaleCurtis wrote:
> This is a lot simpler written as:
> 
> const double microseconds = seconds * base::Time::kMicrosecondsPerSecond;
> return base::TimeDelta::FromMicroseconds(
>     microseconds > 0 ? microseconds + 0.5 : ceil(microseconds - 0.5));

Done.

[DaleCurtis, 2013-04-16 17:38:57]

lgtm

[acolwell GONE FROM CHROMIUM, 2013-04-17 17:44:25]

Committed patchset #2 manually as r194624 (presubmit successful).