Closed
Bug 1203900
Opened 9 years ago
Closed 9 years ago
Fix cycle collection and array buffer creation bug in the implementation of MediaKeyMessageEvent, MediaEncryptedEvent, and BluetoothLeDeviceEvent
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: yrliou, Assigned: yrliou)
References
Details
Attachments
(1 file, 1 obsolete file)
During the review of Bug 1181482, we found bugs in the cycle collection part and array buffer creation in the implementation of MediaKeyMessageEvent, MediaEncryptedEvent, and BluetoothLeDeviceEvent. Details has been described in comment 19-21 of Bug 1181482.
Assignee | ||
Comment 1•9 years ago
|
||
Hi Boris, This patch is fixing the bug we found when you reviewed Bug 1181482, could you help to review it? Thanks, Jocelyn
Attachment #8659826 -
Flags: review?(bzbarsky)
Comment 2•9 years ago
|
||
Comment on attachment 8659826 [details] [diff] [review] Bug 1203900 - Fix cycle collection and array buffer creation bug in the implementation of MediaKeyMessageEvent, MediaEncryptedEvent, and BluetoothLeDeviceEvent. r=me, though I double-checked with mccr8 and the NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS is _not_ in fact needed here (though it also doesn't hurt), because the trace implementation for Event calls it already. That means there is no leak right now, so hard to write tests for one. ;)
Attachment #8659826 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #2) > Comment on attachment 8659826 [details] [diff] [review] > Bug 1203900 - Fix cycle collection and array buffer creation bug in the > implementation of MediaKeyMessageEvent, MediaEncryptedEvent, and > BluetoothLeDeviceEvent. > > r=me, though I double-checked with mccr8 and the > NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS is _not_ in fact needed > here (though it also doesn't hurt), because the trace implementation for > Event calls it already. That means there is no leak right now, so hard to > write tests for one. ;) Ah, thanks, sorry for not finding that out. If so, I would like to remove it. And also remove it in Bug 1181482, is the leak test still needed?
Comment 4•9 years ago
|
||
> If so, I would like to remove it. Makes sense. > And also remove it in Bug 1181482 Yep. > is the leak test still needed? No, since it wouldn't leak... ;)
Assignee | ||
Comment 5•9 years ago
|
||
* remove NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS * convert to hg format
Attachment #8659826 -
Attachment is obsolete: true
Assignee | ||
Comment 6•9 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=24d94f6ac5a1
Assignee | ||
Comment 7•9 years ago
|
||
Another clean try: https://treeherder.mozilla.org/#/jobs?repo=try&tochange=48006a78be5a Those fail items seem already there and unrelated to my patch.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9a3bfacb1a09
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•