Webapp share dialog is not closed when client end of the connection is closed

remoting/host/it2me/it2me_host.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/it2me/it2me_host.h"
 
 #include <cstdint>
 #include <memory>
 #include <string>
 #include <utility>
@@ skipping 238 matching lines @@
   SetState(kRequestedAccessCode, "");
   return;
 }
 
 void It2MeHost::OnAccessDenied(const std::string& jid) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   ++failed_login_attempts_;
   if (failed_login_attempts_ == kMaxLoginAttempts) {
     DisconnectOnNetworkThread();
+  } else if (confirmation_dialog_proxy_) {

[Sergey Ulanov, 2017/01/20 20:05:43]

Can we just delete ConfirmationDialogProxy? ( i.e.
confirmation_dialog_proxy_.reset(); ). 
I would also put it above line 256.

[joedow, 2017/03/14 16:46:36]

Acknowledged.

+    confirmation_dialog_proxy_->Cancel();

[Sergey Ulanov, 2017/01/20 20:05:43]

If I understand correctly this will call OnConfirmationResult(CANCEL), which
will then call DisconnectOnNetworkThread(). I'm not sure we want this behavior.
If user tries to connect, then cancels, then tries to connect again we should
let them connect again instead of shutting down the host.


Also note the host allows multiple concurrent connections in authenticating
state, so potentially OnAccessDenied() can be called for a connection/jid that's
different from the one for which the current confirmation_dialog_proxy_ was
created. It looks already broken in the current host, I've filed this bug:
crbug.com/683317
I don't think we care too much about that scenario: just failing both
connections may be appropriate, but we certainly want to avoid crashes. Since
Cancel() is asynchronous, it makes it harder to make sure this scenario is
handled properly.

[joedow, 2017/03/14 16:46:36]

resetting the proxy will prevent the dialog result from being returned to the
webapp which is what we want.  I think we should reset the connection state as
well here to allow for a reconnect attempt.

   }
 }
 
 void It2MeHost::OnClientConnected(const std::string& jid) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   // ChromotingHost doesn't allow multiple concurrent connection and the
   // host is destroyed in OnClientDisconnected() after the first connection.
   CHECK_NE(state_, kConnected);
 
@@ skipping 291 matching lines @@
   DCHECK(context->ui_task_runner()->BelongsToCurrentThread());
 
   std::unique_ptr<PolicyWatcher> policy_watcher =
       PolicyWatcher::Create(policy_service, context->file_task_runner());
   return new It2MeHost(std::move(context), std::move(policy_watcher),
                        It2MeConfirmationDialog::Create(), observer,
                        std::move(signal_strategy), username, directory_bot_jid);
 }
 
 }  // namespace remoting