Fix another improbable race and clean up node destruction tests

RESOLVED FIXED in Firefox 48

Status

DevTools
Web Audio Editor
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: efaust, Assigned: efaust)

Tracking

unspecified
Firefox 50

Firefox Tracking Flags

(firefox48 fixed, firefox49 fixed, firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Created attachment 8761345 [details] [diff] [review]
Fix

After bug 1115779, I noticed that there was another GC-related race in the node destruction tests. Since then odes are created as garbage, a GC event (though very unlikely) could clean them up before we expected them to be collected, resulting in a different intermittent hang. This should clean up the issue. We can also move the rmoval listeners to a more logical place, since they are not racing singe the beginning anymore.
Attachment #8761345 - Flags: review?(bgrinstead)
Comment on attachment 8761345 [details] [diff] [review]
Fix

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

Better in so many ways, thanks!

::: devtools/client/webaudioeditor/test/doc_destroy-nodes.html
@@ +12,5 @@
>  
>      <script type="text/javascript;version=1.8">
>        "use strict";
> +      // Keep the nodes we want to GC alive until we are ready for them to
> +      // be collected. We will zero this reference by force from the devtools

Nit: 'from the devtools side' -> 'in forceNodeCollection'
Attachment #8761345 - Flags: review?(bgrinstead) → review+
Assignee: nobody → efaustbmo
Status: NEW → ASSIGNED
See Also: → bug 1115779
Comment on attachment 8761345 [details] [diff] [review]
Fix

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

Also, please give this a sensible commit message before landing
(Assignee)

Updated

2 years ago
Summary: Fix another improbably race in node destruction tests → Fix another improbable race and clean up node destruction tests

Comment 3

2 years ago
Pushed by efaustbmo@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/df61facc5659
Clean up webaudioeditor node destruction tests, and fix another race condition. (r=bgrins)

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/df61facc5659
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50

Comment 5

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/24c398084956
status-firefox49: --- → fixed

Comment 6

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/758bf5a6607b
status-firefox48: --- → fixed

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.