Bug 1307928 - Remove private messages on lastPrivateContextExited event; r=bgrins. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Wed, 28 Feb 2018 12:14:26 +0100
changeset 765195 1427c9f47bd078353fdaa61d94a394155d31ab61
parent 765194 55458ef7f104803089391ebb8143342111c92ba3
child 765196 f6f7555e26dc6174c18732685ec161920bc4d51a
push id102000
push userbmo:nchevobbe@mozilla.com
push dateFri, 09 Mar 2018 09:13:17 +0000
reviewersbgrins
bugs1307928
milestone60.0a1
Bug 1307928 - Remove private messages on lastPrivateContextExited event; r=bgrins. Add a redux action to handle the event and clear private messages from the store. A new `cleanState` function was extracted from `limitTopLevelMessage` so we can use it in this case. MozReview-Commit-ID: Hp6o9iZAbZ7
devtools/client/webconsole/jsterm.js
devtools/client/webconsole/new-console-output/actions/messages.js
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
devtools/client/webconsole/new-console-output/utils/messages.js
--- a/devtools/client/webconsole/jsterm.js
+++ b/devtools/client/webconsole/jsterm.js
@@ -1001,19 +1001,23 @@ JSTerm.prototype = {
   },
 
   /**
    * Remove all of the private messages from the Web Console output.
    *
    * This method emits the "private-messages-cleared" notification.
    */
   clearPrivateMessages: function () {
-    let nodes = this.hud.outputNode.querySelectorAll(".message[private]");
-    for (let node of nodes) {
-      this.hud.removeOutputMessage(node);
+    if (this.hud.NEW_CONSOLE_OUTPUT_ENABLED) {
+      this.hud.newConsoleOutput.dispatchPrivateMessagesClear();
+    } else {
+      let nodes = this.hud.outputNode.querySelectorAll(".message[private]");
+      for (let node of nodes) {
+        this.hud.removeOutputMessage(node);
+      }
     }
     this.emit("private-messages-cleared");
   },
 
   /**
    * Updates the size of the input field (command line) to fit its contents.
    *
    * @returns void
--- a/devtools/client/webconsole/new-console-output/actions/messages.js
+++ b/devtools/client/webconsole/new-console-output/actions/messages.js
@@ -16,16 +16,17 @@ const {
   MESSAGES_ADD,
   NETWORK_MESSAGE_UPDATE,
   NETWORK_UPDATE_REQUEST,
   MESSAGES_CLEAR,
   MESSAGE_OPEN,
   MESSAGE_CLOSE,
   MESSAGE_TYPE,
   MESSAGE_TABLE_RECEIVE,
+  PRIVATE_MESSAGES_CLEAR,
 } = require("../constants");
 
 const defaultIdGenerator = new IdGenerator();
 
 function messagesAdd(packets, idGenerator = null) {
   if (idGenerator == null) {
     idGenerator = defaultIdGenerator;
   }
@@ -51,16 +52,22 @@ function messagesAdd(packets, idGenerato
 }
 
 function messagesClear() {
   return {
     type: MESSAGES_CLEAR
   };
 }
 
+function privateMessagesClear() {
+  return {
+    type: PRIVATE_MESSAGES_CLEAR
+  };
+}
+
 function messageOpen(id) {
   return {
     type: MESSAGE_OPEN,
     id
   };
 }
 
 function messageClose(id) {
@@ -124,11 +131,12 @@ function networkUpdateRequest(id, data) 
 module.exports = {
   messagesAdd,
   messagesClear,
   messageOpen,
   messageClose,
   messageTableDataGet,
   networkMessageUpdate,
   networkUpdateRequest,
+  privateMessagesClear,
   // for test purpose only.
   messageTableDataReceive,
 };
--- a/devtools/client/webconsole/new-console-output/constants.js
+++ b/devtools/client/webconsole/new-console-output/constants.js
@@ -16,16 +16,17 @@ const actionTypes = {
   MESSAGE_CLOSE: "MESSAGE_CLOSE",
   MESSAGE_OPEN: "MESSAGE_OPEN",
   MESSAGE_TABLE_RECEIVE: "MESSAGE_TABLE_RECEIVE",
   MESSAGES_ADD: "MESSAGES_ADD",
   MESSAGES_CLEAR: "MESSAGES_CLEAR",
   NETWORK_MESSAGE_UPDATE: "NETWORK_MESSAGE_UPDATE",
   NETWORK_UPDATE_REQUEST: "NETWORK_UPDATE_REQUEST",
   PERSIST_TOGGLE: "PERSIST_TOGGLE",
+  PRIVATE_MESSAGES_CLEAR: "PRIVATE_MESSAGES_CLEAR",
   REMOVED_ACTORS_CLEAR: "REMOVED_ACTORS_CLEAR",
   SELECT_NETWORK_MESSAGE_TAB: "SELECT_NETWORK_MESSAGE_TAB",
   SIDEBAR_CLOSE: "SIDEBAR_CLOSE",
   SHOW_OBJECT_IN_SIDEBAR: "SHOW_OBJECT_IN_SIDEBAR",
   TIMESTAMPS_TOGGLE: "TIMESTAMPS_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
@@ -6,16 +6,18 @@
 const { createElement, createFactory } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const ReactDOM = require("devtools/client/shared/vendor/react-dom");
 const { Provider } = require("devtools/client/shared/vendor/react-redux");
 
 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 { isPacketPrivate } = require("devtools/client/webconsole/new-console-output/utils/messages");
+const { getAllMessagesById, getMessage } = require("devtools/client/webconsole/new-console-output/selectors/messages");
 
 const EventEmitter = require("devtools/shared/event-emitter");
 const ConsoleOutput = createFactory(require("devtools/client/webconsole/new-console-output/components/ConsoleOutput"));
 const FilterBar = createFactory(require("devtools/client/webconsole/new-console-output/components/FilterBar"));
 const SideBar = createFactory(require("devtools/client/webconsole/new-console-output/components/SideBar"));
 
 let store = null;
 
@@ -259,22 +261,68 @@ NewConsoleOutputWrapper.prototype = {
     return promise;
   },
 
   dispatchMessagesAdd: function (messages) {
     store.dispatch(actions.messagesAdd(messages));
   },
 
   dispatchMessagesClear: function () {
+    // We might still have pending message additions and updates when the clear action is
+    // triggered, so we need to flush them to make sure we don't have unexpected behavior
+    // in the ConsoleOutput.
     this.queuedMessageAdds = [];
     this.queuedMessageUpdates = [];
     this.queuedRequestUpdates = [];
     store.dispatch(actions.messagesClear());
   },
 
+  dispatchPrivateMessagesClear: function () {
+    // We might still have pending private message additions when the private messages
+    // clear action is triggered. We need to remove any private-window-issued packets from
+    // the queue so they won't appear in the output.
+
+    // For (network) message updates, we need to check both messages queue and the state
+    // since we can receive updates even if the message isn't rendered yet.
+    const messages = [...getAllMessagesById(store.getState()).values()];
+    this.queuedMessageUpdates = this.queuedMessageUpdates.filter(({networkInfo}) => {
+      const { actor } = networkInfo;
+
+      const queuedNetworkMessage = this.queuedMessageAdds.find(p => p.actor === actor);
+      if (queuedNetworkMessage && isPacketPrivate(queuedNetworkMessage)) {
+        return false;
+      }
+
+      const requestMessage = messages.find(message => actor === message.actor);
+      if (requestMessage && requestMessage.private === true) {
+        return false;
+      }
+
+      return true;
+    });
+
+    // For (network) requests updates, we can check only the state, since there must be a
+    // user interaction to get an update (i.e. the network message is displayed and thus
+    // in the state).
+    this.queuedRequestUpdates = this.queuedRequestUpdates.filter(({id}) => {
+      const requestMessage = getMessage(store.getState(), id);
+      if (requestMessage && requestMessage.private === true) {
+        return false;
+      }
+
+      return true;
+    });
+
+    // Finally we clear the messages queue. This needs to be done here since we use it to
+    // clean the other queues.
+    this.queuedMessageAdds = this.queuedMessageAdds.filter(p => !isPacketPrivate(p));
+
+    store.dispatch(actions.privateMessagesClear());
+  },
+
   dispatchTimestampsToggle: function (enabled) {
     store.dispatch(actions.timestampsToggle(enabled));
   },
 
   dispatchMessageUpdate: function (message, res) {
     // network-message-updated will emit when all the update message arrives.
     // Since we can't ensure the order of the network update, we check
     // that networkInfo.updates has all we need.
--- a/devtools/client/webconsole/new-console-output/reducers/messages.js
+++ b/devtools/client/webconsole/new-console-output/reducers/messages.js
@@ -192,21 +192,38 @@ function messages(state = MessageState()
 
       return limitTopLevelMessageCount(newState, logLimit);
 
     case constants.MESSAGES_CLEAR:
       return MessageState({
         // Store all actors from removed messages. This array is used by
         // `releaseActorsEnhancer` to release all of those backend actors.
         removedActors: [...state.messagesById.values()].reduce((res, msg) => {
-          res.push(...getAllActorsInMessage(msg, state));
+          res.push(...getAllActorsInMessage(msg));
           return res;
         }, [])
       });
 
+    case constants.PRIVATE_MESSAGES_CLEAR:
+      const removedIds = [];
+      for (const [id, message] of messagesById) {
+        if (message.private === true) {
+          removedIds.push(id);
+        }
+      }
+
+      // If there's no private messages, there's no need to change the state.
+      if (removedIds.length === 0) {
+        return state;
+      }
+
+      return removeMessagesFromState({
+        ...state,
+      }, removedIds);
+
     case constants.MESSAGE_OPEN:
       const openState = {...state};
       openState.messagesUiById = [...messagesUiById, action.id];
       let currMessage = messagesById.get(action.id);
 
       // If the message is a group
       if (isGroupType(currMessage.type)) {
         // We want to make its children visible
@@ -333,27 +350,44 @@ function messages(state = MessageState()
         visibleMessages: messagesToShow,
         filteredMessagesCount: filtered,
       };
   }
 
   return state;
 }
 
-function getNewCurrentGroup(currentGroup, groupsById) {
-  let newCurrentGroup = null;
-  if (currentGroup) {
-    // Retrieve the parent groups of the current group.
-    let parents = groupsById.get(currentGroup);
-    if (Array.isArray(parents) && parents.length > 0) {
-      // If there's at least one parent, make the first one the new currentGroup.
-      newCurrentGroup = parents[0];
+/**
+ * Returns the new current group id given the previous current group and the groupsById
+ * state property.
+ *
+ * @param {String} currentGroup: id of the current group
+ * @param {Map} groupsById
+ * @param {Array} ignoredIds: An array of ids which can't be the new current group.
+ * @returns {String|null} The new current group id, or null if there isn't one.
+ */
+function getNewCurrentGroup(currentGroup, groupsById, ignoredIds = []) {
+  if (!currentGroup) {
+    return null;
+  }
+
+  // Retrieve the parent groups of the current group.
+  let parents = groupsById.get(currentGroup);
+
+  // If there's at least one parent, make the first one the new currentGroup.
+  if (Array.isArray(parents) && parents.length > 0) {
+    // If the found group must be ignored, let's search for its parent.
+    if (ignoredIds.includes(parents[0])) {
+      return getNewCurrentGroup(parents[0], groupsById, ignoredIds);
     }
+
+    return parents[0];
   }
-  return newCurrentGroup;
+
+  return null;
 }
 
 function getParentGroups(currentGroup, groupsById) {
   let groups = [];
   if (currentGroup) {
     // If there is a current group, we add it as a parent
     groups = [currentGroup];
 
@@ -377,18 +411,16 @@ function limitTopLevelMessageCount(newSt
     ? newState.messagesById.size
     : getToplevelMessageCount(newState);
 
   if (topLevelCount <= logLimit) {
     return newState;
   }
 
   const removedMessagesId = [];
-  const removedActors = [];
-  let visibleMessages = [...newState.visibleMessages];
 
   let cleaningGroup = false;
   for (let [id, message] of newState.messagesById) {
     // If we were cleaning a group and the current message does not have
     // a groupId, we're done cleaning.
     if (cleaningGroup === true && !message.groupId) {
       cleaningGroup = false;
     }
@@ -405,82 +437,109 @@ function limitTopLevelMessageCount(newSt
       cleaningGroup = true;
     }
 
     if (!message.groupId) {
       topLevelCount--;
     }
 
     removedMessagesId.push(id);
-    removedActors.push(...getAllActorsInMessage(message, newState));
+  }
+
+  return removeMessagesFromState(newState, removedMessagesId);
+}
 
+/**
+ * Clean the properties for a given state object and an array of removed messages ids.
+ * Be aware that this function MUTATE the `state` argument.
+ *
+ * @param {MessageState} state
+ * @param {Array} removedMessagesIds
+ * @returns {MessageState}
+ */
+function removeMessagesFromState(state, removedMessagesIds) {
+  if (!Array.isArray(removedMessagesIds) || removedMessagesIds.length === 0) {
+    return state;
+  }
+
+  const removedActors = [];
+  const visibleMessages = [...state.visibleMessages];
+  removedMessagesIds.forEach(id => {
     const index = visibleMessages.indexOf(id);
     if (index > -1) {
       visibleMessages.splice(index, 1);
     }
+
+    removedActors.push(...getAllActorsInMessage(state.messagesById.get(id)));
+  });
+
+  if (state.visibleMessages.length > visibleMessages.length) {
+    state.visibleMessages = visibleMessages;
   }
 
   if (removedActors.length > 0) {
-    newState.removedActors =  newState.removedActors.concat(removedActors);
+    state.removedActors =  state.removedActors.concat(removedActors);
   }
 
-  if (newState.visibleMessages.length > visibleMessages.length) {
-    newState.visibleMessages = visibleMessages;
-  }
-
-  const isInRemovedId = id => removedMessagesId.includes(id);
-  const mapHasRemovedIdKey = map => removedMessagesId.some(id => map.has(id));
+  const isInRemovedId = id => removedMessagesIds.includes(id);
+  const mapHasRemovedIdKey = map => removedMessagesIds.some(id => map.has(id));
   const objectHasRemovedIdKey = obj => Object.keys(obj).findIndex(isInRemovedId) !== -1;
 
   const cleanUpMap = map => {
     const clonedMap = new Map(map);
-    removedMessagesId.forEach(id => clonedMap.delete(id));
+    removedMessagesIds.forEach(id => clonedMap.delete(id));
     return clonedMap;
   };
   const cleanUpObject = object => [...Object.entries(object)]
     .reduce((res, [id, value]) => {
       if (!isInRemovedId(id)) {
         res[id] = value;
       }
       return res;
     }, {});
 
-  newState.messagesById = cleanUpMap(newState.messagesById);
+  state.messagesById = cleanUpMap(state.messagesById);
 
-  if (newState.messagesUiById.find(isInRemovedId)) {
-    newState.messagesUiById = newState.messagesUiById.filter(id => !isInRemovedId(id));
+  if (state.messagesUiById.find(isInRemovedId)) {
+    state.messagesUiById = state.messagesUiById.filter(id => !isInRemovedId(id));
+  }
+
+  if (isInRemovedId(state.currentGroup)) {
+    state.currentGroup =
+      getNewCurrentGroup(state.currentGroup, state.groupsById, removedMessagesIds);
   }
 
-  if (mapHasRemovedIdKey(newState.messagesTableDataById)) {
-    newState.messagesTableDataById =
-      cleanUpMap(newState.messagesTableDataById);
+  if (mapHasRemovedIdKey(state.messagesTableDataById)) {
+    state.messagesTableDataById = cleanUpMap(state.messagesTableDataById);
   }
-  if (mapHasRemovedIdKey(newState.groupsById)) {
-    newState.groupsById = cleanUpMap(newState.groupsById);
+  if (mapHasRemovedIdKey(state.groupsById)) {
+    state.groupsById = cleanUpMap(state.groupsById);
   }
-  if (objectHasRemovedIdKey(newState.repeatById)) {
-    newState.repeatById = cleanUpObject(newState.repeatById);
+  if (mapHasRemovedIdKey(state.groupsById)) {
+    state.groupsById = cleanUpMap(state.groupsById);
   }
 
-  if (objectHasRemovedIdKey(newState.networkMessagesUpdateById)) {
-    newState.networkMessagesUpdateById =
-      cleanUpObject(newState.networkMessagesUpdateById);
+  if (objectHasRemovedIdKey(state.repeatById)) {
+    state.repeatById = cleanUpObject(state.repeatById);
   }
 
-  return newState;
+  if (objectHasRemovedIdKey(state.networkMessagesUpdateById)) {
+    state.networkMessagesUpdateById = cleanUpObject(state.networkMessagesUpdateById);
+  }
+
+  return state;
 }
 
 /**
  * Get an array of all the actors logged in a specific message.
  *
  * @param {Message} message: The message to get actors from.
- * @param {Record} state: The redux state.
  * @return {Array} An array containing all the actors logged in a message.
  */
-function getAllActorsInMessage(message, state) {
+function getAllActorsInMessage(message) {
   const {
     parameters,
     messageText,
   } = message;
 
   let actors = [];
   if (Array.isArray(parameters)) {
     message.parameters.forEach(parameter => {
--- 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 {
   BATCH_ACTIONS
 } = require("devtools/client/shared/redux/middleware/debounce");
 const {
   MESSAGE_OPEN,
   MESSAGES_ADD,
   MESSAGES_CLEAR,
+  PRIVATE_MESSAGES_CLEAR,
   REMOVED_ACTORS_CLEAR,
   NETWORK_MESSAGE_UPDATE,
   PREFS,
 } = require("devtools/client/webconsole/new-console-output/constants");
 const { reducers } = require("./reducers/index");
 const {
   getMessage,
   getAllMessagesUiById,
@@ -139,17 +140,20 @@ 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 && ([MESSAGES_ADD, MESSAGES_CLEAR].includes(type))) {
+      if (
+        proxy &&
+        ([MESSAGES_ADD, MESSAGES_CLEAR, PRIVATE_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/utils/messages.js
+++ b/devtools/client/webconsole/new-console-output/utils/messages.js
@@ -383,16 +383,26 @@ function isGroupType(type) {
   ].includes(type);
 }
 
 function getInitialMessageCountForViewport(win) {
   const minMessageHeight = 20;
   return Math.ceil(win.innerHeight / minMessageHeight);
 }
 
+function isPacketPrivate(packet) {
+  return (
+    packet.private === true ||
+    (packet.message && packet.message.private === true) ||
+    (packet.pageError && packet.pageError.private === true) ||
+    (packet.networkEvent && packet.networkEvent.private === true)
+  );
+}
+
 module.exports = {
   getInitialMessageCountForViewport,
   isGroupType,
+  isPacketPrivate,
   l10n,
   prepareMessage,
   // Export for use in testing.
   getRepeatId,
 };