Bug 1417439 - Release actors when message are pruned due to an MESSAGES_ADD action. r=Honza, a=gchang
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 15 Nov 2017 14:54:34 +0100
changeset 445100 35df0c984d4a2ba59a0eb16cbc073545ed13e80b
parent 445099 9db3839609e097e84106c385e53ed12202d77f8a
child 445101 9e0f2dcb95f5966621c0ca8e446ebecb12d7e652
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza, gchang
bugs1417439, 1371721
milestone58.0
Bug 1417439 - Release actors when message are pruned due to an MESSAGES_ADD action. r=Honza, a=gchang In Bug 1371721, we created a new action to batch our message addition into the store. Which means that messages can be pruned when using this action, and implies releasing server actors. But, at the moment, we only do that when we use the MESSAGE_ADD action. This wasn't caught by tests because they explicitly use the MESSAGE_ADD action. This patch makes the console release actors in reaction to a MESSAGES_ADD action and add a test to make sure we handle this as expected. MozReview-Commit-ID: FfgvmKi9nM9
devtools/client/webconsole/new-console-output/store.js
devtools/client/webconsole/new-console-output/test/helpers.js
devtools/client/webconsole/new-console-output/test/store/release-actors.test.js
--- a/devtools/client/webconsole/new-console-output/store.js
+++ b/devtools/client/webconsole/new-console-output/store.js
@@ -13,16 +13,17 @@ const {
 } = require("devtools/client/shared/vendor/redux");
 const { thunk } = require("devtools/client/shared/redux/middleware/thunk");
 const {
   BATCH_ACTIONS
 } = require("devtools/client/shared/redux/middleware/debounce");
 const {
   MESSAGE_ADD,
   MESSAGE_OPEN,
+  MESSAGES_ADD,
   MESSAGES_CLEAR,
   REMOVED_ACTORS_CLEAR,
   NETWORK_MESSAGE_UPDATE,
   PREFS,
 } = require("devtools/client/webconsole/new-console-output/constants");
 const { reducers } = require("./reducers/index");
 const Services = require("Services");
 const {
@@ -125,17 +126,17 @@ function enableBatching() {
  */
 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)) {
+      if (proxy && ([MESSAGE_ADD, MESSAGES_ADD, MESSAGES_CLEAR].includes(type))) {
         releaseActors(state.messages.removedActors, proxy);
 
         // Reset `removedActors` in message reducer.
         state = reducer(state, {
           type: REMOVED_ACTORS_CLEAR
         });
       }
 
--- a/devtools/client/webconsole/new-console-output/test/helpers.js
+++ b/devtools/client/webconsole/new-console-output/test/helpers.js
@@ -16,24 +16,21 @@ const {
 } = require("devtools/client/webconsole/new-console-output/selectors/messages");
 
 /**
  * Prepare actions for use in testing.
  */
 function setupActions() {
   // Some actions use dependency injection. This helps them avoid using state in
   // a hard-to-test way. We need to inject stubbed versions of these dependencies.
-  const wrappedActions = Object.assign({}, actions);
-
   const idGenerator = new IdGenerator();
-  wrappedActions.messageAdd = (packet) => {
-    return actions.messageAdd(packet, idGenerator);
-  };
-
-  return wrappedActions;
+  return Object.assign({}, actions, {
+    messageAdd: packet => actions.messageAdd(packet, idGenerator),
+    messagesAdd: packets => actions.messagesAdd(packets, idGenerator)
+  });
 }
 
 /**
  * Prepare the store for use in testing.
  */
 function setupStore(input = [], hud, options) {
   const store = configureStore(hud, options);
 
--- a/devtools/client/webconsole/new-console-output/test/store/release-actors.test.js
+++ b/devtools/client/webconsole/new-console-output/test/store/release-actors.test.js
@@ -18,17 +18,17 @@ const expect = require("expect");
 describe("Release actor enhancer:", () => {
   let actions;
 
   before(() => {
     actions = setupActions();
   });
 
   describe("Client proxy", () => {
-    it("properly releases backend actors when limit is reached", () => {
+    it("releases backend actors when limit reached adding a single message", () => {
       const logLimit = 100;
       let releasedActors = [];
       const { dispatch, getState } = setupStore([], {
         proxy: {
           releaseActor: (actor) => {
             releasedActors.push(actor);
           }
         }
@@ -58,16 +58,64 @@ describe("Release actor enhancer:", () =
       }
 
       expect(releasedActors.length).toBe(3);
       expect(releasedActors).toInclude(firstMessageActor);
       expect(releasedActors).toInclude(secondMessageActor);
       expect(releasedActors).toInclude(thirdMessageActor);
     });
 
+    it("releases backend actors when limit reached adding multiple messages", () => {
+      const logLimit = 100;
+      let releasedActors = [];
+      const { dispatch, getState } = setupStore([], {
+        proxy: {
+          releaseActor: (actor) => {
+            releasedActors.push(actor);
+          }
+        }
+      }, { logLimit });
+
+      // Add a log message.
+      dispatch(actions.messageAdd(
+        stubPackets.get("console.log('myarray', ['red', 'green', 'blue'])")));
+
+      let messages = getAllMessagesById(getState());
+      const firstMessage = messages.first();
+      const firstMessageActor = firstMessage.parameters[1].actor;
+
+      // Add an evaluation result message (see Bug 1408321).
+      const evaluationResultPacket = stubPackets.get("new Date(0)");
+      dispatch(actions.messageAdd(evaluationResultPacket));
+      const secondMessageActor = evaluationResultPacket.result.actor;
+
+      // Add an assertion message.
+      const assertPacket = stubPackets.get("console.assert(false, {message: 'foobar'})");
+      dispatch(actions.messageAdd(assertPacket));
+      const thirdMessageActor = assertPacket.message.arguments[0].actor;
+
+      // Add ${logLimit} messages so we prune the ones we added before.
+      const packets = [];
+      // Alternate between 2 packets so we don't trigger the repeat message mechanism.
+      const oddPacket = stubPackets.get("console.log(undefined)");
+      const evenPacket = stubPackets.get("console.log('foobar', 'test')");
+      for (let i = 0; i < logLimit; i++) {
+        const packet = i % 2 === 0 ? evenPacket : oddPacket;
+        packets.push(packet);
+      }
+
+      // Add all the packets at once. This will prune the first 3 messages.
+      dispatch(actions.messagesAdd(packets));
+
+      expect(releasedActors.length).toBe(3);
+      expect(releasedActors).toInclude(firstMessageActor);
+      expect(releasedActors).toInclude(secondMessageActor);
+      expect(releasedActors).toInclude(thirdMessageActor);
+    });
+
     it("properly releases backend actors after clear", () => {
       let releasedActors = [];
       const { dispatch, getState } = setupStore([], {
         proxy: {
           releaseActor: (actor) => {
             releasedActors.push(actor);
           }
         }