Bug 1408321 - Change EvaluationResult message shape to match other messages;r=bgrins. draft
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Fri, 13 Oct 2017 11:46:13 +0200
changeset 680033 932a339fc440fe98446d586f02e47941b30a1e8d
parent 679629 25aad10380b10b6efa50c2b4d97245f078d870a0
child 680044 b2960a7ca6e9d64c9d3372f91d2a16d6430f1791
push id84374
push userbmo:nchevobbe@mozilla.com
push dateFri, 13 Oct 2017 11:56:20 +0000
reviewersbgrins
bugs1408321
milestone58.0a1
Bug 1408321 - Change EvaluationResult message shape to match other messages;r=bgrins. The parameters property in EvaluationResult message was a plain object, whereas in other message types, it is an array. This could cause some issues in the reducer, in particular for releasing actors. Changing the parameters from object to array gives us a consistent way of dealing with this property through the console app. The change required the stubs to be updated, as well as a minor change in the EvaluationResult component. A test was added to make sure we do release actors from EvaluationResult messages. MozReview-Commit-ID: LKQEFqCCw1U
devtools/client/webconsole/new-console-output/components/message-types/EvaluationResult.js
devtools/client/webconsole/new-console-output/test/fixtures/stubs/evaluationResult.js
devtools/client/webconsole/new-console-output/test/store/release-actors.test.js
devtools/client/webconsole/new-console-output/utils/messages.js
--- a/devtools/client/webconsole/new-console-output/components/message-types/EvaluationResult.js
+++ b/devtools/client/webconsole/new-console-output/components/message-types/EvaluationResult.js
@@ -54,17 +54,17 @@ function EvaluationResult(props) {
       && message.messageText.type === "longString"
     ) {
       messageBody = `${message.messageText.initial}…`;
     }
   } else {
     messageBody = GripMessageBody({
       dispatch,
       messageId,
-      grip: parameters,
+      grip: parameters[0],
       serviceContainer,
       useQuotes: true,
       escapeWhitespace: false,
       type,
       helperType,
     });
   }
 
