Improve curtain mode error message.

remoting/host/remoting_me2me_host.cc

 // Copyright (c) 2012 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.
 //
 // This file implements a standalone host process for Me2Me.
 
 #include <string>
 
 #include "base/at_exit.h"
 #include "base/bind.h"
@@ skipping 194 matching lines @@
   bool OnNatPolicyUpdate(bool nat_traversal_enabled);
   bool OnCurtainPolicyUpdate(bool curtain_required);
   bool OnHostTalkGadgetPrefixPolicyUpdate(const std::string& talkgadget_prefix);
 
   void StartHostStatusService();
 
   void StartHost();
 
   void OnAuthFailed();
 
-  void RejectAuthenticatingClient();
+  void OnCurtainModeFailed();
+
+  void OnRemoteSessionSwitchedToConsole();
 
   // Invoked when the user uses the Disconnect windows to terminate
   // the sessions, or when the local session is activated in curtain mode.
   void OnDisconnectRequested();
 
   void RestartHost();
 
   void RestartOnHostShutdown();
 
   void Shutdown(int exit_code);
@@ skipping 79 matching lines @@
       desktop_session_connector_(NULL),
 #endif  // defined(REMOTING_MULTI_PROCESS)
       ALLOW_THIS_IN_INITIALIZER_LIST(self_(this)),
       exit_code_out_(exit_code_out) {
   // Create a NetworkChangeNotifier for use by the signalling connector.
   network_change_notifier_.reset(net::NetworkChangeNotifier::Create());
 
   // Create the platform-specific curtain-mode implementation.
   // TODO(wez): Create this on the network thread?
   curtain_ = CurtainMode::Create(
-      base::Bind(&HostProcess::OnDisconnectRequested,
+      base::Bind(&HostProcess::OnRemoteSessionSwitchedToConsole,
                  base::Unretained(this)),
-      base::Bind(&HostProcess::RejectAuthenticatingClient,
+      base::Bind(&HostProcess::OnCurtainModeFailed,
                  base::Unretained(this)));
 
   StartOnUiThread();
 }
 
 HostProcess::~HostProcess() {
   // Verify that UI components have been torn down.
   DCHECK(!config_watcher_);
   DCHECK(!daemon_channel_);
   DCHECK(!desktop_environment_factory_);
@@ skipping 598 matching lines @@
 
   host_->Start(xmpp_login_);
 
   CreateAuthenticatorFactory();
 }
 
 void HostProcess::OnAuthFailed() {
   Shutdown(kInvalidOauthCredentialsExitCode);
 }
 
-void HostProcess::RejectAuthenticatingClient() {
-  DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-  DCHECK(host_);
-  host_->RejectAuthenticatingClient();

[Jamie, 2012/11/28 23:52:32]

There's a comment on this method saying that it should only be used from
OnClientAuthenticated(), so I don't think it's the right thing to use here,
although it's not the cause of the bogus error message.

[rmsousa, 2012/11/29 00:11:14]

Right now this is only called when activating the curtain mode failed (so
perhaps OnCurtainModeActivationFailed would be more explicit), which is
ultimately done from an OnClientAuthenticated callback from
CurtainingHostObserver.

The problem with using OnDisconnectRequested() here is that it immediately
disconnects the session, and calls the OnClientDisconnected callback in all
HostObservers. This means that if it's called from an OnClientAuthenticated
handler, some HostObservers will receive the OnClientDisconnected callback
*before* the OnClientAuthenticated (and the CurtainingHostObserver will call
OnClientDisconnected *from* an OnClientAuthenticated callstack)

I'd prefer to change the messaging on the client side from "the PIN was wrong"
to something on the lines of "the host rejected your connection, due to a wrong
PIN or a host security policy", since there could be more policy-related reasons
why a host rejects a client during authentication (username policy, etc).

[Jamie, 2012/11/29 01:05:58]

which is
You're right. I'd forgotten that we are effectively calling it in the right
context, it's just not obvious from a cursory inspection of the code, and it
feels brittle to rely on it.
Yuck. Okay, I'll switch back to calling RejectAuthenticatingClient. Thanks for
the drive-by.
Since the most likely reason a user might see this message is that they've
entered an invalid PIN, I don't want to make the error message more complicated.

[Sergey Ulanov, 2012/11/30 01:09:39]

+1. A message that says "maybe your entered incorrect PIN or maybe something
else is wrong" is not helpful and we should avoid it.

+void HostProcess::OnCurtainModeFailed() {
+  LOG(ERROR) << "Curtain mode failed to activate. Closing connection.";
+  OnDisconnectRequested();
+}
+
+void HostProcess::OnRemoteSessionSwitchedToConsole() {
+  LOG(INFO) << "The remote session switched was to the console."
+             << " Closing connection.";
+  OnDisconnectRequested();
 }
 
 // Invoked when the user uses the Disconnect windows to terminate
 // the sessions, or when the local session is activated in curtain mode.
 void HostProcess::OnDisconnectRequested() {
   if (!context_->network_task_runner()->BelongsToCurrentThread()) {
     context_->network_task_runner()->PostTask(FROM_HERE,
         base::Bind(&HostProcess::OnDisconnectRequested, this));
     return;
   }
@@ skipping 172 matching lines @@
             user32.GetFunctionPointer("SetProcessDPIAware"));
     set_process_dpi_aware();
   }
 
   // CommandLine::Init() ignores the passed |argc| and |argv| on Windows getting
   // the command line from GetCommandLineW(), so we can safely pass NULL here.
   return main(0, NULL);
 }
 
 #endif  // defined(OS_WIN)