Simplify PIN dialog closing assumptions

Don't require the implementations of the SecurityTokenPinDialogHost
interface to run any callback when CloseSecurityTokenPinDialog() is
called. The caller knows anyway that the dialog is being closed, so it's
just unnecessary and error-prone to rely on every implementation to
notify the caller about the closing.

This is a small refactoring, which should have no effect on the
currently implemented behavior. This change should slightly simplify
writing other implementations of the PIN dialog host - for the
Login/Lock screens.

Bug: 964069
Change-Id: Ied94a1e90bb3f4063081427888d8fcc566719777
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1743729
Commit-Queue: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Igor <igorcov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685164}

chrome/browser/chromeos/certificate_provider/pin_dialog_manager.cc

@@ -120,19 +120,14 @@ bool PinDialogManager::CloseDialog(const std::string& extension_id) {
     return false;
   }
 
-  active_dialog_state_->host->CloseSecurityTokenPinDialog();
-
-  // The active dialog state should have been cleared by OnPinDialogClosed().
-  DCHECK(!active_dialog_state_);
-
-  last_response_closed_[extension_id] = true;
+  CloseActiveDialog();
   return true;
 }
 
 void PinDialogManager::ExtensionUnloaded(const std::string& extension_id) {
   if (active_dialog_state_ &&
       active_dialog_state_->extension_id == extension_id) {
-    CloseDialog(extension_id);
+    CloseActiveDialog();
   }
 
   last_response_closed_[extension_id] = false;
@@ -154,10 +149,8 @@ void PinDialogManager::AddPinDialogHost(
 
 void PinDialogManager::RemovePinDialogHost(
     SecurityTokenPinDialogHost* pin_dialog_host) {
-  if (active_dialog_state_ && active_dialog_state_->host == pin_dialog_host) {
-    pin_dialog_host->CloseSecurityTokenPinDialog();
-    DCHECK(!active_dialog_state_);
-  }
+  if (active_dialog_state_ && active_dialog_state_->host == pin_dialog_host)
+    CloseActiveDialog();
   DCHECK(base::Contains(added_dialog_hosts_, pin_dialog_host));
   base::Erase(added_dialog_hosts_, pin_dialog_host);
 }
@@ -201,4 +194,17 @@ SecurityTokenPinDialogHost* PinDialogManager::GetHostForNewDialog() {
   return added_dialog_hosts_.back();
 }
 
+void PinDialogManager::CloseActiveDialog() {
+  if (!active_dialog_state_)
+    return;
+
+  // Ignore any further callbacks from the host. Instead of relying on the host
+  // to call the closing callback, run OnPinDialogClosed() below explicitly.
+  weak_factory_.InvalidateWeakPtrs();
+
+  active_dialog_state_->host->CloseSecurityTokenPinDialog();
+  OnPinDialogClosed();
+  DCHECK(!active_dialog_state_);
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/certificate_provider/pin_dialog_manager.h

@@ -147,6 +147,9 @@ class PinDialogManager final {
   // no host has been added).
   SecurityTokenPinDialogHost* GetHostForNewDialog();
 
+  // Closes the active dialog, if there's any, and runs the necessary callbacks.
+  void CloseActiveDialog();
+
   // Tells whether user closed the last request PIN dialog issued by an
   // extension. The extension_id is the key and value is true if user closed the
   // dialog. Used to determine if the limit of dialogs rejected by the user has

chrome/browser/chromeos/certificate_provider/security_token_pin_dialog_host.h

@@ -57,9 +57,10 @@ class SecurityTokenPinDialogHost {
   //     (note that a non-empty error does NOT disable the user input per se).
   // |attempts_left| - when non-negative, the UI should indicate this number to
   //     the user; otherwise must be equal to -1.
-  // |pin_entered_callback| - will be called when the user submits the input.
-  // |pin_dialog_closed_callback| - will be called when the dialog is closed
-  //     (either by the user or programmatically).
+  // |pin_entered_callback| - called when the user submits the input.
+  // |pin_dialog_closed_callback| - called when the dialog is closed (either by
+  //     the user or programmatically; it's optional whether to call it after
+  //     CloseSecurityTokenPinDialog()).
   virtual void ShowSecurityTokenPinDialog(
       const std::string& caller_extension_name,
       SecurityTokenPinCodeType code_type,
@@ -69,8 +70,8 @@ class SecurityTokenPinDialogHost {
       SecurityTokenPinEnteredCallback pin_entered_callback,
       SecurityTokenPinDialogClosedCallback pin_dialog_closed_callback) = 0;
 
-  // Closes the currently shown PIN dialog, if there's any. This will
-  // immediately trigger |pin_dialog_closed_callback|.
+  // Closes the currently shown PIN dialog, if there's any. The implementation
+  // is NOT required to run |pin_dialog_closed_callback| after the closing.
   virtual void CloseSecurityTokenPinDialog() = 0;
 };