From c322cccd53b432f5d9e467daf7bee5b164f66b4d Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Tue, 11 Jun 2019 11:21:23 -0400
Subject: [PATCH] FIX: Memory Leaks when decorating posts (#7749)

* Remove long-deprecated method

* FIX: Memory Leaks when decorating posts

Previously we'd keep creating mixins dynamically when decorating the
same class.

This code changes the API to recommend an `id` parameter for each
decorator which will avoid leaks. All plugins should be updated to
include this parameter, although if they don't in the meantime it'll
just mean a warning in the console (and a continued leak.)
---
 .../initializers/post-decorations.js.es6      | 35 +++++++++------
 .../discourse/lib/lazy-load-images.js.es6     |  2 +-
 .../discourse/lib/plugin-api.js.es6           | 43 +++++++++++++------
 .../initializers/apply-details.js.es6         |  4 +-
 .../initializers/discourse-local-dates.js.es6 |  9 ++--
 .../javascripts/initializers/lazyYT.js.es6    | 33 +++++++-------
 .../initializers/extend-for-poll.js.es6       |  2 +-
 7 files changed, 81 insertions(+), 47 deletions(-)

diff --git a/app/assets/javascripts/discourse/initializers/post-decorations.js.es6 b/app/assets/javascripts/discourse/initializers/post-decorations.js.es6
index e9e69217daa..a9e1cc24d02 100644
--- a/app/assets/javascripts/discourse/initializers/post-decorations.js.es6
+++ b/app/assets/javascripts/discourse/initializers/post-decorations.js.es6
@@ -9,25 +9,32 @@ export default {
   initialize(container) {
     withPluginApi("0.1", api => {
       const siteSettings = container.lookup("site-settings:main");
-      api.decorateCooked(highlightSyntax);
-      api.decorateCooked(lightbox);
+      api.decorateCooked(highlightSyntax, {
+        id: "discourse-syntax-highlighting"
+      });
+      api.decorateCooked(lightbox, { id: "discourse-lightbox" });
       if (siteSettings.support_mixed_text_direction) {
-        api.decorateCooked(setTextDirections);
+        api.decorateCooked(setTextDirections, {
+          id: "discourse-text-direction"
+        });
       }
 
       setupLazyLoading(api);
 
-      api.decorateCooked($elem => {
-        const players = $("audio", $elem);
-        if (players.length) {
-          players.on("play", () => {
-            const postId = parseInt($elem.closest("article").data("post-id"));
-            if (postId) {
-              api.preventCloak(postId);
-            }
-          });
-        }
-      });
+      api.decorateCooked(
+        $elem => {
+          const players = $("audio", $elem);
+          if (players.length) {
+            players.on("play", () => {
+              const postId = parseInt($elem.closest("article").data("post-id"));
+              if (postId) {
+                api.preventCloak(postId);
+              }
+            });
+          }
+        },
+        { id: "discourse-audio" }
+      );
     });
   }
 };
diff --git a/app/assets/javascripts/discourse/lib/lazy-load-images.js.es6 b/app/assets/javascripts/discourse/lib/lazy-load-images.js.es6
index 035ccf572f3..12f78fc3f50 100644
--- a/app/assets/javascripts/discourse/lib/lazy-load-images.js.es6
+++ b/app/assets/javascripts/discourse/lib/lazy-load-images.js.es6
@@ -95,6 +95,6 @@ export function setupLazyLoading(api) {
         }
       });
     },
-    { onlyStream: true }
+    { onlyStream: true, id: "discourse-lazy-load" }
   );
 }
diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js.es6 b/app/assets/javascripts/discourse/lib/plugin-api.js.es6
index 7b682b6f6ed..1fa2f449d03 100644
--- a/app/assets/javascripts/discourse/lib/plugin-api.js.es6
+++ b/app/assets/javascripts/discourse/lib/plugin-api.js.es6
@@ -193,8 +193,14 @@ class PluginApi {
    * For example, to add a yellow background to all posts you could do this:
    *
    * ```
-   * api.decorateCooked($elem => $elem.css({ backgroundColor: 'yellow' }));
+   * api.decorateCooked(
+   *   $elem => $elem.css({ backgroundColor: 'yellow' }),
+   *   { id: 'yellow-decorator' }
+   * );
    * ```
+   *
+   * NOTE: To avoid memory leaks, it is highly recommended to pass a unique `id` parameter.
+   * You will receive a warning if you do not.
    **/
   decorateCooked(callback, opts) {
     opts = opts || {};
@@ -202,12 +208,13 @@ class PluginApi {
     addDecorator(callback);
 
     if (!opts.onlyStream) {
-      decorate(ComposerEditor, "previewRefreshed", callback);
-      decorate(DiscourseBanner, "didInsertElement", callback);
+      decorate(ComposerEditor, "previewRefreshed", callback, opts.id);
+      decorate(DiscourseBanner, "didInsertElement", callback, opts.id);
       decorate(
         this.container.factoryFor("component:user-stream").class,
         "didInsertElement",
-        callback
+        callback,
+        opts.id
       );
     }
   }
@@ -930,7 +937,26 @@ export function withPluginApi(version, apiCodeCallback, opts) {
 }
 
 let _decorateId = 0;
-function decorate(klass, evt, cb) {
+let _decorated = new WeakMap();
+
+function decorate(klass, evt, cb, id) {
+  if (!id) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      "`decorateCooked` should be supplied with an `id` option to avoid memory leaks."
+    );
+  } else {
+    if (!_decorated.has(klass)) {
+      _decorated.set(klass, new Set());
+    }
+    id = `${id}:${evt}`;
+    let set = _decorated.get(klass);
+    if (set.has(id)) {
+      return;
+    }
+    set.add(id);
+  }
+
   const mixin = {};
   mixin["_decorate_" + _decorateId++] = function($elem) {
     $elem = $elem || this.$();
@@ -944,10 +970,3 @@ function decorate(klass, evt, cb) {
 export function resetPluginApi() {
   _pluginv01 = null;
 }
-
-export function decorateCooked() {
-  // eslint-disable-next-line no-console
-  console.warn(
-    "`decorateCooked` has been removed. Use `getPluginApi(version).decorateCooked` instead"
-  );
-}
diff --git a/plugins/discourse-details/assets/javascripts/initializers/apply-details.js.es6 b/plugins/discourse-details/assets/javascripts/initializers/apply-details.js.es6
index 246f88ec9be..78773e8d43d 100644
--- a/plugins/discourse-details/assets/javascripts/initializers/apply-details.js.es6
+++ b/plugins/discourse-details/assets/javascripts/initializers/apply-details.js.es6
@@ -1,7 +1,9 @@
 import { withPluginApi } from "discourse/lib/plugin-api";
 
 function initializeDetails(api) {
-  api.decorateCooked($elem => $("details", $elem).details());
+  api.decorateCooked($elem => $("details", $elem).details(), {
+    id: "discourse-details"
+  });
 
   api.addToolbarPopupMenuOptionsCallback(() => {
     return {
diff --git a/plugins/discourse-local-dates/assets/javascripts/initializers/discourse-local-dates.js.es6 b/plugins/discourse-local-dates/assets/javascripts/initializers/discourse-local-dates.js.es6
index 28117194f74..352ea1159e6 100644
--- a/plugins/discourse-local-dates/assets/javascripts/initializers/discourse-local-dates.js.es6
+++ b/plugins/discourse-local-dates/assets/javascripts/initializers/discourse-local-dates.js.es6
@@ -2,9 +2,12 @@ import { withPluginApi } from "discourse/lib/plugin-api";
 import showModal from "discourse/lib/show-modal";
 
 function initializeDiscourseLocalDates(api) {
-  api.decorateCooked($elem => {
-    $(".discourse-local-date", $elem).applyLocalDates();
-  });
+  api.decorateCooked(
+    $elem => {
+      $(".discourse-local-date", $elem).applyLocalDates();
+    },
+    { id: "discourse-local-date" }
+  );
 
   api.onToolbarCreate(toolbar => {
     toolbar.addButton({
diff --git a/plugins/lazyYT/assets/javascripts/initializers/lazyYT.js.es6 b/plugins/lazyYT/assets/javascripts/initializers/lazyYT.js.es6
index 2531f66e407..b34dd942b8d 100644
--- a/plugins/lazyYT/assets/javascripts/initializers/lazyYT.js.es6
+++ b/plugins/lazyYT/assets/javascripts/initializers/lazyYT.js.es6
@@ -4,22 +4,25 @@ export default {
   name: "apply-lazyYT",
   initialize() {
     withPluginApi("0.1", api => {
-      api.decorateCooked($elem => {
-        const iframes = $(".lazyYT", $elem);
-        if (iframes.length === 0) {
-          return;
-        }
-
-        $(".lazyYT", $elem).lazyYT({
-          onPlay(e, $el) {
-            // don't cloak posts that have playing videos in them
-            const postId = parseInt($el.closest("article").data("post-id"));
-            if (postId) {
-              api.preventCloak(postId);
-            }
+      api.decorateCooked(
+        $elem => {
+          const iframes = $(".lazyYT", $elem);
+          if (iframes.length === 0) {
+            return;
           }
-        });
-      });
+
+          $(".lazyYT", $elem).lazyYT({
+            onPlay(e, $el) {
+              // don't cloak posts that have playing videos in them
+              const postId = parseInt($el.closest("article").data("post-id"));
+              if (postId) {
+                api.preventCloak(postId);
+              }
+            }
+          });
+        },
+        { id: "discourse-lazyyt" }
+      );
     });
   }
 };
diff --git a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6
index 46a54fd77ba..eec06621539 100644
--- a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6
+++ b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6
@@ -104,7 +104,7 @@ function initializePolls(api) {
   }
 
   api.includePostAttributes("polls", "polls_votes");
-  api.decorateCooked(attachPolls, { onlyStream: true });
+  api.decorateCooked(attachPolls, { onlyStream: true, id: "discourse-poll" });
   api.cleanupStream(cleanUpPolls);
 }