Bug 1307884 - Clean up actors; r=nchevobbe draft
authorJan Odvarko <odvarko@gmail.com>
Mon, 22 May 2017 19:59:06 +0200
changeset 582479 8daa22c4645c15a04af636a02315a9246211dcb0
parent 582478 baebb8c6aeed8ff15fc9e81f9470dfad3371118c
child 582480 31e80ce3ea2dc3734a01a83e8871607d79fa3020
push id60109
push userjodvarko@mozilla.com
push dateMon, 22 May 2017 18:06:18 +0000
reviewersnchevobbe
bugs1307884
milestone55.0a1
Bug 1307884 - Clean up actors; r=nchevobbe MozReview-Commit-ID: Ls5zAExKtkP
devtools/client/webconsole/new-console-output/constants.js
devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
devtools/client/webconsole/new-console-output/reducers/messages.js
devtools/client/webconsole/new-console-output/store.js
--- a/devtools/client/webconsole/new-console-output/constants.js
+++ b/devtools/client/webconsole/new-console-output/constants.js
@@ -8,16 +8,17 @@
 const actionTypes = {
   BATCH_ACTIONS: "BATCH_ACTIONS",
   MESSAGE_ADD: "MESSAGE_ADD",
   MESSAGES_CLEAR: "MESSAGES_CLEAR",
   MESSAGE_OPEN: "MESSAGE_OPEN",
   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",
   FILTERS_CLEAR: "FILTERS_CLEAR",
   FILTER_BAR_TOGGLE: "FILTER_BAR_TOGGLE",
 };
 
 const prefs = {
--- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
+++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
@@ -11,30 +11,32 @@ const { Provider } = require("devtools/c
 const actions = require("devtools/client/webconsole/new-console-output/actions/index");
 const { createContextMenu } = require("devtools/client/webconsole/new-console-output/utils/context-menu");
 const { configureStore } = require("devtools/client/webconsole/new-console-output/store");
 
 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;
 
 function NewConsoleOutputWrapper(parentNode, jsterm, toolbox, owner, document) {
   EventEmitter.decorate(this);
 
   this.parentNode = parentNode;
   this.jsterm = jsterm;
   this.toolbox = toolbox;
   this.owner = owner;
   this.document = document;
 
   this.init = this.init.bind(this);
+
+  store = configureStore(this.jsterm.hud);
 }
 
 NewConsoleOutputWrapper.prototype = {
   init: function () {
     const attachRefToHud = (id, node) => {
       this.jsterm.hud[id] = node;
     };
 
--- a/devtools/client/webconsole/new-console-output/reducers/messages.js
+++ b/devtools/client/webconsole/new-console-output/reducers/messages.js
@@ -20,16 +20,19 @@ const MessageState = Immutable.Record({
   // Map of the form {messageId : tableData}, which represent the data passed
   // as an argument in console.table calls.
   messagesTableDataById: Immutable.Map(),
   // Map of the form {groupMessageId : groupArray},
   // where groupArray is the list of of all the parent groups' ids of the groupMessageId.
   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) {
   const {
     messagesById,
     messagesUiById,
     messagesTableDataById,
     groupsById,
@@ -83,26 +86,27 @@ function messages(state = new MessageSta
           if (newMessage.type === constants.MESSAGE_TYPE.START_GROUP) {
             // We want the group to be open by default.
             record.set("messagesUiById", messagesUiById.push(newMessage.id));
           }
         }
 
         // 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());
         record.set("currentGroup", null);
       });
     case constants.MESSAGE_OPEN:
       return state.set("messagesUiById", messagesUiById.push(action.id));
     case constants.MESSAGE_CLOSE:
@@ -111,16 +115,18 @@ function messages(state = new MessageSta
     case constants.MESSAGE_TABLE_RECEIVE:
       const {id, data} = action;
       return state.set("messagesTableDataById", messagesTableDataById.set(id, data));
     case constants.NETWORK_MESSAGE_UPDATE:
       let updateMessage = action.message;
       return state.set("messagesById", messagesById.map((message) =>
         (message.id === updateMessage.id) ? updateMessage : message
       ));
+    case constants.REMOVED_MESSAGES_CLEAR:
+      return state.set("removedMessages", []);
   }
 
   return state;
 }
 
 function getNewCurrentGroup(currentGoup, groupsById) {
   let newCurrentGroup = null;
   if (currentGoup) {
@@ -146,52 +152,91 @@ function getParentGroups(currentGroup, g
       groups = groups.concat(parentGroups);
     }
   }
 
   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).
  */
 function getToplevelMessageCount(record) {
   return [...record.messagesById].filter(message => !message.groupId).length;
 }
 
 /**
  * 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;
--- a/devtools/client/webconsole/new-console-output/store.js
+++ b/devtools/client/webconsole/new-console-output/store.js
@@ -9,23 +9,26 @@ const {UiState} = require("devtools/clie
 const {
   applyMiddleware,
   combineReducers,
   compose,
   createStore
 } = 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),
     }),
     filters: new FilterState({
       error: Services.prefs.getBoolPref(PREFS.FILTER.ERROR),
       warn: Services.prefs.getBoolPref(PREFS.FILTER.WARN),
       info: Services.prefs.getBoolPref(PREFS.FILTER.INFO),
@@ -38,17 +41,17 @@ function configureStore() {
     ui: new UiState({
       filterBarVisible: Services.prefs.getBoolPref(PREFS.UI.FILTER_BAR),
     })
   };
 
   return createStore(
     combineReducers(reducers),
     initialState,
-    compose(applyMiddleware(thunk), enableBatching())
+    compose(applyMiddleware(thunk), enableActorReleaser(hud), enableBatching())
   );
 }
 
 /**
  * A enhancer for the store to handle batched actions.
  */
 function enableBatching() {
   return next => (reducer, initialState, enhancer) => {
@@ -65,12 +68,58 @@ function enableBatching() {
       enhancer = initialState;
       initialState = undefined;
     }
 
     return next(batchingReducer, initialState, enhancer);
   };
 }
 
+/**
+ * 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;