Bug 1428717 - Fix netProviderEnhancer for MESSAGE_OPEN action for non-network message;r=Honza. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Mon, 08 Jan 2018 15:37:23 +0100
changeset 717326 089dce93d1de60e4b44936b514627803aa2c5100
parent 717183 ca379fcca95b1f4a3744242ea8647004b99b3507
child 745214 4a238563ef5e2f153ff31ed8ea30d9f1eb2b5778
push id94631
push userbmo:nchevobbe@mozilla.com
push dateMon, 08 Jan 2018 17:10:11 +0000
reviewersHonza
bugs1428717
milestone59.0a1
Bug 1428717 - Fix netProviderEnhancer for MESSAGE_OPEN action for non-network message;r=Honza. There was an exception thrown by the netProviderEnhancer because it retrieves the networkMessageUpdate and access a property on it. On non-network message, the networkMessageUpdate is undefined, and accessing the property throws. We fix this by simply checking if message is not falsy before accessing the property. A couple of tests were added to make sure we don't regress this case. This required changing the setupStore helper a bit to pass a hud stub to the createStore function (the net enhancer is called only if the hud has a proxy object). This made some test fail because they weren't dispatching enough argument to the networkMessageUpdate action. MozReview-Commit-ID: 7h35ebHSdbF
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/messages.test.js
devtools/client/webconsole/new-console-output/test/store/network-messages.test.js
--- a/devtools/client/webconsole/new-console-output/store.js
+++ b/devtools/client/webconsole/new-console-output/store.js
@@ -195,17 +195,17 @@ function enableNetProvider(hud) {
       // If network message has been opened, fetch all HTTP details
       // from the backend. It can happen (especially in test) that
       // the message is opened before all network event updates are
       // received. The rest of updates will be handled below, see:
       // NETWORK_MESSAGE_UPDATE action handler.
       if (type == MESSAGE_OPEN) {
         let updates = getAllNetworkMessagesUpdateById(newState);
         let message = updates[action.id];
-        if (!message.openedOnce && message.source == "network") {
+        if (message && !message.openedOnce && message.source == "network") {
           dataProvider.onNetworkEvent(null, message);
           message.updates.forEach(updateType => {
             dataProvider.onNetworkEventUpdate(null, {
               packet: { updateType: updateType },
               networkInfo: message,
             });
           });
         }
--- a/devtools/client/webconsole/new-console-output/test/helpers.js
+++ b/devtools/client/webconsole/new-console-output/test/helpers.js
@@ -35,16 +35,23 @@ function setupActions() {
     messagesAdd: packets => actions.messagesAdd(packets, idGenerator)
   };
 }
 
 /**
  * Prepare the store for use in testing.
  */
 function setupStore(input = [], hud, options, wrappedActions) {
+  if (!hud) {
+    hud = {
+      proxy: {
+        releaseActor: () => {}
+      }
+    };
+  }
   const store = configureStore(hud, options);
 
   // Add the messages from the input commands to the store.
   const messagesAdd = wrappedActions
     ? wrappedActions.messagesAdd
     : actions.messagesAdd;
   store.dispatch(messagesAdd(input.map(cmd => stubPackets.get(cmd))));
 
--- a/devtools/client/webconsole/new-console-output/test/store/messages.test.js
+++ b/devtools/client/webconsole/new-console-output/test/store/messages.test.js
@@ -538,16 +538,55 @@ describe("Message reducer:", () => {
       const { dispatch, getState } = setupStore([]);
 
       const message = stubPackets.get("console.groupCollapsed('foo')");
       dispatch(actions.messagesAdd([message]));
 
       const expanded = getAllMessagesUiById(getState());
       expect(expanded.length).toBe(0);
     });
+
+    it("reacts to messageClose/messageOpen actions on console.group", () => {
+      const { dispatch, getState } = setupStore(["console.group('bar')"]);
+      const firstMessageId = getFirstMessage(getState()).id;
+
+      let expanded = getAllMessagesUiById(getState());
+      expect(expanded.length).toBe(1);
+      expect(expanded[0]).toBe(firstMessageId);
+
+      dispatch(actions.messageClose(firstMessageId));
+
+      expanded = getAllMessagesUiById(getState());
+      expect(expanded.length).toBe(0);
+
+      dispatch(actions.messageOpen(firstMessageId));
+
+      expanded = getAllMessagesUiById(getState());
+      expect(expanded.length).toBe(1);
+      expect(expanded[0]).toBe(firstMessageId);
+    });
+
+    it("reacts to messageClose/messageOpen actions on exception", () => {
+      const { dispatch, getState } = setupStore(["ReferenceError: asdf is not defined"]);
+      const firstMessageId = getFirstMessage(getState()).id;
+
+      let expanded = getAllMessagesUiById(getState());
+      expect(expanded.length).toBe(0);
+
+      dispatch(actions.messageOpen(firstMessageId));
+
+      expanded = getAllMessagesUiById(getState());
+      expect(expanded.length).toBe(1);
+      expect(expanded[0]).toBe(firstMessageId);
+
+      dispatch(actions.messageClose(firstMessageId));
+
+      expanded = getAllMessagesUiById(getState());
+      expect(expanded.length).toBe(0);
+    });
   });
 
   describe("currentGroup", () => {
     it("sets the currentGroup when console.group message is added", () => {
       const { dispatch, getState } = setupStore([]);
 
       const packet = stubPackets.get("console.group('bar')");
       dispatch(actions.messagesAdd([packet]));
@@ -668,39 +707,42 @@ describe("Message reducer:", () => {
       const { dispatch, getState } = setupStore([]);
 
       let packet = clonePacket(stubPackets.get("GET request"));
       let updatePacket = clonePacket(stubPackets.get("GET request update"));
 
       packet.actor = "message1";
       updatePacket.networkInfo.actor = "message1";
       dispatch(actions.messagesAdd([packet]));
-      dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+      dispatch(
+        actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
 
       let networkUpdates = getAllNetworkMessagesUpdateById(getState());
       expect(Object.keys(networkUpdates)).toEqual(["message1"]);
 
       packet = clonePacket(stubPackets.get("GET request"));
       updatePacket = stubPackets.get("XHR GET request update");
       packet.actor = "message2";
       updatePacket.networkInfo.actor = "message2";
       dispatch(actions.messagesAdd([packet]));
-      dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+      dispatch(
+        actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
 
       networkUpdates = getAllNetworkMessagesUpdateById(getState());
       expect(Object.keys(networkUpdates)).toEqual(["message1", "message2"]);
     });
 
     it("resets networkMessagesUpdateById in response to MESSAGES_CLEAR action", () => {
       const { dispatch, getState } = setupStore([
         "XHR GET request"
       ]);
 
       const updatePacket = stubPackets.get("XHR GET request update");
-      dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+      dispatch(
+        actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
 
       let networkUpdates = getAllNetworkMessagesUpdateById(getState());
       expect(Object.keys(networkUpdates).length).toBe(1);
 
       dispatch(actions.messagesClear());
 
       networkUpdates = getAllNetworkMessagesUpdateById(getState());
       expect(Object.keys(networkUpdates).length).toBe(0);
@@ -712,27 +754,30 @@ describe("Message reducer:", () => {
       });
 
       // Add 3 network messages and their updates
       let packet = clonePacket(stubPackets.get("XHR GET request"));
       let updatePacket = clonePacket(stubPackets.get("XHR GET request update"));
       packet.actor = "message1";
       updatePacket.networkInfo.actor = "message1";
       dispatch(actions.messagesAdd([packet]));
-      dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+      dispatch(
+        actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
 
       packet.actor = "message2";
       updatePacket.networkInfo.actor = "message2";
       dispatch(actions.messagesAdd([packet]));
-      dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+      dispatch(
+        actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
 
       packet.actor = "message3";
       updatePacket.networkInfo.actor = "message3";
       dispatch(actions.messagesAdd([packet]));
-      dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+      dispatch(
+        actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
 
       // Check that we have the expected data.
       let messages = getAllMessagesById(getState());
       const [
         firstNetworkMessageId,
         secondNetworkMessageId,
         thirdNetworkMessageId
       ] = [...messages.keys()];
--- a/devtools/client/webconsole/new-console-output/test/store/network-messages.test.js
+++ b/devtools/client/webconsole/new-console-output/test/store/network-messages.test.js
@@ -30,17 +30,17 @@ describe("Network message reducer:", () 
     dispatch = store.dispatch;
 
     let packet = clonePacket(stubPackets.get("GET request"));
     let updatePacket = clonePacket(stubPackets.get("GET request update"));
 
     packet.actor = "message1";
     updatePacket.networkInfo.actor = "message1";
     dispatch(actions.messagesAdd([packet]));
-    dispatch(actions.networkMessageUpdate(updatePacket.networkInfo));
+    dispatch(actions.networkMessageUpdate(updatePacket.networkInfo, null, updatePacket));
   });
 
   describe("networkMessagesUpdateById", () => {
     it("adds fetched HTTP request headers", () => {
       let headers = {
         headers: []
       };