1036300 - Expose AnimationPlayer.timeline

Status: RESOLVED FIXED

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

Comment 1

[Brian Birtles (:birtles)]

Attachment: Expose AnimationPlayer.timeline

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8452932 [details] [diff] [review]
Expose AnimationPlayer.timeline

>+already_AddRefed<dom::AnimationTimeline>
>+ElementAnimation::Timeline() const

Just have this return dom::AnimationTimeline*.  And then the body will just be "return mTimeline".

I assume mTimeline is in fact never null?

r=me

Comment 3

[Brian Birtles (:birtles)]

(In reply to Boris Zbarsky [:bz] from comment #2)
> Just have this return dom::AnimationTimeline*.  And then the body will just
> be "return mTimeline".

Will do.

> I assume mTimeline is in fact never null?

Yes, that's right.

Comment 4

[Brian Birtles (:birtles)]

Attachment: Expose AnimationPlayer.timeline

Address review feedback

Comment 5

[Brian Birtles (:birtles)]

(In reply to Boris Zbarsky [:bz] from comment #2)
> Just have this return dom::AnimationTimeline*.  And then the body will just
> be "return mTimeline".

I updated Bindings.conf to mark timeline as resultNotAddRefed.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> I updated Bindings.conf to mark timeline as resultNotAddRefed.

That's fine, but note that it's not required to return a raw pointer.

Basically, bindings codegen for a normal interface-type getter looks like this:

  nsRefPtr<TheType> retval = self->TheGetter();

and this can deal equally well with already_AddRefed<TheType> and TheType* as return types.

Using resultNotAddRefed changes the codegen to:

  TheType* retval = self->TheGetter();

This obviously requires TheType* as return type.

The latter form is really only needed in cases in which the performance of the addref/release is relevant.

Comment 7

[Brian Birtles (:birtles)]

(In reply to Boris Zbarsky [:bz] from comment #6)
> > I updated Bindings.conf to mark timeline as resultNotAddRefed.
> 
> That's fine, but note that it's not required to return a raw pointer.
>...

Thanks for the explanation. I was curious why it wasn't needed. I'll probably drop the changes to Bindings.conf before landing.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Makes sense.  I updated the docs a bit to make this clearer as well.

Comment 9

[Brian Birtles (:birtles)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75433ee796dc

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75433ee796dc