various ConvolverNode simplifications

RESOLVED FIXED in Firefox 57

Status

()

P1
normal
Rank:
15
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla57
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(4 attachments)

Comment hidden (empty)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

a year ago
mozreview-review
Comment on attachment 8895268 [details]
bug 1388656 size to actual used length in nsTArray::SetCapacity() optimization

https://reviewboard.mozilla.org/r/166430/#review171690
Attachment #8895268 - Flags: review?(padenot) → review+

Comment 7

a year ago
mozreview-review
Comment on attachment 8895269 [details]
bug 1388656 remove unused SAMPLE_RATE SetInt32Parameter case

https://reviewboard.mozilla.org/r/166432/#review171692
Attachment #8895269 - Flags: review?(padenot) → review+

Comment 8

a year ago
mozreview-review
Comment on attachment 8895270 [details]
bug 1388656 re-initialize the Reverb only once when the ConvolverNode buffer changes

https://reviewboard.mozilla.org/r/166434/#review171694
Attachment #8895270 - Flags: review?(padenot) → review+

Comment 9

a year ago
mozreview-review
Comment on attachment 8895271 [details]
bug 1388656 keep response buffer on graph thread only long enough to initialize the Reverb

https://reviewboard.mozilla.org/r/166436/#review171696
Attachment #8895271 - Flags: review?(padenot) → review+

Comment 10

a year ago
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be8e60de5c0c
size to actual used length in nsTArray::SetCapacity() optimization r=padenot
https://hg.mozilla.org/integration/autoland/rev/1eb9ee70ccf1
remove unused SAMPLE_RATE SetInt32Parameter case r=padenot
https://hg.mozilla.org/integration/autoland/rev/ece4f9694407
re-initialize the Reverb only once when the ConvolverNode buffer changes r=padenot
https://hg.mozilla.org/integration/autoland/rev/af73f36f7469
keep response buffer on graph thread only long enough to initialize the Reverb r=padenot
Backed out bug 1388656 and bug 1024182 for heap buffer overflow at AudioNodeEngine.cpp:375:12 in mozilla::AudioBufferSumOfSquares:

Bug 1388656
https://hg.mozilla.org/integration/autoland/rev/24627256990de09d0ea84eac1a7eda1d1a67c2bf
https://hg.mozilla.org/integration/autoland/rev/0f4c205ad82d371714e1a2b9a19e61bc9a21ce5c
https://hg.mozilla.org/integration/autoland/rev/1f7bad50144216f900e4683498831fb74a19c07d
https://hg.mozilla.org/integration/autoland/rev/c55ff6b0d67864730791e04036b091ebc502fce2

Bug 1024182
https://hg.mozilla.org/integration/autoland/rev/49cfa5269c2c1e40ef62f570321e754ed380536b

Push which ran failing test: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f03833f24817e9cf748a89508eefc347033cb462&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122195448&repo=autoland

==1104==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020000d7a7c at pc 0x7fcddd00270c bp 0x7fcd692835e0 sp 0x7fcd692835d8
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/worker/workspace/build/src/dom/media/webaudio/AudioNodeEngine.cpp:375:12 in mozilla::AudioBufferSumOfSquares(float const*, unsigned int)
REFTEST ERROR | file:///home/worker/workspace/build/tests/reftest/tests/dom/media/test/crashtests/884459.html | application timed out after 330 seconds with no output
Flags: needinfo?(karlt)
Rank: 15
Priority: -- → P1
(Assignee)

Comment 12

a year ago
The failing test passes on the push of comment 10, and so I'll reland this.

Comment 13

a year ago
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a88ed8010517
size to actual used length in nsTArray::SetCapacity() optimization r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/aef648113c68
remove unused SAMPLE_RATE SetInt32Parameter case r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/111a7a257807
re-initialize the Reverb only once when the ConvolverNode buffer changes r=padenot
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd8681109e96
keep response buffer on graph thread only long enough to initialize the Reverb r=padenot
(Assignee)

Updated

a year ago
Flags: needinfo?(karlt)

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a88ed8010517
https://hg.mozilla.org/mozilla-central/rev/aef648113c68
https://hg.mozilla.org/mozilla-central/rev/111a7a257807
https://hg.mozilla.org/mozilla-central/rev/cd8681109e96
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(Assignee)

Updated

a year ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.