Closed Bug 1034907 Opened 5 years ago Closed 5 years ago

Remove dangerous public destructor of dom::TimeRanges

Categories

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

Other Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: bjacob, Assigned: michael, Mentored)

References

Details

(Whiteboard: [lang=c++])

Attachments

(1 file)

In bug 1028588 we removed dangerous public destructors of XPCOM-refcounted classes outside of a finite whitelist, see HasDangerousPublicDestructor. Now we are going over the entries in this whitelist.

One of them is: dom::TimeRanges
Mentor: continuation
Whiteboard: [lang=c++]
Attached patch PatchSplinter Review
I've removed the public destructor of TimeRanges and updated affected uses accordingly.

Here are the results from the try server:

https://tbpl.mozilla.org/?tree=Try&rev=d816e20e068b
Attachment #8454962 - Flags: review?(continuation)
Comment on attachment 8454962 [details] [diff] [review]
Patch

Review of attachment 8454962 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.  I'm going to forward the review to a DOM peer.
Attachment #8454962 - Flags: review?(continuation)
Attachment #8454962 - Flags: review?(bugs)
Attachment #8454962 - Flags: feedback+
For a little change like this, it probably would have been enough to do a Linux64 debug-only try run.
Assignee: nobody → michael
Comment on attachment 8454962 [details] [diff] [review]
Patch


>   // Clamp the seek target to inside the seekable ranges.
>-  dom::TimeRanges seekable;
>-  if (NS_FAILED(mDecoder->GetSeekable(&seekable))) {
>+  nsRefPtr<dom::TimeRanges> seekable = new dom::TimeRanges();
>+  if (NS_FAILED(mDecoder->GetSeekable(seekable))) {

Uhhuh. Thank you for fixing this crazyness.
Attachment #8454962 - Flags: review?(bugs) → review+
Thanks for the patch!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/877406ac6f1d
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.