Closed Bug 1036300 Opened 5 years ago Closed 5 years ago

Expose AnimationPlayer.timeline

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: birtles, Assigned: birtles)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch Expose AnimationPlayer.timeline (obsolete) — Splinter Review
Attachment #8452932 - Flags: review?(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
Attachment #8452932 - Flags: review?(bzbarsky) → review+
(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.
Address review feedback
Attachment #8452932 - Attachment is obsolete: true
(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.
> 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.
(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.
Makes sense.  I updated the docs a bit to make this clearer as well.
https://hg.mozilla.org/mozilla-central/rev/75433ee796dc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
See Also: → 1039924
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.