1078119 - Rename AnimationPlayer::GetCurrentTimeDuration

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Brian Birtles (:birtles)]

Since I discovered BinaryName in WebIDL I'm a bit unsure how much to use it in the Web Animations work.

There are a few cases where we have functions which we use internally and from the DOM but where they behave differently.

In bug 1037321 we have both:

  PlayState()
  PlayStateFromJS()

The latter flushes style first. However, we often want to query to play state internally so it seems to make sense to give the nice name to the one intended for internal use rather than risk people accidentally flushing style.

I was thinking of doing the same thing for methods that get/set times. Internally we work with TimeDuration objects but for DOM callers we convert this to a DOMHighResTimeStamp (a double).

So I was thinking we should have:

  Nullable<TimeDuration> GetCurrentTime()
  Nullable<double>       GetCurrentTimeAsDouble()

We fetch the current time a lot internally so this is nicer than having to write GetCurrentTimeAsDuration() or something similar.

The only problem I can see is that nearly every method in the public API will end up being renamed for one of the above reasons. I'm not sure if this is a problem or not.

Comment 1

[Brian Birtles (:birtles)]

Attachment: Rename AnimationTimeline/AnimationPlayer GetCurrentTimeDuration

This patch performs the following renaming:

  AnimationPlayer::GetCurrentTime -> GetCurrentTimeAsDouble
  AnimationPlayer::GetCurrentTimeDuration -> GetCurrentTime
  AnimationTimeline::GetCurrentTime -> GetCurrentTimeAsDouble
  AnimationTimeline::GetCurrentTimeDuration -> GetCurrentTime

Comment 2

[Brian Birtles (:birtles)]

Attachment: Rename AnimationTimeline/AnimationPlayer GetCurrentTimeDuration

Forgot to drop a declaration of GetCurrentTimeDuration

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8500257 [details] [diff] [review]
Rename AnimationTimeline/AnimationPlayer GetCurrentTimeDuration

r=me

In this case another option would have been to return some class with auto-conversions to both double and TimeDuration (e.g. a subclass of TimeDuration with a conversion operator to double).  But that seems a lot more confusing than just having separate methods.

Comment 4

[Brian Birtles (:birtles)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6866bdaa73d

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f6866bdaa73d