Bug 1363681 - Move the network update information outside of the message type. r=Honza draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 06 Jun 2017 11:56:22 +0200
changeset 589526 465ed3a20c6c202fccf0af9f57f36dcd63db04e6
parent 589301 2c6289f56812c30254acfdddabcfec1e149c0336
child 631914 3a46a28eb7c95346db1073b5c594c2d61e2d9004
push id62409
push userbmo:nchevobbe@mozilla.com
push dateTue, 06 Jun 2017 10:02:16 +0000
reviewersHonza
bugs1363681
milestone55.0a1
Bug 1363681 - Move the network update information outside of the message type. r=Honza This moves network update messages to their own property on the store. We take this as an opportunity to only dispatch the last network update, i.e. `eventTimings` since it has all we need and saves us some time (there are 8 network update per network messages, which can be costly). MozReview-Commit-ID: 2AQN3IlgHg7
devtools/client/webconsole/new-console-output/components/console-output.js
devtools/client/webconsole/new-console-output/components/message-container.js
devtools/client/webconsole/new-console-output/components/message-types/network-event-message.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/selectors/messages.js
devtools/client/webconsole/new-console-output/test/components/network-event-message.test.js
devtools/client/webconsole/new-console-output/test/store/messages.test.js
--- a/devtools/client/webconsole/new-console-output/components/console-output.js
+++ b/devtools/client/webconsole/new-console-output/components/console-output.js
@@ -9,16 +9,17 @@ const {
   DOM: dom,
   PropTypes
 } = require("devtools/client/shared/vendor/react");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
 
 const {
   getAllMessagesUiById,
   getAllMessagesTableDataById,
+  getAllNetworkMessagesUpdateById,
   getVisibleMessages,
   getAllRepeatById,
 } = require("devtools/client/webconsole/new-console-output/selectors/messages");
 const MessageContainer = createFactory(require("devtools/client/webconsole/new-console-output/components/message-container").MessageContainer);
 
 const ConsoleOutput = createClass({
 
   displayName: "ConsoleOutput",
@@ -29,16 +30,17 @@ const ConsoleOutput = createClass({
       attachRefToHud: PropTypes.func.isRequired,
       openContextMenu: PropTypes.func.isRequired,
       sourceMapService: PropTypes.object,
     }),
     dispatch: PropTypes.func.isRequired,
     timestampsVisible: PropTypes.bool,
     messagesTableData: PropTypes.object.isRequired,
     messagesRepeat: PropTypes.object.isRequired,
+    networkMessagesUpdate: PropTypes.object.isRequired,
     visibleMessages: PropTypes.array.isRequired,
   },
 
   componentDidMount() {
     // Do the scrolling in the nextTick since this could hit console startup performances.
     // See https://bugzilla.mozilla.org/show_bug.cgi?id=1355869
     setTimeout(() => {
       scrollToBottom(this.outputNode);
@@ -73,32 +75,34 @@ const ConsoleOutput = createClass({
 
   render() {
     let {
       dispatch,
       visibleMessages,
       messagesUi,
       messagesTableData,
       messagesRepeat,
+      networkMessagesUpdate,
       serviceContainer,
       timestampsVisible,
     } = this.props;
 
     let messageNodes = visibleMessages.map((message) => {
       return (
         MessageContainer({
           dispatch,
           message,
           key: message.id,
           serviceContainer,
           open: messagesUi.includes(message.id),
           tableData: messagesTableData.get(message.id),
           indent: message.indent,
           timestampsVisible,
-          repeat: messagesRepeat[message.id]
+          repeat: messagesRepeat[message.id],
+          networkMessageUpdate: networkMessagesUpdate[message.id],
         })
       );
     });
 
     return (
       dom.div({
         className: "webconsole-output",
         onContextMenu: this.onContextMenu,
@@ -123,13 +127,14 @@ function isScrolledToBottom(outputNode, 
 }
 
 function mapStateToProps(state, props) {
   return {
     visibleMessages: getVisibleMessages(state),
     messagesUi: getAllMessagesUiById(state),
     messagesTableData: getAllMessagesTableDataById(state),
     messagesRepeat: getAllRepeatById(state),
+    networkMessagesUpdate: getAllNetworkMessagesUpdateById(state),
     timestampsVisible: state.ui.timestampsVisible,
   };
 }
 
 module.exports = connect(mapStateToProps)(ConsoleOutput);
--- a/devtools/client/webconsole/new-console-output/components/message-container.js
+++ b/devtools/client/webconsole/new-console-output/components/message-container.js
@@ -33,16 +33,17 @@ const MessageContainer = createClass({
   propTypes: {
     message: PropTypes.object.isRequired,
     open: PropTypes.bool.isRequired,
     serviceContainer: PropTypes.object.isRequired,
     indent: PropTypes.number.isRequired,
     tableData: PropTypes.object,
     timestampsVisible: PropTypes.bool.isRequired,
     repeat: PropTypes.object,
+    networkMessageUpdate: PropTypes.object.isRequired,
   },
 
   getDefaultProps: function () {
     return {
       open: false,
       indent: 0,
     };
   },
@@ -50,23 +51,26 @@ const MessageContainer = createClass({
   shouldComponentUpdate(nextProps, nextState) {
     const repeatChanged = this.props.repeat !== nextProps.repeat;
     const openChanged = this.props.open !== nextProps.open;
     const tableDataChanged = this.props.tableData !== nextProps.tableData;
     const responseChanged = this.props.message.response !== nextProps.message.response;
     const totalTimeChanged = this.props.message.totalTime !== nextProps.message.totalTime;
     const timestampVisibleChanged =
       this.props.timestampsVisible !== nextProps.timestampsVisible;
+    const networkMessageUpdateChanged =
+      this.props.networkMessageUpdate !== nextProps.networkMessageUpdate;
 
     return repeatChanged
       || openChanged
       || tableDataChanged
       || responseChanged
       || totalTimeChanged
-      || timestampVisibleChanged;
+      || timestampVisibleChanged
+      || networkMessageUpdateChanged;
   },
 
   render() {
     const { message } = this.props;
 
     let MessageComponent = getMessageComponent(message);
     return MessageComponent(this.props);
   }
--- a/devtools/client/webconsole/new-console-output/components/message-types/network-event-message.js
+++ b/devtools/client/webconsole/new-console-output/components/message-types/network-event-message.js
@@ -19,48 +19,55 @@ NetworkEventMessage.displayName = "Netwo
 
 NetworkEventMessage.propTypes = {
   message: PropTypes.object.isRequired,
   serviceContainer: PropTypes.shape({
     openNetworkPanel: PropTypes.func.isRequired,
   }),
   indent: PropTypes.number.isRequired,
   timestampsVisible: PropTypes.bool.isRequired,
+  networkMessageUpdate: PropTypes.object.isRequired,
 };
 
 NetworkEventMessage.defaultProps = {
   indent: 0,
 };
 
 function NetworkEventMessage({
   indent,
   message = {},
   serviceContainer,
   timestampsVisible,
+  networkMessageUpdate = {},
 }) {
   const {
     actor,
     source,
     type,
     level,
     request,
-    response: {
-      httpVersion,
-      status,
-      statusText,
-    },
     isXHR,
     timeStamp,
+  } = message;
+
+  const {
+    response = {},
     totalTime,
-  } = message;
+  } = networkMessageUpdate;
+
+  const {
+    httpVersion,
+    status,
+    statusText,
+  } = response;
 
   const topLevelClasses = [ "cm-s-mozilla" ];
   let statusInfo;
 
-  if (httpVersion && status && statusText && totalTime !== undefined) {
+  if (httpVersion && status && statusText !== undefined && totalTime !== undefined) {
     statusInfo = `[${httpVersion} ${status} ${statusText} ${totalTime}ms]`;
   }
 
   const openNetworkMonitor = serviceContainer.openNetworkPanel
     ? () => serviceContainer.openNetworkPanel(actor)
     : null;
 
   const method = dom.span({className: "method" }, request.method);
--- a/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
+++ b/devtools/client/webconsole/new-console-output/new-console-output-wrapper.js
@@ -179,21 +179,20 @@ NewConsoleOutputWrapper.prototype = {
     store.dispatch(actions.messagesClear());
   },
 
   dispatchTimestampsToggle: function (enabled) {
     store.dispatch(actions.timestampsToggle(enabled));
   },
 
   dispatchMessageUpdate: function (message, res) {
-    batchedMessageAdd(actions.networkMessageUpdate(message));
-
     // network-message-updated will emit when eventTimings message arrives
     // which is the last one of 8 updates happening on network message update.
     if (res.packet.updateType === "eventTimings") {
+      batchedMessageAdd(actions.networkMessageUpdate(message));
       this.jsterm.hud.emit("network-message-updated", res);
     }
   },
 
   // Should be used for test purpose only.
   getStore: function () {
     return store;
   }
--- a/devtools/client/webconsole/new-console-output/reducers/messages.js
+++ b/devtools/client/webconsole/new-console-output/reducers/messages.js
@@ -31,24 +31,28 @@ const MessageState = Immutable.Record({
   // 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: [],
   // Map of the form {messageId : numberOfRepeat}
-  repeatById: {}
+  repeatById: {},
+  // Map of the form {messageId : networkInformation}
+  // `networkInformation` holds request, response, totalTime, ...
+  networkMessagesUpdateById: {},
 });
 
 function messages(state = new MessageState(), action, filtersState, prefsState) {
   const {
     messagesById,
     messagesUiById,
     messagesTableDataById,
+    networkMessagesUpdateById,
     groupsById,
     currentGroup,
     repeatById,
     visibleMessages,
   } = state;
 
   const {logLimit} = prefsState;
 
@@ -184,21 +188,22 @@ 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.set(
-        updateMessage.id,
-        updateMessage
-      ));
+      return state.set(
+        "networkMessagesUpdateById",
+        Object.assign({}, networkMessagesUpdateById, {
+          [action.message.id]: action.message
+        })
+      );
 
     case constants.REMOVED_MESSAGES_CLEAR:
       return state.set("removedMessages", []);
 
     case constants.FILTER_TOGGLE:
     case constants.FILTER_TEXT_SET:
       return state.set(
         "visibleMessages",
@@ -307,39 +312,44 @@ function limitTopLevelMessageCount(state
 
   if (record.visibleMessages.length > visibleMessages.length) {
     record.set("visibleMessages", visibleMessages);
   }
 
   const isInRemovedId = id => removedMessagesId.includes(id);
   const mapHasRemovedIdKey = map => map.findKey((value, id) => isInRemovedId(id));
   const cleanUpCollection = map => removedMessagesId.forEach(id => map.remove(id));
+  const cleanUpObject = object => [...Object.entries(object)]
+    .reduce((res, [id, value]) => {
+      if (!isInRemovedId(id)) {
+        res[id] = value;
+      }
+      return res;
+    }, {});
 
   record.set("messagesById", record.messagesById.withMutations(cleanUpCollection));
 
   if (record.messagesUiById.find(isInRemovedId)) {
     record.set("messagesUiById", record.messagesUiById.withMutations(cleanUpCollection));
   }
   if (mapHasRemovedIdKey(record.messagesTableDataById)) {
     record.set("messagesTableDataById",
       record.messagesTableDataById.withMutations(cleanUpCollection));
   }
   if (mapHasRemovedIdKey(record.groupsById)) {
     record.set("groupsById", record.groupsById.withMutations(cleanUpCollection));
   }
 
   if (Object.keys(record.repeatById).includes(removedMessagesId)) {
-    record.set("repeatById",
-      [...Object.entries(record.repeatById)].reduce((res, [id, repeat]) => {
-        if (!isInRemovedId(id)) {
-          res[id] = repeat;
-        }
-        return res;
-      }, {})
-    );
+    record.set("repeatById", cleanUpObject(record.repeatById));
+  }
+
+  if (Object.keys(record.networkMessagesUpdateById).includes(removedMessagesId)) {
+    record.set("networkMessagesUpdateById",
+      cleanUpObject(record.networkMessagesUpdateById));
   }
 
   return record;
 }
 
 /**
  * Returns total count of top level messages (those which are not
  * within a group).
--- a/devtools/client/webconsole/new-console-output/selectors/messages.js
+++ b/devtools/client/webconsole/new-console-output/selectors/messages.js
@@ -32,18 +32,23 @@ function getCurrentGroup(state) {
 function getVisibleMessages(state) {
   return state.messages.visibleMessages.map(id => getMessage(state, id));
 }
 
 function getAllRepeatById(state) {
   return state.messages.repeatById;
 }
 
+function getAllNetworkMessagesUpdateById(state) {
+  return state.messages.networkMessagesUpdateById;
+}
+
 module.exports = {
   getMessage,
   getAllMessagesById,
   getAllMessagesUiById,
   getAllMessagesTableDataById,
   getAllGroupsById,
   getCurrentGroup,
   getVisibleMessages,
   getAllRepeatById,
+  getAllNetworkMessagesUpdateById,
 };
--- a/devtools/client/webconsole/new-console-output/test/components/network-event-message.test.js
+++ b/devtools/client/webconsole/new-console-output/test/components/network-event-message.test.js
@@ -18,21 +18,23 @@ const { stubPreparedMessages } = require
 const serviceContainer = require("devtools/client/webconsole/new-console-output/test/fixtures/serviceContainer");
 
 const EXPECTED_URL = "http://example.com/browser/devtools/client/webconsole/new-console-output/test/fixtures/stub-generators/inexistent.html";
 const EXPECTED_STATUS = /\[HTTP\/\d\.\d \d+ [A-Za-z ]+ \d+ms\]/;
 
 describe("NetworkEventMessage component:", () => {
   describe("GET request", () => {
     it("renders as expected", () => {
-      const message = stubPreparedMessages.get("GET request eventTimings");
+      const message = stubPreparedMessages.get("GET request");
+      const update = stubPreparedMessages.get("GET request eventTimings");
       const wrapper = render(NetworkEventMessage({
         message,
         serviceContainer,
         timestampsVisible: true,
+        networkMessageUpdate: update,
       }));
       const { timestampString } = require("devtools/client/webconsole/webconsole-l10n");
 
       expect(wrapper.find(".timestamp").text()).toBe(timestampString(message.timeStamp));
       expect(wrapper.find(".message-body .method").text()).toBe("GET");
       expect(wrapper.find(".message-body .xhr").length).toBe(0);
       expect(wrapper.find(".message-body .url").length).toBe(1);
       expect(wrapper.find(".message-body .url").text()).toBe(EXPECTED_URL);
@@ -61,31 +63,41 @@ describe("NetworkEventMessage component:
 
       wrapper = render(NetworkEventMessage({ message, serviceContainer }));
       expect(wrapper.find(".indent").prop("style").width).toBe(`0`);
     });
   });
 
   describe("XHR GET request", () => {
     it("renders as expected", () => {
-      const message = stubPreparedMessages.get("XHR GET request eventTimings");
-      const wrapper = render(NetworkEventMessage({ message, serviceContainer }));
+      const message = stubPreparedMessages.get("XHR GET request");
+      const update = stubPreparedMessages.get("XHR GET request eventTimings");
+      const wrapper = render(NetworkEventMessage({
+        message,
+        serviceContainer,
+        networkMessageUpdate: update,
+      }));
 
       expect(wrapper.find(".message-body .method").text()).toBe("GET");
       expect(wrapper.find(".message-body .xhr").length).toBe(1);
       expect(wrapper.find(".message-body .xhr").text()).toBe("XHR");
       expect(wrapper.find(".message-body .url").text()).toBe(EXPECTED_URL);
       expect(wrapper.find(".message-body .status").text()).toMatch(EXPECTED_STATUS);
     });
   });
 
   describe("XHR POST request", () => {
     it("renders as expected", () => {
-      const message = stubPreparedMessages.get("XHR POST request eventTimings");
-      const wrapper = render(NetworkEventMessage({ message, serviceContainer }));
+      const message = stubPreparedMessages.get("XHR POST request");
+      const update = stubPreparedMessages.get("XHR POST request eventTimings");
+      const wrapper = render(NetworkEventMessage({
+        message,
+        serviceContainer,
+        networkMessageUpdate: update,
+      }));
 
       expect(wrapper.find(".message-body .method").text()).toBe("POST");
       expect(wrapper.find(".message-body .xhr").length).toBe(1);
       expect(wrapper.find(".message-body .xhr").text()).toBe("XHR");
       expect(wrapper.find(".message-body .url").length).toBe(1);
       expect(wrapper.find(".message-body .url").text()).toBe(EXPECTED_URL);
       expect(wrapper.find(".message-body .status").length).toBe(1);
       expect(wrapper.find(".message-body .status").text()).toMatch(EXPECTED_STATUS);
--- a/devtools/client/webconsole/new-console-output/test/store/messages.test.js
+++ b/devtools/client/webconsole/new-console-output/test/store/messages.test.js
@@ -2,16 +2,17 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
 const {
   getAllGroupsById,
   getAllMessagesById,
   getAllMessagesTableDataById,
   getAllMessagesUiById,
+  getAllNetworkMessagesUpdateById,
   getAllRepeatById,
   getCurrentGroup,
   getVisibleMessages,
 } = require("devtools/client/webconsole/new-console-output/selectors/messages");
 const {
   clonePacket,
   getMessageAt,
   setupActions,
@@ -472,9 +473,50 @@ describe("Message reducer:", () => {
       expect(groupsById.size).toBe(2);
 
       dispatch(actions.messagesClear());
 
       groupsById = getAllGroupsById(getState());
       expect(groupsById.size).toBe(0);
     });
   });
+
+  describe("networkMessagesUpdateById", () => {
+    it("adds the network update message when network update action is called", () => {
+      const { dispatch, getState } = setupStore([
+        "GET request",
+        "XHR GET request"
+      ]);
+
+      let networkUpdates = getAllNetworkMessagesUpdateById(getState());
+      expect(Object.keys(networkUpdates).length).toBe(0);
+
+      let updatePacket = stubPackets.get("GET request eventTimings");
+      dispatch(actions.networkMessageUpdate(updatePacket));
+
+      networkUpdates = getAllNetworkMessagesUpdateById(getState());
+      expect(Object.keys(networkUpdates).length).toBe(1);
+
+      let xhrUpdatePacket = stubPackets.get("XHR GET request eventTimings");
+      dispatch(actions.networkMessageUpdate(xhrUpdatePacket));
+
+      networkUpdates = getAllNetworkMessagesUpdateById(getState());
+      expect(Object.keys(networkUpdates).length).toBe(2);
+    });
+
+    it("resets networkMessagesUpdateById in response to MESSAGES_CLEAR action", () => {
+      const { dispatch, getState } = setupStore([
+        "XHR GET request"
+      ]);
+
+      const updatePacket = stubPackets.get("XHR GET request eventTimings");
+      dispatch(actions.networkMessageUpdate(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);
+    });
+  });
 });