Bug 1507125 - Front should throw when sending packet without actorID or destination;r=ochameau
☠☠ backed out by eefaa80b6745 ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 28 Nov 2018 17:36:25 +0000
changeset 507763 2b9aaf2f3b58ee8d39ce4d4b2c4f4c935894d030
parent 507762 641de22d6480c8656b5dc6371c57f8b666771c19
child 507764 f6632f5349d67c87d41aab7b07577abd8cc58a48
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1507125
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1507125 - Front should throw when sending packet without actorID or destination;r=ochameau Depends on D13137. I could use help to write the test in a better. I believe there is a cleaner way to create the front here? I also had other suggestions for making the fronts more robust in the bug. Let me know if you think I should try to investigate them more. Differential Revision: https://phabricator.services.mozilla.com/D13138
devtools/server/tests/unit/test_front_destroy.js
devtools/server/tests/unit/xpcshell.ini
devtools/shared/protocol.js
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/unit/test_front_destroy.js
@@ -0,0 +1,31 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test that fronts throw errors if they are called after being destroyed.
+ */
+
+"use strict";
+
+add_task(async function test() {
+  DebuggerServer.init();
+  DebuggerServer.registerAllActors();
+
+  info("Create and connect the DebuggerClient");
+  const transport = DebuggerServer.connectPipe();
+  const client = new DebuggerClient(transport);
+  await client.connect();
+
+  info("Get the device front and check calling getDescription() on it");
+  const front = await client.mainRoot.getFront("device");
+  const description = await front.getDescription();
+  ok(!!description, "Check that the getDescription() method returns a valid response.");
+
+  info("Destroy the device front and try calling getDescription again");
+  front.destroy();
+  Assert.throws(() => front.getDescription(),
+    /Can not send request because front 'device' is already destroyed\./,
+    "Check device front throws when getDescription() is called after destroy()");
+
+  await client.close();
+});
--- a/devtools/server/tests/unit/xpcshell.ini
+++ b/devtools/server/tests/unit/xpcshell.ini
@@ -78,16 +78,17 @@ skip-if = (verify && !debug && (os == 'w
 [test_threadlifetime-02.js]
 [test_threadlifetime-03.js]
 [test_threadlifetime-04.js]
 [test_threadlifetime-05.js]
 [test_threadlifetime-06.js]
 [test_functiongrips-01.js]
 [test_frameclient-01.js]
 [test_frameclient-02.js]
+[test_front_destroy.js]
 [test_nativewrappers.js]
 [test_nodelistactor.js]
 [test_eval-01.js]
 [test_eval-02.js]
 [test_eval-03.js]
 [test_eval-04.js]
 [test_eval-05.js]
 [test_format_command.js]
--- a/devtools/shared/protocol.js
+++ b/devtools/shared/protocol.js
@@ -1354,24 +1354,30 @@ Front.prototype = extend(Pool.prototype,
    * Subclasses should override this.
    */
   form: function(form) {},
 
   /**
    * Send a packet on the connection.
    */
   send: function(packet) {
-    if (packet.to) {
-      this.conn._transport.send(packet);
-    } else {
+    if (!packet.to) {
       packet.to = this.actorID;
-      // The connection might be closed during the promise resolution
-      if (this.conn._transport) {
-        this.conn._transport.send(packet);
-      }
+    }
+
+    // If packet.to and this.actorID are not available, the request will not be able to
+    // complete. The front was probably destroyed earlier.
+    if (!packet.to) {
+      throw new Error(
+        `Can not send request because front '${this.typeName}' is already destroyed.`);
+    }
+
+    // The connection might be closed during the promise resolution
+    if (this.conn._transport) {
+      this.conn._transport.send(packet);
     }
   },
 
   /**
    * Send a two-way request on the connection.
    */
   request: function(packet) {
     const deferred = defer();