fix destroy in MediaElement backend (#1778)

* fix destroy in MediaElement backend * add pr nr * make var public * move var to webaudio backend
katspaugh · Oct 24, 2019 · 65894cd · 65894cd
1 parent a32b54e
commit 65894cd
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 46 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -13,15 +13,17 @@ wavesurfer.js changelog
     a long duration. You can also supply peaks data, so the entire audio file
     does not have to be decoded.
     For example:
-    ` wavesurfer.load(url | HTMLMediaElement, peaks, preload, duration);
-      wavesurfer.play();
-      wavesurfer.setFilter(customFilter);
-    `
+    ```
+    wavesurfer.load(url | HTMLMediaElement, peaks, preload, duration);
+    wavesurfer.play();
+    wavesurfer.setFilter(customFilter);
+    ```
 - Add `barRadius` option to create waveforms with rounded bars (#953)
 - Throw error when the url parameter supplied to `wavesurfer.load()`
   is empty (#1773, #1775)
 - Specify non-minified wavesurfer.js in `main` entry of `package.json` (#1759)
 - Add `dblclick` event listener to wavesurfer wrapper (#1764)
+- Fix `destroy()` in `MediaElement` backend (#1778)
 - Cursor plugin: flip position of time text to left of the cursor where needed
   to improve readability (#1776)
 - Regions plugin: change region end handler position (#1762)

diff --git a/src/mediaelement.js b/src/mediaelement.js
@@ -15,8 +15,11 @@ export default class MediaElement extends WebAudio {
         /** @private */
         this.params = params;
 
-        // Dummy media to catch errors
-        /** @private */
+        /**
+         * Initially a dummy media element to catch errors. Once `_load` is
+         * called, this will contain the actual `HTMLMediaElement`.
+         * @private
+         */
         this.media = {
             currentTime: 0,
             duration: 0,
@@ -43,6 +46,8 @@ export default class MediaElement extends WebAudio {
         this.buffer = null;
         /** @private */
         this.onPlayEnd = null;
+        /** @private */
+        this.mediaListeners = {};
     }
 
     /**
@@ -53,9 +58,49 @@ export default class MediaElement extends WebAudio {
         this.createTimer();
     }
 
+    /**
+     * Attach event listeners to media element.
+     */
+    _setupMediaListeners() {
+        this.mediaListeners.error = () => {
+            this.fireEvent('error', 'Error loading media element');
+        };
+        this.mediaListeners.canplay = () => {
+            this.fireEvent('canplay');
+        };
+        this.mediaListeners.ended = () => {
+            this.fireEvent('finish');
+        };
+        // listen to and relay play, pause and seeked events to enable
+        // playback control from the external media element
+        this.mediaListeners.play = () => {
+            this.fireEvent('play');
+        };
+        this.mediaListeners.pause = () => {
+            this.fireEvent('pause');
+        };
+        this.mediaListeners.seeked = event => {
+            this.fireEvent('seek');
+        };
+        this.mediaListeners.volumechange = event => {
+            this.isMuted = this.media.muted;
+            if (this.isMuted) {
+                this.volume = 0;
+            } else {
+                this.volume = this.media.volume;
+            }
+            this.fireEvent('volume');
+        };
+
+        // reset event listeners
+        Object.keys(this.mediaListeners).forEach(id => {
+            this.media.removeEventListener(id, this.mediaListeners[id]);
+            this.media.addEventListener(id, this.mediaListeners[id]);
+        });
+    }
+
     /**
      * Create a timer to provide a more precise `audioprocess` event.
-     *
      */
     createTimer() {
         const onAudioProcess = () => {
@@ -146,43 +191,8 @@ export default class MediaElement extends WebAudio {
             media.load();
         }
 
-        media.addEventListener('error', () => {
-            this.fireEvent('error', 'Error loading media element');
-        });
-
-        media.addEventListener('canplay', () => {
-            this.fireEvent('canplay');
-        });
-
-        media.addEventListener('ended', () => {
-            this.fireEvent('finish');
-        });
-
-        // Listen to and relay play, pause and seeked events to enable
-        // playback control from the external media element
-        media.addEventListener('play', () => {
-            this.fireEvent('play');
-        });
-
-        media.addEventListener('pause', () => {
-            this.fireEvent('pause');
-        });
-
-        media.addEventListener('seeked', event => {
-            this.fireEvent('seek');
-        });
-
-        media.addEventListener('volumechange', event => {
-            this.isMuted = media.muted;
-            if (this.isMuted) {
-                this.volume = 0;
-            } else {
-                this.volume = media.volume;
-            }
-            this.fireEvent('volume');
-        });
-
         this.media = media;
+        this._setupMediaListeners();
         this.peaks = peaks;
         this.onPlayEnd = null;
         this.buffer = null;
@@ -391,6 +401,14 @@ export default class MediaElement extends WebAudio {
     destroy() {
         this.pause();
         this.unAll();
+        this.destroyed = true;
+
+        // cleanup media event listeners
+        Object.keys(this.mediaListeners).forEach(id => {
+            if (this.media) {
+                this.media.removeEventListener(id, this.mediaListeners[id]);
+            }
+        });
 
         if (
             this.params.removeMediaElementOnDestroy &&

diff --git a/src/wavesurfer.js b/src/wavesurfer.js
@@ -1398,9 +1398,12 @@ export default class WaveSurfer extends util.Observer {
 
         this.tmpEvents.push(
             this.backend.once('canplay', () => {
-                this.drawBuffer();
-                this.isReady = true;
-                this.fireEvent('ready');
+                // ignore when backend was already destroyed
+                if (!this.backend.destroyed) {
+                    this.drawBuffer();
+                    this.isReady = true;
+                    this.fireEvent('ready');
+                }
             }),
             this.backend.once('error', err => this.fireEvent('error', err))
         );

diff --git a/src/webaudio.js b/src/webaudio.js
@@ -145,6 +145,10 @@ export default class WebAudio extends util.Observer {
         this.state = null;
         /** @private */
         this.explicitDuration = params.duration;
+        /**
+         * Boolean indicating if the backend was destroyed.
+         */
+        this.destroyed = false;
     }
 
     /**
@@ -500,6 +504,7 @@ export default class WebAudio extends util.Observer {
         }
         this.unAll();
         this.buffer = null;
+        this.destroyed = true;
         this.disconnectFilters();
         this.disconnectSource();
         this.gainNode.disconnect();