--- a/devtools/client/webconsole/new-console-output/test/fixtures/stubs/evaluationResult.js
+++ b/devtools/client/webconsole/new-console-output/test/fixtures/stubs/evaluationResult.js
@@ -16,29 +16,31 @@ let stubPackets = new Map();
 stubPreparedMessages.set("new Date(0)", new ConsoleMessage({
   "id": "1",
   "allowRepeating": true,
   "source": "javascript",
   "timeStamp": 1479159921364,
   "type": "result",
   "helperType": null,
   "level": "log",
-  "parameters": {
-    "type": "object",
-    "actor": "server1.conn0.child1/obj30",
-    "class": "Date",
-    "extensible": true,
-    "frozen": false,
-    "sealed": false,
-    "ownPropertyLength": 0,
-    "preview": {
-      "timestamp": 0
+  "parameters": [
+    {
+      "type": "object",
+      "actor": "server1.conn0.child1/obj30",
+      "class": "Date",
+      "extensible": true,
+      "frozen": false,
+      "sealed": false,
+      "ownPropertyLength": 0,
+      "preview": {
+        "timestamp": 0
+      }
     }
-  },
-  "repeatId": "{\"frame\":null,\"groupId\":null,\"indent\":0,\"level\":\"log\",\"parameters\":{\"type\":\"object\",\"actor\":\"server1.conn0.child1/obj30\",\"class\":\"Date\",\"extensible\":true,\"frozen\":false,\"sealed\":false,\"ownPropertyLength\":0,\"preview\":{\"timestamp\":0}},\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
+  ],
+  "repeatId": "{\"frame\":null,\"groupId\":null,\"indent\":0,\"level\":\"log\",\"parameters\":[{\"type\":\"object\",\"actor\":\"server1.conn0.child1/obj30\",\"class\":\"Date\",\"extensible\":true,\"frozen\":false,\"sealed\":false,\"ownPropertyLength\":0,\"preview\":{\"timestamp\":0}}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
   "stacktrace": null,
   "frame": null,
   "groupId": null,
   "userProvidedStyles": null,
   "notes": null,
   "indent": 0
 }));
 
@@ -46,20 +48,22 @@ stubPreparedMessages.set("asdf()", new C
   "id": "1",
   "allowRepeating": true,
   "source": "javascript",
   "timeStamp": 1479159921377,
   "type": "result",
   "helperType": null,
   "level": "error",
   "messageText": "ReferenceError: asdf is not defined",
-  "parameters": {
-    "type": "undefined"
-  },
-  "repeatId": "{\"frame\":{\"source\":\"debugger eval code\",\"line\":1,\"column\":1},\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":\"ReferenceError: asdf is not defined\",\"parameters\":{\"type\":\"undefined\"},\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
+  "parameters": [
+    {
+      "type": "undefined"
+    }
+  ],
+  "repeatId": "{\"frame\":{\"source\":\"debugger eval code\",\"line\":1,\"column\":1},\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":\"ReferenceError: asdf is not defined\",\"parameters\":[{\"type\":\"undefined\"}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
   "stacktrace": null,
   "frame": {
     "source": "debugger eval code",
     "line": 1,
     "column": 1
   },
   "groupId": null,
   "exceptionDocURL": "https://developer.mozilla.org/docs/Web/JavaScript/Reference/Errors/Not_defined?utm_source=mozilla&utm_medium=firefox-console-errors&utm_campaign=default",
@@ -72,20 +76,22 @@ stubPreparedMessages.set("1 + @", new Co
   "id": "1",
   "allowRepeating": true,
   "source": "javascript",
   "timeStamp": 1479159921399,
   "type": "result",
   "helperType": null,
   "level": "error",
   "messageText": "SyntaxError: illegal character",
-  "parameters": {
-    "type": "undefined"
-  },
-  "repeatId": "{\"frame\":{\"source\":\"debugger eval code\",\"line\":1,\"column\":4},\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":\"SyntaxError: illegal character\",\"parameters\":{\"type\":\"undefined\"},\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
+  "parameters": [
+    {
+      "type": "undefined"
+    }
+  ],
+  "repeatId": "{\"frame\":{\"source\":\"debugger eval code\",\"line\":1,\"column\":4},\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":\"SyntaxError: illegal character\",\"parameters\":[{\"type\":\"undefined\"}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
   "stacktrace": null,
   "frame": {
     "source": "debugger eval code",
     "line": 1,
     "column": 4
   },
   "groupId": null,
   "exceptionDocURL": "https://developer.mozilla.org/docs/Web/JavaScript/Reference/Errors/Illegal_character?utm_source=mozilla&utm_medium=firefox-console-errors&utm_campaign=default",
@@ -97,41 +103,43 @@ stubPreparedMessages.set("1 + @", new Co
 stubPreparedMessages.set("inspect({a: 1})", new ConsoleMessage({
   "id": "1",
   "allowRepeating": true,
   "source": "javascript",
   "timeStamp": 1499776070751,
   "type": "result",
   "helperType": "inspectObject",
   "level": "log",
-  "parameters": {
-    "type": "object",
-    "actor": "server1.conn0.child1/obj35",
-    "class": "Object",
-    "extensible": true,
-    "frozen": false,
-    "sealed": false,
-    "ownPropertyLength": 1,
-    "preview": {
-      "kind": "Object",
-      "ownProperties": {
-        "a": {
-          "configurable": true,
-          "enumerable": true,
-          "writable": true,
-          "value": 1
-        }
-      },
-      "ownSymbols": [],
-      "ownPropertiesLength": 1,
-      "ownSymbolsLength": 0,
-      "safeGetterValues": {}
+  "parameters": [
+    {
+      "type": "object",
+      "actor": "server1.conn0.child1/obj35",
+      "class": "Object",
+      "extensible": true,
+      "frozen": false,
+      "sealed": false,
+      "ownPropertyLength": 1,
+      "preview": {
+        "kind": "Object",
+        "ownProperties": {
+          "a": {
+            "configurable": true,
+            "enumerable": true,
+            "writable": true,
+            "value": 1
+          }
+        },
+        "ownSymbols": [],
+        "ownPropertiesLength": 1,
+        "ownSymbolsLength": 0,
+        "safeGetterValues": {}
+      }
     }
-  },
-  "repeatId": "{\"frame\":null,\"groupId\":null,\"indent\":0,\"level\":\"log\",\"parameters\":{\"type\":\"object\",\"actor\":\"server1.conn0.child1/obj35\",\"class\":\"Object\",\"extensible\":true,\"frozen\":false,\"sealed\":false,\"ownPropertyLength\":1,\"preview\":{\"kind\":\"Object\",\"ownProperties\":{\"a\":{\"configurable\":true,\"enumerable\":true,\"writable\":true,\"value\":1}},\"ownSymbols\":[],\"ownPropertiesLength\":1,\"ownSymbolsLength\":0,\"safeGetterValues\":{}}},\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
+  ],
+  "repeatId": "{\"frame\":null,\"groupId\":null,\"indent\":0,\"level\":\"log\",\"parameters\":[{\"type\":\"object\",\"actor\":\"server1.conn0.child1/obj35\",\"class\":\"Object\",\"extensible\":true,\"frozen\":false,\"sealed\":false,\"ownPropertyLength\":1,\"preview\":{\"kind\":\"Object\",\"ownProperties\":{\"a\":{\"configurable\":true,\"enumerable\":true,\"writable\":true,\"value\":1}},\"ownSymbols\":[],\"ownPropertiesLength\":1,\"ownSymbolsLength\":0,\"safeGetterValues\":{}}}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
   "stacktrace": null,
   "frame": null,
   "groupId": null,
   "userProvidedStyles": null,
   "notes": null,
   "indent": 0
 }));
 
@@ -144,20 +152,22 @@ stubPreparedMessages.set("longString mes
   "helperType": null,
   "level": "error",
   "messageText": {
     "type": "longString",
     "initial": "Error: Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Lon",
     "length": 110007,
     "actor": "server1.conn0.child1/longString37"
   },
-  "parameters": {
-    "type": "undefined"
-  },
-  "repeatId": "{\"frame\":null,\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":{\"type\":\"longString\",\"initial\":\"Error: Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Lon\",\"length\":110007,\"actor\":\"server1.conn0.child1/longString37\"},\"parameters\":{\"type\":\"undefined\"},\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
+  "parameters": [
+    {
+      "type": "undefined"
+    }
+  ],
+  "repeatId": "{\"frame\":null,\"groupId\":null,\"indent\":0,\"level\":\"error\",\"messageText\":{\"type\":\"longString\",\"initial\":\"Error: Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Long error Lon\",\"length\":110007,\"actor\":\"server1.conn0.child1/longString37\"},\"parameters\":[{\"type\":\"undefined\"}],\"source\":\"javascript\",\"type\":\"result\",\"userProvidedStyles\":null}",
   "stacktrace": null,
   "frame": null,
   "groupId": null,
   "userProvidedStyles": null,
   "notes": null,
   "indent": 0
 }));
 
--- 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
@@ -37,29 +37,35 @@ describe("Release actor enhancer:", () =
       // 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;
+
       const logCount = logLimit + 1;
       const packet = clonePacket(stubPackets.get(
         "console.assert(false, {message: 'foobar'})"));
-      const secondMessageActor = packet.message.arguments[0].actor;
+      const thirdMessageActor = packet.message.arguments[0].actor;
 
       for (let i = 1; i <= logCount; i++) {
         packet.message.arguments.push(`message num ${i}`);
         dispatch(actions.messageAdd(packet));
       }
 
-      expect(releasedActors.length).toBe(2);
+      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);
@@ -75,16 +81,23 @@ describe("Release actor enhancer:", () =
       const firstMessage = messages.first();
       const firstMessageActor = firstMessage.parameters[1].actor;
 
       const packet = clonePacket(stubPackets.get(
         "console.assert(false, {message: 'foobar'})"));
       const secondMessageActor = packet.message.arguments[0].actor;
       dispatch(actions.messageAdd(packet));
 
+      // Add an evaluation result message (see Bug 1408321).
+      const evaluationResultPacket = stubPackets.get("new Date(0)");
+      dispatch(actions.messageAdd(evaluationResultPacket));
+      const thirdMessageActor = evaluationResultPacket.result.actor;
+
+      // Kick-off the actor release.
       dispatch(actions.messagesClear());
 
-      expect(releasedActors.length).toBe(2);
+      expect(releasedActors.length).toBe(3);
       expect(releasedActors).toInclude(firstMessageActor);
       expect(releasedActors).toInclude(secondMessageActor);
+      expect(releasedActors).toInclude(thirdMessageActor);
     });
   });
 });
--- a/devtools/client/webconsole/new-console-output/utils/messages.js
+++ b/devtools/client/webconsole/new-console-output/utils/messages.js
@@ -247,28 +247,28 @@ function transformEvaluationResultPacket
     exceptionDocURL,
     frame,
     result,
     helperResult,
     timestamp: timeStamp,
     notes,
   } = packet;
 
-  let parameters = helperResult && helperResult.object
+  const parameter = helperResult && helperResult.object
     ? helperResult.object
     : result;
 
   const level = messageText ? MESSAGE_LEVEL.ERROR : MESSAGE_LEVEL.LOG;
   return new ConsoleMessage({
     source: MESSAGE_SOURCE.JAVASCRIPT,
     type: MESSAGE_TYPE.RESULT,
     helperType: helperResult ? helperResult.type : null,
     level,
     messageText,
-    parameters,
+    parameters: [parameter],
     exceptionDocURL,
     frame,
     timeStamp,
     notes,
   });
 }
 
 // Helpers