Moving It2Me confirmation prompt into the Validation callback flow

remoting/host/it2me/it2me_host.cc

Index: remoting/host/it2me/it2me_host.cc
diff --git a/remoting/host/it2me/it2me_host.cc b/remoting/host/it2me/it2me_host.cc
index 9aefebb241cac6ecfa5efc2b1e0440922dff87ce..222c495894d756f0afbeadbc50696bdba288a927 100644
--- a/remoting/host/it2me/it2me_host.cc
+++ b/remoting/host/it2me/it2me_host.cc
@@ -99,7 +99,10 @@ void It2MeHost::Connect() {
 
   // Switch to the network thread to start the actual connection.
   host_context_->network_task_runner()->PostTask(
-      FROM_HERE, base::Bind(&It2MeHost::ShowConfirmationPrompt, this));
+      FROM_HERE, base::Bind(&It2MeHost::SetState, this, kStarting, ""));

[Sergey Ulanov, 2016/09/09 18:50:40]

Call SetState() in ReadPolicyAndConnect()?

[joedow, 2016/09/10 02:50:36]

Done.

+
+  host_context_->network_task_runner()->PostTask(
+      FROM_HERE, base::Bind(&It2MeHost::ReadPolicyAndConnect, this));
 }
 
 void It2MeHost::Disconnect() {
@@ -150,56 +153,16 @@ void It2MeHost::RequestNatPolicy() {
     UpdateNatPolicy(nat_traversal_enabled_);
 }
 
-void It2MeHost::ShowConfirmationPrompt() {
-  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
-
-  SetState(kStarting, "");
-
-  std::unique_ptr<It2MeConfirmationDialog> confirmation_dialog =
-      confirmation_dialog_factory_->Create();
-
-  // TODO(dcaiafa): Remove after dialog implementations for all platforms exist.
-  if (!confirmation_dialog) {
-    ReadPolicyAndConnect();
-    return;
-  }
-
-  confirmation_dialog_proxy_.reset(
-      new It2MeConfirmationDialogProxy(host_context_->ui_task_runner(),
-                                       std::move(confirmation_dialog)));
-
-  confirmation_dialog_proxy_->Show(
-      base::Bind(&It2MeHost::OnConfirmationResult, base::Unretained(this)));
-}
-
-void It2MeHost::OnConfirmationResult(It2MeConfirmationDialog::Result result) {
-  switch (result) {
-    case It2MeConfirmationDialog::Result::OK:
-      ReadPolicyAndConnect();
-      break;
-
-    case It2MeConfirmationDialog::Result::CANCEL:
-      DisconnectOnNetworkThread();
-      break;
-
-    default:
-      NOTREACHED();
-      return;
-  }
-}
-
 void It2MeHost::ReadPolicyAndConnect() {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
   DCHECK_EQ(kStarting, state_);
 
   // Only proceed to FinishConnect() if at least one policy update has been
-  // received.
+  // received.  Otherwise, create the policy watcher and thunk the connect.
   if (policy_received_) {
     FinishConnect();
   } else {
-    // Otherwise, create the policy watcher, and thunk the connect.
-    pending_connect_ =
-        base::Bind(&It2MeHost::FinishConnect, this);
+    pending_connect_ = base::Bind(&It2MeHost::FinishConnect, this);
   }
 }
 
@@ -311,10 +274,11 @@ void It2MeHost::OnClientConnected(const std::string& jid) {
   // host is destroyed in OnClientDisconnected() after the first connection.
   CHECK_NE(state_, kConnected);
 
-  std::string client_username = jid;
-  size_t pos = client_username.find('/');
-  if (pos != std::string::npos)
-    client_username.replace(pos, std::string::npos, "");
+  std::string client_username;
+  if (!SplitJidResource(jid, &client_username, /*resource=*/nullptr)) {
+    LOG(WARNING) << "Incorrectly formatted JID received: " << jid;
+    client_username = jid;
+  }
 
   HOST_LOG << "Client " << client_username << " connected.";
 
@@ -522,15 +486,18 @@ void It2MeHost::OnReceivedSupportID(
 void It2MeHost::ValidateConnectionDetails(
     const std::string& remote_jid,
     const protocol::ValidatingAuthenticator::ResultCallback& result_callback) {
+  // First ensure the JID we received is valid.
+  std::string client_username;
+  if (!SplitJidResource(remote_jid, &client_username, /*resource=*/nullptr)) {
+    LOG(ERROR) << "Rejecting incoming connection from " << remote_jid
+               << ": Invalid JID.";
+    result_callback.Run(
+        protocol::ValidatingAuthenticator::Result::ERROR_INVALID_ACCOUNT);
+    return;
+  }
+
   // Check the client domain policy.
   if (!required_client_domain_.empty()) {
-    std::string client_username;
-    if (!SplitJidResource(remote_jid, &client_username, /*resource=*/nullptr)) {
-      LOG(ERROR) << "Rejecting incoming connection from " << remote_jid
-                 << ": Invalid JID.";
-      result_callback.Run(ValidationResult::ERROR_INVALID_ACCOUNT);
-      return;
-    }
     if (!base::EndsWith(client_username,
                         std::string("@") + required_client_domain_,
                         base::CompareCase::INSENSITIVE_ASCII)) {
@@ -541,7 +508,35 @@ void It2MeHost::ValidateConnectionDetails(
     }
   }
 
-  result_callback.Run(ValidationResult::SUCCESS);
+  // Show a confirmation dialog to the user to allow them to confirm/reject it.
+  std::unique_ptr<It2MeConfirmationDialog> confirmation_dialog =
+      confirmation_dialog_factory_->Create();
+
+  if (!confirmation_dialog) {

[Sergey Ulanov, 2016/09/09 18:46:57]

Add a comment that this is only temporary until we have the dialog implemented
on all platforms and add a TODO to fix this?

[joedow, 2016/09/10 02:50:36]

Done.

+    result_callback.Run(ValidationResult::SUCCESS);
+    return;
+  }
+
+  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(
+    const protocol::ValidatingAuthenticator::ResultCallback& result_callback,
+    It2MeConfirmationDialog::Result result) {
+  switch (result) {
+    case It2MeConfirmationDialog::Result::OK:
+      result_callback.Run(ValidationResult::SUCCESS);
+      break;
+
+    case It2MeConfirmationDialog::Result::CANCEL:
+      result_callback.Run(ValidationResult::ERROR_REJECTED_BY_USER);
+      break;
+  }
 }
 
 It2MeHostFactory::It2MeHostFactory() {}