It2Me Host changes to better support Confirmation Dialog

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 375 matching lines @@
 void It2MeHost::SetState(It2MeHostState state,
                          const std::string& error_message) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   switch (state_) {
     case kDisconnected:
       DCHECK(state == kStarting ||
              state == kError) << state;
       break;
     case kStarting:
       DCHECK(state == kRequestedAccessCode ||

[Sergey Ulanov, 2016/10/27 22:44:12]

maybe update formatting for all DCHECKS here to make it consistent

[joedow, 2016/10/31 17:16:07]

I'm going to update the changed conditions to match the old format as I prefer
scanning the conditions vertically.

              state == kDisconnected ||
              state == kError ||
              state == kInvalidDomainError) << state;
       break;
     case kRequestedAccessCode:
       DCHECK(state == kReceivedAccessCode ||
              state == kDisconnected ||
              state == kError) << state;
       break;
     case kReceivedAccessCode:
-      DCHECK(state == kConnected ||
-             state == kDisconnected ||
-             state == kError) << state;
+      DCHECK(state == kConnecting || state == kDisconnected || state == kError)
+          << state;
       break;
     case kConnected:
       DCHECK(state == kDisconnected ||
              state == kError) << state;
       break;
+    case kConnecting:

[Sergey Ulanov, 2016/10/27 22:44:12]

Put kConnecting before kConnected

[joedow, 2016/10/31 17:16:07]

Done.

+      DCHECK(state == kConnected || state == kDisconnected || state == kError)
+          << state;
+      break;
     case kError:
       DCHECK(state == kDisconnected) << state;
       break;
     case kInvalidDomainError:
       DCHECK(state == kDisconnected) << state;
       break;
   };
 
   state_ = state;
 
   // Post a state-change notification to the web-app.
   host_context_->ui_task_runner()->PostTask(
       FROM_HERE, base::Bind(&It2MeHost::Observer::OnStateChanged, observer_,
                             state, error_message));
 }
 
 bool It2MeHost::IsConnected() const {

[Sergey Ulanov, 2016/10/27 22:44:12]

This doesn't look like the right name for this method. Maybe rename it to
IsOnline()?

[joedow, 2016/10/31 17:16:07]

Changed.  Agreed that IsConnected is ambiguous since it means either connected
to the service or the client.

IsOnline is almost correct but the check contains the kRequestedAccessCode which
means it has sent a request but isn't technically online yet.

I'm going to call it IsRunning() since it encompasses the non-error states that
occur after the kStarting state.  Let me know if that sounds good.

   return state_ == kRequestedAccessCode || state_ == kReceivedAccessCode ||
-      state_ == kConnected;
+         state_ == kConnected || state_ == kConnecting;
 }
 
 void It2MeHost::OnReceivedSupportID(
     const std::string& support_id,
     const base::TimeDelta& lifetime,
     const std::string& error_message) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   if (!error_message.empty()) {
     SetState(kError, error_message);
@@ skipping 60 matching lines @@
                         std::string("@") + required_client_domain_,
                         base::CompareCase::INSENSITIVE_ASCII)) {
       LOG(ERROR) << "Rejecting incoming connection from " << remote_jid
                  << ": Domain mismatch.";
       result_callback.Run(ValidationResult::ERROR_INVALID_ACCOUNT);
       DisconnectOnNetworkThread();
       return;
     }
   }
 
+  HOST_LOG << "Client " << client_username << " connecting.";
+  SetState(kConnecting, std::string());
+
   // Show a confirmation dialog to the user to allow them to confirm/reject it.
   confirmation_dialog_proxy_.reset(new It2MeConfirmationDialogProxy(
       host_context_->ui_task_runner(), std::move(confirmation_dialog_)));
 
   confirmation_dialog_proxy_->Show(
       client_username, base::Bind(&It2MeHost::OnConfirmationResult,
                                   base::Unretained(this), result_callback));
 }
 
 void It2MeHost::OnConfirmationResult(
@@ skipping 27 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