Skip to content

Commit

Permalink
7128 - React document listener memory leak fix.
Browse files Browse the repository at this point in the history
  • Loading branch information
@mP1
mP1 committed Jul 6, 2016
1 parent 6cc037b commit 89ce402
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 11 deletions.
69 changes: 59 additions & 10 deletions src/renderers/dom/client/ReactBrowserEventEmitter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

var EventConstants = require('EventConstants');
var EventPluginRegistry = require('EventPluginRegistry');
var ReactEventListener = require('ReactEventListener');
var ReactEventEmitterMixin = require('ReactEventEmitterMixin');
var ViewportMetrics = require('ViewportMetrics');

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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];
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -328,6 +369,14 @@ var ReactBrowserEventEmitter = Object.assign({}, ReactEventEmitterMixin, {
isListening[dependency] = true;
}
}

return function() {
listenerRemovers.forEach(
function(remover) {
remover();
}
);
};
},

trapBubbledEvent: function(topLevelType, handlerBaseName, handle) {
Expand Down
45 changes: 45 additions & 0 deletions src/renderers/dom/client/ReactEventListener.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
14 changes: 14 additions & 0 deletions src/renderers/dom/client/ReactMount.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -604,6 +605,10 @@ var ReactMount = {
container,
false
);

if (ReactMount._countReactRootElements() === 0) {
ReactEventListener.resetDocument(container.ownerDocument);
}
return true;
},

Expand Down Expand Up @@ -724,6 +729,15 @@ var ReactMount = {
}
}
},
/**
* Rather than count mounts and unmounts, we query the dom for the DOMProperty.ROOT_ATTRIBUTE_NAME. This is used to
* dispose of global event listeners, when the last component is removed/unmounted.
* @returns Returns the number of react tagged root elements.
* @private
*/
_countReactRootElements: function() {
return document.querySelectorAll('[ROOT_ATTR_NAME]').length;
},
};

module.exports = ReactMount;
7 changes: 6 additions & 1 deletion src/renderers/dom/shared/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 89ce402

Please sign in to comment.