Bug 1417439 - Release actors when message are pruned due to an MESSAGES_ADD action; r=Honza. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 15 Nov 2017 14:54:34 +0100
changeset 704941 a0416fd911699dac7addb37fffc42aa61d8824a5
parent 704807 cb9092a90f6ef501e6de8eb5fc6ce19e2717193f
child 742200 aa811147109eb715ff672891d11acfc9a80205c8
push id91293
push userbmo:nchevobbe@mozilla.com
push dateWed, 29 Nov 2017 07:24:22 +0000
reviewersHonza
bugs1417439, 1371721
milestone59.0a1
Bug 1417439 - Release actors when message are pruned due to an MESSAGES_ADD action; r=Honza. 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 {
@@ -127,17 +128,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
@@ -18,24 +18,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);
           }
         }