7128 - React document listener memory leak fix.

mP1 · mP1 · commit 6c76f2f0482d · 2016-07-07T22:29:55.000+10:00
diff --git a/src/renderers/dom/client/ReactBrowserEventEmitter.js b/src/renderers/dom/client/ReactBrowserEventEmitter.js
@@ -13,6 +13,7 @@
 
 var EventConstants = require('EventConstants');
 var EventPluginRegistry = require('EventPluginRegistry');
+var ReactEventListener = require('ReactEventListener');
 var ReactEventEmitterMixin = require('ReactEventEmitterMixin');
 var ViewportMetrics = require('ViewportMetrics');
 
@@ -154,6 +155,22 @@ var topEventMapping = {
  */
 var topListenersIDKey = '_reactListenersID' + String(Math.random()).slice(2);
 
+/**
+ * Resets the document state that records registered listener tracking.
+ * Document is passed as a parameter allowing the react app to be loaded in a single window, and within an IFRAME that
+ * renders to the main window.
+ * @param {object} The document holding the root container.
+ */
+ReactEventListener.setDocumentResetCallback(function(doc) {
+  // This must be called during a document reset (@see ReactEventListener.resetDocument)
+  // after unmounting the last component, otherwise the next render will be visible,
+  // but document level event listeners will be missing, and input (clicks, typing etc)
+  // wont work.
+  reactTopListenersCounter = 0;
+  alreadyListeningTo = {};
+  delete doc[topListenersIDKey];
+});
+
 function getListeningForDocument(mountAt) {
   // In IE8, `mountAt` is a host object and doesn't have `hasOwnProperty`
   // directly.
@@ -234,13 +251,37 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
    *
    * @param {string} registrationName Name of listener (e.g. `onClick`).
    * @param {object} contentDocumentHandle Document which owns the container
+   * @return {function} A function that removes ALL listeners that were added.
    */
   listenTo: function(registrationName, contentDocumentHandle) {
     var mountAt = contentDocumentHandle;
     var isListening = getListeningForDocument(mountAt);
     var dependencies =
       EventPluginRegistry.registrationNameDependencies[registrationName];
 
+    // aggregates handler remove functions...
+    var listenerRemovers = [];
+    var trapBubbledEvent = function(topLevelType, handlerBaseName) {
+      var remover = ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+        topLevelType,
+        handlerBaseName,
+        mountAt
+      );
+      if (remover && remover.remove) {
+        listenerRemovers.push(remover.remove);
+      }
+    };
+    var trapCapturedEvent = function(topLevelType, handlerBaseName, handle) {
+      var remover = ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
+        topLevelType,
+        handlerBaseName,
+        handle
+      );
+      if (remover && remover.remove) {
+        listenerRemovers.push(remover.remove);
+      }
+    };
+
     var topLevelTypes = EventConstants.topLevelTypes;
     for (var i = 0; i < dependencies.length; i++) {
       var dependency = dependencies[i];
@@ -250,21 +291,21 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
           )) {
         if (dependency === topLevelTypes.topWheel) {
           if (isEventSupported('wheel')) {
-            ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+            trapBubbledEvent(
               topLevelTypes.topWheel,
               'wheel',
               mountAt
             );
           } else if (isEventSupported('mousewheel')) {
-            ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+            trapBubbledEvent(
               topLevelTypes.topWheel,
               'mousewheel',
               mountAt
             );
           } else {
             // Firefox needs to capture a different mouse scroll event.
             // @see http://www.quirksmode.org/dom/events/tests/scroll.html
-            ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+            trapBubbledEvent(
               topLevelTypes.topWheel,
               'DOMMouseScroll',
               mountAt
@@ -273,13 +314,13 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
         } else if (dependency === topLevelTypes.topScroll) {
 
           if (isEventSupported('scroll', true)) {
-            ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
+            trapCapturedEvent(
               topLevelTypes.topScroll,
               'scroll',
               mountAt
             );
           } else {
-            ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+            trapBubbledEvent(
               topLevelTypes.topScroll,
               'scroll',
               ReactBrowserEventEmitter.ReactEventListener.WINDOW_HANDLE
@@ -289,25 +330,25 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
             dependency === topLevelTypes.topBlur) {
 
           if (isEventSupported('focus', true)) {
-            ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
+            trapCapturedEvent(
               topLevelTypes.topFocus,
               'focus',
               mountAt
             );
-            ReactBrowserEventEmitter.ReactEventListener.trapCapturedEvent(
+            trapCapturedEvent(
               topLevelTypes.topBlur,
               'blur',
               mountAt
             );
           } else if (isEventSupported('focusin')) {
             // IE has `focusin` and `focusout` events which bubble.
             // @see http://www.quirksmode.org/blog/archives/2008/04/delegating_the.html
-            ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+            trapBubbledEvent(
               topLevelTypes.topFocus,
               'focusin',
               mountAt
             );
-            ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+            trapBubbledEvent(
               topLevelTypes.topBlur,
               'focusout',
               mountAt
@@ -318,7 +359,7 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
           isListening[topLevelTypes.topBlur] = true;
           isListening[topLevelTypes.topFocus] = true;
         } else if (topEventMapping.hasOwnProperty(dependency)) {
-          ReactBrowserEventEmitter.ReactEventListener.trapBubbledEvent(
+          trapBubbledEvent(
             dependency,
             topEventMapping[dependency],
             mountAt
@@ -328,6 +369,14 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
         isListening[dependency] = true;
       }
     }
+
+    return function() {
+      listenerRemovers.forEach(
+        function(remover) {
+          remover();
+        }
+      );
+    };
   },
 
   trapBubbledEvent: function(topLevelType, handlerBaseName, handle) {
diff --git a/src/renderers/dom/client/ReactEventListener.js b/src/renderers/dom/client/ReactEventListener.js
@@ -20,6 +20,17 @@ var ReactUpdates = require('ReactUpdates');
 var getEventTarget = require('getEventTarget');
 var getUnboundedScrollPosition = require('getUnboundedScrollPosition');
 
+/**
+ * Tracks all listeners added to a document, for later batch removal.
+ * @type {Array}
+ */
+var documentEventListenerRemovers = [];
+
+/**
+ * This is invoked when the document is reset.
+ */
+var documentResetCallback = function() {};
+
 /**
  * Find the deepest React component completely containing the root of the
  * passed-in instance (for use when entire React trees are nested within each
@@ -172,6 +183,40 @@ var ReactEventListener = {
       TopLevelCallbackBookKeeping.release(bookKeeping);
     }
   },
+  /**
+   * Records the remover for a previously added document event listener.
+   * @param {function} remover A function that removes an added listener.
+   */
+  addDocumentEventListenerRemover: function(remover) {
+    if (remover) {
+      documentEventListenerRemovers.push(remover);
+    }
+  },
+
+  /**
+   * This callback will be invoked when resetDocument is also called.
+   * @param callback
+     */
+  setDocumentResetCallback: function(callback) {
+    documentResetCallback = callback;
+  },
+
+  /**
+   * Removes all previously added document or global event listeners.
+   * @param {object} The document holding the last component.
+   */
+  resetDocument: function(doc) {
+    for (;;) {
+      var remover = documentEventListenerRemovers.pop();
+      if (!remover) {
+        break;
+      }
+      remover();
+    }
+
+    documentResetCallback(doc);
+  },
+
 };
 
 module.exports = ReactEventListener;
diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js
@@ -19,6 +19,7 @@ var ReactDOMComponentTree = require('ReactDOMComponentTree');
 var ReactDOMContainerInfo = require('ReactDOMContainerInfo');
 var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
 var ReactElement = require('ReactElement');
+var ReactEventListener = require('ReactEventListener');
 var ReactFeatureFlags = require('ReactFeatureFlags');
 var ReactInstanceMap = require('ReactInstanceMap');
 var ReactInstrumentation = require('ReactInstrumentation');
@@ -604,6 +605,10 @@ var ReactMount = {
       container,
       false
     );
+
+    if (Object.keys(instancesByReactRootID).length===0) {
+      ReactEventListener.resetDocument(container.ownerDocument);
+    }
     return true;
   },
 
diff --git a/src/renderers/dom/client/__tests__/ReactMount-test.js b/src/renderers/dom/client/__tests__/ReactMount-test.js
@@ -14,6 +14,7 @@
 var React;
 var ReactDOM;
 var ReactDOMServer;
+var ReactEventListener;
 var ReactMount;
 var ReactTestUtils;
 var WebComponents;
@@ -25,6 +26,7 @@ describe('ReactMount', function() {
     React = require('React');
     ReactDOM = require('ReactDOM');
     ReactDOMServer = require('ReactDOMServer');
+    ReactEventListener = require('ReactEventListener');
     ReactMount = require('ReactMount');
     ReactTestUtils = require('ReactTestUtils');
 
@@ -278,6 +280,35 @@ describe('ReactMount', function() {
     expect(Object.keys(ReactMount._instancesByReactRootID).length).toBe(1);
   });
 
+  it('only calls ReactEventListener.resetDocument when without any roots', function() {
+    var documentResetted = false;
+    ReactEventListener.setDocumentResetCallback(function(){
+      expect(Object.keys(ReactMount._instancesByReactRootID).length).toBe(0);
+      documentResetted = true;
+    });
+
+    // Used by devtools.
+    var container = document.createElement('div');
+    ReactDOM.render(<span />, container);
+    ReactDOM.unmountComponentAtNode(container);
+    expect(documentResetted).toBe(true);
+  });
+
+  it('only calls ReactEventListener.resetDocument when without any roots 2', function() {
+    var documentResetted = false;
+    ReactEventListener.setDocumentResetCallback(function(){
+      expect(Object.keys(ReactMount._instancesByReactRootID).length).toBe(0);
+      documentResetted = true;
+    });
+
+    // Used by devtools.
+    var container = document.createElement('div');
+    ReactDOM.render(<div />, container);
+    ReactDOM.render(<span />, container);
+    ReactDOM.unmountComponentAtNode(container);
+    expect(documentResetted).toBe(true);
+  });
+
   it('marks top-level mounts', function() {
     var ReactFeatureFlags = require('ReactFeatureFlags');
 
diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js
@@ -32,6 +32,7 @@ var ReactDOMInput = require('ReactDOMInput');
 var ReactDOMOption = require('ReactDOMOption');
 var ReactDOMSelect = require('ReactDOMSelect');
 var ReactDOMTextarea = require('ReactDOMTextarea');
+var ReactEventListener = require('ReactEventListener');
 var ReactInstrumentation = require('ReactInstrumentation');
 var ReactMultiChild = require('ReactMultiChild');
 var ReactServerRenderingTransaction = require('ReactServerRenderingTransaction');
@@ -224,7 +225,11 @@ function enqueuePutListener(inst, registrationName, listener, transaction) {
   var containerInfo = inst._hostContainerInfo;
   var isDocumentFragment = containerInfo._node && containerInfo._node.nodeType === DOC_FRAGMENT_TYPE;
   var doc = isDocumentFragment ? containerInfo._node : containerInfo._ownerDocument;
-  listenTo(registrationName, doc);
+  var listenerRemover = listenTo(registrationName, doc, !isDocumentFragment);
+  if (!isDocumentFragment) {
+    ReactEventListener.addDocumentEventListenerRemover(listenerRemover);
+  }
+
   transaction.getReactMountReady().enqueue(putListener, {
     inst: inst,
     registrationName: registrationName,
