Fix dispose called on null during ClientHandler disposal (#824)

This fixes #823.
GoogleChromeLabs · Jul 21, 2023 · 48738ec · 48738ec
1 parent a52d8ed
commit 48738ec
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 10 deletions.
diff --git a/third_party/pcsc-lite/naclport/server_clients_management/src/client-handler-unittest.js b/third_party/pcsc-lite/naclport/server_clients_management/src/client-handler-unittest.js
@@ -17,8 +17,10 @@
 
 goog.require('GoogleSmartCard.PcscLiteServerClientsManagement.ClientHandler');
 goog.require('goog.Promise');
+goog.require('goog.reflect');
 goog.require('goog.testing');
 goog.require('goog.testing.MockControl');
+goog.require('goog.testing.asserts');
 goog.require('goog.testing.jsunit');
 goog.require('goog.testing.messaging.MockMessageChannel');
 
@@ -44,6 +46,18 @@ let mockServerMessageChannel;
 let mockClientMessageChannel;
 const stubPermissionsChecker = new StubPermissionsChecker();
 
+/**
+ * @param {!goog.testing.messaging.MockMessageChannel} mockMessageChannel
+ */
+function disposeOfMockMessageChannel(mockMessageChannel) {
+  mockMessageChannel.dispose();
+  // Hack: call the protected disposeInternal() method in order to trigger the
+  // disposal notifications; the MockMessageChannel.dispose() doesn't call
+  // them.
+  mockMessageChannel[goog.reflect.objectProperty(
+      'disposeInternal', mockMessageChannel)]();
+}
+
 goog.exportSymbol('testClientHandler', {
   'setUp': function() {
     mockControl = new goog.testing.MockControl();
@@ -69,5 +83,25 @@ goog.exportSymbol('testClientHandler', {
 
     handler.dispose();
   },
+
+  'testClientChannelDisposed': function() {
+    const handler = new ClientHandler(
+        mockServerMessageChannel, goog.Promise.resolve(),
+        mockClientMessageChannel, EXTENSION_ID_A);
+    disposeOfMockMessageChannel(mockClientMessageChannel);
+    assert(
+        'ClientHandler should dispose on client channel disposal',
+        handler.isDisposed());
+  },
+
+  'testServerChannelDisposed': function() {
+    const handler = new ClientHandler(
+        mockServerMessageChannel, goog.Promise.resolve(),
+        mockClientMessageChannel, EXTENSION_ID_A);
+    disposeOfMockMessageChannel(mockServerMessageChannel);
+    assert(
+        'ClientHandler should dispose on server channel disposal',
+        handler.isDisposed());
+  },
 });
 });  // goog.scope
diff --git a/third_party/pcsc-lite/naclport/server_clients_management/src/client-handler.js b/third_party/pcsc-lite/naclport/server_clients_management/src/client-handler.js
@@ -228,6 +228,7 @@ ClientHandler.prototype.disposeInternal = function() {
   this.deferredProcessor_ = null;
 
   this.serverRequester_.dispose();
+  this.serverRequester_ = null;
 
   this.clientMessageChannel_.dispose();
   this.clientMessageChannel_ = null;
@@ -317,14 +318,6 @@ ClientHandler.prototype.addChannelDisposedListeners_ = function() {
  * @private
  */
 ClientHandler.prototype.serverRequesterDisposedListener_ = function() {
-  if (this.isDisposed())
-    return;
-
-  // Note: this assignment is important because it prevents from sending of any
-  // messages through the server message channel, which is normally happening
-  // during the disposal process.
-  this.serverRequester_ = null;
-
   this.dispose();
 };
 

diff --git a/third_party/pcsc-lite/naclport/server_clients_management/src/server-request-handler.js b/third_party/pcsc-lite/naclport/server_clients_management/src/server-request-handler.js
@@ -181,8 +181,6 @@ Pcsc.ServerRequestHandler = class extends goog.Disposable {
 
   /** @override */
   disposeInternal() {
-    super.disposeInternal();
-
     if (this.serverRequester_) {
       this.serverRequester_.dispose();
       this.serverRequester_ = null;
@@ -208,6 +206,8 @@ Pcsc.ServerRequestHandler = class extends goog.Disposable {
     // sendServerDeleteHandlerMessage_ method is called, because the latter
     // accesses this member.
     this.serverMessageChannel_ = null;
+
+    super.disposeInternal();
   }
 
   /**