From f3f602739e2b2bd482eb6e4c69bc7afd156fac1e Mon Sep 17 00:00:00 2001 From: Jan Odvarko Date: Mon, 22 May 2017 19:59:06 +0200 Subject: [PATCH] Bug 1307884 - Clean up actors; r=nchevobbe MozReview-Commit-ID: Ls5zAExKtkP --HG-- extra : rebase_source : 8daa22c4645c15a04af636a02315a9246211dcb0 --- .../new-console-output/constants.js | 1 + .../new-console-output-wrapper.js | 4 +- .../new-console-output/reducers/messages.js | 71 +++++++++++++++---- .../webconsole/new-console-output/store.js | 53 +++++++++++++- 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/devtools/client/webconsole/new-console-output/constants.js b/devtools/client/webconsole/new-console-output/constants.js index 696e81428c93..81ec1a0509de 100644 --- a/devtools/client/webconsole/new-console-output/constants.js +++ b/devtools/client/webconsole/new-console-output/constants.js @@ -13,6 +13,7 @@ const actionTypes = { MESSAGE_CLOSE: "MESSAGE_CLOSE", NETWORK_MESSAGE_UPDATE: "NETWORK_MESSAGE_UPDATE", MESSAGE_TABLE_RECEIVE: "MESSAGE_TABLE_RECEIVE", + REMOVED_MESSAGES_CLEAR: "REMOVED_MESSAGES_CLEAR", TIMESTAMPS_TOGGLE: "TIMESTAMPS_TOGGLE", FILTER_TOGGLE: "FILTER_TOGGLE", FILTER_TEXT_SET: "FILTER_TEXT_SET", diff --git a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js index af768d0a2b24..78d13c69c324 100644 --- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js +++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js @@ -16,7 +16,7 @@ const EventEmitter = require("devtools/shared/event-emitter"); const ConsoleOutput = React.createFactory(require("devtools/client/webconsole/new-console-output/components/console-output")); const FilterBar = React.createFactory(require("devtools/client/webconsole/new-console-output/components/filter-bar")); -const store = configureStore(); +let store = null; let queuedActions = []; let throttledDispatchTimeout = false; @@ -30,6 +30,8 @@ function NewConsoleOutputWrapper(parentNode, jsterm, toolbox, owner, document) { this.document = document; this.init = this.init.bind(this); + + store = configureStore(this.jsterm.hud); } NewConsoleOutputWrapper.prototype = { diff --git a/devtools/client/webconsole/new-console-output/reducers/messages.js b/devtools/client/webconsole/new-console-output/reducers/messages.js index d607230d9008..0c166bef05e8 100644 --- a/devtools/client/webconsole/new-console-output/reducers/messages.js +++ b/devtools/client/webconsole/new-console-output/reducers/messages.js @@ -25,6 +25,9 @@ const MessageState = Immutable.Record({ groupsById: Immutable.Map(), // Message id of the current group (no corresponding console.groupEnd yet). currentGroup: null, + // List of removed messages is used to release related (parameters) actors. + // This array is not supposed to be consumed by any UI component. + removedMessages: [], }); function messages(state = new MessageState(), action) { @@ -88,16 +91,17 @@ function messages(state = new MessageState(), action) { // Remove top level message if the total count of top level messages // exceeds the current limit. - let topLevelCount = getToplevelMessageCount(record); - while (topLevelCount > logLimit) { - let removedMessage = removeFirstMessage(record); - if (!removedMessage.groupId) { - topLevelCount--; - } - } + limitTopLevelMessageCount(state, record); }); case constants.MESSAGES_CLEAR: return state.withMutations(function (record) { + // Store all removed messages associated with some arguments. + // This array is used by `releaseActorsEnhancer` to release + // all related backend actors. + record.set("removedMessages", + record.messagesById.filter(msg => msg.parameters).toArray()); + + // Clear immutable state. record.set("messagesById", Immutable.List()); record.set("messagesUiById", Immutable.List()); record.set("groupsById", Immutable.Map()); @@ -116,6 +120,8 @@ function messages(state = new MessageState(), action) { return state.set("messagesById", messagesById.map((message) => (message.id === updateMessage.id) ? updateMessage : message )); + case constants.REMOVED_MESSAGES_CLEAR: + return state.set("removedMessages", []); } return state; @@ -150,6 +156,40 @@ function getParentGroups(currentGroup, groupsById) { return groups; } +/** + * Remove all top level messages that exceeds message limit. + * Also release all backend actors associated with these + * messages. + */ +function limitTopLevelMessageCount(state, record) { + let tempRecord = { + messagesById: record.messagesById, + messagesUiById: record.messagesUiById, + messagesTableDataById: record.messagesTableDataById, + groupsById: record.groupsById, + }; + + let removedMessages = state.removedMessages; + + // Remove top level messages logged over the limit. + let topLevelCount = getToplevelMessageCount(tempRecord); + while (topLevelCount > logLimit) { + removedMessages.push(...removeFirstMessage(tempRecord)); + topLevelCount--; + } + + // Filter out messages with no arguments. Only actual arguments + // can be associated with backend actors. + removedMessages = state.removedMessages.filter(msg => msg.parameters); + + // Update original record object + record.set("messagesById", tempRecord.messagesById); + record.set("messagesUiById", tempRecord.messagesUiById); + record.set("messagesTableDataById", tempRecord.messagesTableDataById); + record.set("groupsById", tempRecord.groupsById); + record.set("removedMessages", removedMessages); +} + /** * Returns total count of top level messages (those which are not * within a group). @@ -161,37 +201,42 @@ function getToplevelMessageCount(record) { /** * Remove first (the oldest) message from the store. The methods removes * also all its references and children from the store. + * + * @return {Array} Flat array of removed messages. */ function removeFirstMessage(record) { let firstMessage = record.messagesById.first(); - record.set("messagesById", record.messagesById.shift()); + record.messagesById = record.messagesById.shift(); // Remove from list of opened groups. let uiIndex = record.messagesUiById.indexOf(firstMessage); if (uiIndex >= 0) { - record.set("messagesUiById", record.messagesUiById.delete(uiIndex)); + record.messagesUiById = record.messagesUiById.delete(uiIndex); } // Remove from list of tables. if (record.messagesTableDataById.has(firstMessage.id)) { - record.set("messagesTableDataById", record.messagesTableDataById.delete(firstMessage.id)); + record.messagesTableDataById = record.messagesTableDataById.delete(firstMessage.id); } // Remove from list of parent groups. if (record.groupsById.has(firstMessage.id)) { - record.set("groupsById", record.groupsById.delete(firstMessage.id)); + record.groupsById = record.groupsById.delete(firstMessage.id); } + let removedMessages = [firstMessage]; + // Remove all children. This loop assumes that children of removed // group immediately follows the group. We use recursion since // there might be inner groups. let message = record.messagesById.first(); while (message.groupId == firstMessage.id) { - removeFirstMessage(record); + removedMessages.push(...removeFirstMessage(record)); message = record.messagesById.first(); } - return firstMessage; + // Return array with all removed messages. + return removedMessages; } exports.messages = messages; diff --git a/devtools/client/webconsole/new-console-output/store.js b/devtools/client/webconsole/new-console-output/store.js index d6c7bd8c5590..b58719f2e4ad 100644 --- a/devtools/client/webconsole/new-console-output/store.js +++ b/devtools/client/webconsole/new-console-output/store.js @@ -14,13 +14,16 @@ const { } = require("devtools/client/shared/vendor/redux"); const { thunk } = require("devtools/client/shared/redux/middleware/thunk"); const { + MESSAGE_ADD, + MESSAGES_CLEAR, + REMOVED_MESSAGES_CLEAR, BATCH_ACTIONS, PREFS, } = require("devtools/client/webconsole/new-console-output/constants"); const { reducers } = require("./reducers/index"); const Services = require("Services"); -function configureStore() { +function configureStore(hud) { const initialState = { prefs: new PrefState({ logLimit: Math.max(Services.prefs.getIntPref("devtools.hud.loglimit"), 1), @@ -43,7 +46,7 @@ function configureStore() { return createStore( combineReducers(reducers), initialState, - compose(applyMiddleware(thunk), enableBatching()) + compose(applyMiddleware(thunk), enableActorReleaser(hud), enableBatching()) ); } @@ -70,6 +73,52 @@ function enableBatching() { }; } +/** + * This enhancer is responsible for releasing actors on the backend. + * When messages with arguments are removed from the store we should also + * clean up the backend. + */ +function enableActorReleaser(hud) { + return next => (reducer, initialState, enhancer) => { + function releaseActorsEnhancer(state, action) { + state = reducer(state, action); + + let type = action.type; + let proxy = hud ? hud.proxy : null; + if (proxy && (type == MESSAGE_ADD || type == MESSAGES_CLEAR)) { + releaseActors(state.messages.removedMessages, proxy); + + // Reset `removedMessages` in message reducer. + state = reducer(state, { + type: REMOVED_MESSAGES_CLEAR + }); + } + + return state; + } + + return next(releaseActorsEnhancer, initialState, enhancer); + }; +} + +/** + * Helper function for releasing backend actors. + */ +function releaseActors(removedMessages, proxy) { + if (!proxy) { + return; + } + + removedMessages.forEach(msg => { + for (let i = 0; i < msg.parameters.length; i++) { + let param = msg.parameters[i]; + if (param && param.actor) { + proxy.releaseActor(param.actor); + } + } + }); +} + // Provide the store factory for test code so that each test is working with // its own instance. module.exports.configureStore = configureStore;