Break noDialogs It2Me flag into 2 distinct flags for dialogs and notifications

Using the noDialogs flags is fine for an unattended scenario, however it
there may be cases where we want to provide a notification on the
desktop that a user is connected but still allow the remote user to
connect w/o prompting or requiring periodic reapproval.

This CL splits up the 'noDialogs' flag into 2 flags which control
suppression of the user dialogs (to allow / reapprove the remote
connection) and the desktop notification that is displayed.

Change-Id: Ifd2eb4f9270fadb4ffc2267bd2b1dfbe0ec38ca4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1962530Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#724766}

chrome/browser/chromeos/policy/remote_commands/crd_host_delegate.cc

@@ -55,7 +55,8 @@ constexpr char kCRDConnectAuth[] = "authServiceWithToken";
 constexpr char kCRDConnectXMPPServer[] = "xmppServerAddress";
 constexpr char kCRDConnectXMPPTLS[] = "xmppServerUseTls";
 constexpr char kCRDConnectDirectoryBot[] = "directoryBotJid";
-constexpr char kCRDConnectNoDialogs[] = "noDialogs";
+constexpr char kCRDConnectSuppressUserDialogs[] = "suppressUserDialogs";
+constexpr char kCRDConnectSuppressNotifications[] = "suppressNotifications";
 constexpr char kCRDTerminateUponInput[] = "terminateUponInput";
 
 // Connect message parameter values:
@@ -209,7 +210,8 @@ void CRDHostDelegate::StartCRDHostAndGetCode(
   connect_params.SetKey(kCRDConnectXMPPTLS, base::Value(true));
   connect_params.SetKey(kCRDConnectDirectoryBot,
                         base::Value(kCRDConnectDirectoryBotValue));
-  connect_params.SetKey(kCRDConnectNoDialogs, base::Value(true));
+  connect_params.SetKey(kCRDConnectSuppressUserDialogs, base::Value(true));
+  connect_params.SetKey(kCRDConnectSuppressNotifications, base::Value(true));
   connect_params.SetKey(kCRDTerminateUponInput,
                         base::Value(terminate_upon_input));
   connect_params_ = std::move(connect_params);

remoting/host/desktop_environment_options.cc

@@ -79,6 +79,14 @@ void DesktopEnvironmentOptions::set_enable_user_interface(bool enabled) {
   enable_user_interface_ = enabled;
 }
 
+bool DesktopEnvironmentOptions::enable_notifications() const {
+  return enable_notifications_;
+}
+
+void DesktopEnvironmentOptions::set_enable_notifications(bool enabled) {
+  enable_notifications_ = enabled;
+}
+
 bool DesktopEnvironmentOptions::terminate_upon_input() const {
   return terminate_upon_input_;
 }

remoting/host/desktop_environment_options.h

@@ -36,6 +36,9 @@ class DesktopEnvironmentOptions final {
   bool enable_user_interface() const;
   void set_enable_user_interface(bool enabled);
 
+  bool enable_notifications() const;
+  void set_enable_notifications(bool enabled);
+
   bool terminate_upon_input() const;
   void set_terminate_upon_input(bool enabled);
 
@@ -59,6 +62,9 @@ class DesktopEnvironmentOptions final {
   // True if a user-interactive window is showing up in it2me scenario.
   bool enable_user_interface_ = true;
 
+  // True if a notification should be shown when a remote user is connected.
+  bool enable_notifications_ = true;
+
   // True if the session should be terminated when local input is detected.
   bool terminate_upon_input_ = false;
 

remoting/host/it2me/it2me_host.cc

@@ -73,6 +73,15 @@ void It2MeHost::set_enable_dialogs(bool enable) {
 #endif
 }
 
+void It2MeHost::set_enable_notifications(bool enable) {
+#if defined(OS_CHROMEOS) || !defined(NDEBUG)
+  enable_notifications_ = enable;
+#else
+  NOTREACHED() << "It2MeHost::set_enable_notifications is only supported on "
+               << "ChromeOS";
+#endif
+}
+
 void It2MeHost::set_terminate_upon_input(bool terminate_upon_input) {
 #if defined(OS_CHROMEOS) || !defined(NDEBUG)
   terminate_upon_input_ = terminate_upon_input;
@@ -197,6 +206,7 @@ void It2MeHost::ConnectOnNetworkThread(
   // Create the host.
   DesktopEnvironmentOptions options(DesktopEnvironmentOptions::CreateDefault());
   options.set_enable_user_interface(enable_dialogs_);
+  options.set_enable_notifications(enable_notifications_);
   options.set_terminate_upon_input(terminate_upon_input_);
   host_.reset(new ChromotingHost(
       desktop_environment_factory_.get(), std::move(session_manager),

remoting/host/it2me/it2me_host.h

@@ -75,6 +75,11 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
   void set_enable_dialogs(bool enable);
   bool enable_dialogs() const { return enable_dialogs_; }
 
+  // Enable, disable, or query whether or not connection notifications are
+  // shown when a remote user has connected.
+  void set_enable_notifications(bool enable);
+  bool enable_notifications() const { return enable_notifications_; }
+
   // Enable or disable whether or not the session should be terminated if local
   // input is detected.
   void set_terminate_upon_input(bool terminate_upon_input);
@@ -200,6 +205,7 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
   std::string connecting_jid_;
 
   bool enable_dialogs_ = true;
+  bool enable_notifications_ = true;
   bool terminate_upon_input_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(It2MeHost);

remoting/host/it2me/it2me_host_unittest.cc

@@ -163,7 +163,7 @@ class It2MeHostTest : public testing::Test, public It2MeHost::Observer {
 
   void RunValidationCallback(const std::string& remote_jid);
 
-  void StartHost(bool enable_dialogs = true);
+  void StartHost(bool enable_dialogs = true, bool enable_notifications = true);
   void ShutdownHost();
 
   static base::ListValue MakeList(
@@ -268,7 +268,7 @@ void It2MeHostTest::StartupHostStateHelper(const base::Closure& quit_closure) {
                                       base::Unretained(this), quit_closure);
 }
 
-void It2MeHostTest::StartHost(bool enable_dialogs) {
+void It2MeHostTest::StartHost(bool enable_dialogs, bool enable_notifications) {
   if (!policies_) {
     policies_ = PolicyWatcher::GetDefaultPolicies();
   }
@@ -292,6 +292,11 @@ void It2MeHostTest::StartHost(bool enable_dialogs) {
     // false should only be run on ChromeOS.
     it2me_host_->set_enable_dialogs(enable_dialogs);
   }
+  if (!enable_notifications) {
+    // Only ChromeOS supports this method, so tests setting enable_dialogs to
+    // false should only be run on ChromeOS.
+    it2me_host_->set_enable_notifications(enable_notifications);
+  }
   auto register_host_request =
       std::make_unique<XmppRegisterSupportHostRequest>("fake_bot_jid");
   auto log_to_server = std::make_unique<XmppLogToServer>(
@@ -625,12 +630,18 @@ TEST_F(It2MeHostTest, MultipleConnectionsTriggerDisconnect) {
 }
 
 #if defined(OS_CHROMEOS)
-TEST_F(It2MeHostTest, ConnectRespectsNoDialogsParameter) {
+TEST_F(It2MeHostTest, ConnectRespectsSuppressDialogsParameter) {
   StartHost(false);
   EXPECT_FALSE(dialog_factory_->dialog_created());
   EXPECT_FALSE(
       GetHost()->desktop_environment_options().enable_user_interface());
 }
+
+TEST_F(It2MeHostTest, ConnectRespectsSuppressNotificationsParameter) {
+  StartHost(true, false);
+  EXPECT_FALSE(dialog_factory_->dialog_created());
+  EXPECT_FALSE(GetHost()->desktop_environment_options().enable_notifications());
+}
 #endif
 
 }  // namespace remoting

remoting/host/it2me/it2me_native_messaging_host.cc

@@ -242,8 +242,11 @@ void It2MeNativeMessagingHost::ProcessConnect(
   std::string username;
   message->GetString("userName", &username);
 
-  bool no_dialogs = false;
-  message->GetBoolean("noDialogs", &no_dialogs);
+  bool suppress_user_dialogs = false;
+  message->GetBoolean("suppressUserDialogs", &suppress_user_dialogs);
+
+  bool suppress_notifications = false;
+  message->GetBoolean("suppressNotifications", &suppress_notifications);
 
   bool terminate_upon_input = false;
   message->GetBoolean("terminateUponInput", &terminate_upon_input);
@@ -299,7 +302,8 @@ void It2MeNativeMessagingHost::ProcessConnect(
   // only supported on ChromeOS.
   it2me_host_ = factory_->CreateIt2MeHost();
 #if defined(OS_CHROMEOS) || !defined(NDEBUG)
-  it2me_host_->set_enable_dialogs(!no_dialogs);
+  it2me_host_->set_enable_dialogs(!suppress_user_dialogs);
+  it2me_host_->set_enable_notifications(!suppress_notifications);
   it2me_host_->set_terminate_upon_input(terminate_upon_input);
 #endif
   it2me_host_->Connect(host_context_->Copy(), std::move(policies),

remoting/host/it2me/it2me_native_messaging_host_unittest.cc

@@ -637,10 +637,10 @@ TEST_F(It2MeNativeMessagingHostTest, ConnectMultiple) {
 }
 
 TEST_F(It2MeNativeMessagingHostTest,
-       ConnectRespectsNoDialogsParameterOnChromeOsOnly) {
+       ConnectRespectsSuppressUserDialogsParameterOnChromeOsOnly) {
   int next_id = 1;
   base::DictionaryValue connect_message = CreateConnectMessage(next_id);
-  connect_message.SetBoolean("noDialogs", true);
+  connect_message.SetBoolean("suppressUserDialogs", true);
   WriteMessageToInputPipe(connect_message);
   VerifyConnectResponses(next_id);
 #if defined(OS_CHROMEOS) || !defined(NDEBUG)
@@ -653,6 +653,22 @@ TEST_F(It2MeNativeMessagingHostTest,
   VerifyDisconnectResponses(next_id);
 }
 
+TEST_F(It2MeNativeMessagingHostTest,
+       ConnectRespectsSuppressNotificationsParameterOnChromeOsOnly) {
+  int next_id = 1;
+  base::DictionaryValue connect_message = CreateConnectMessage(next_id);
+  connect_message.SetBoolean("suppressNotifications", true);
+  WriteMessageToInputPipe(connect_message);
+  VerifyConnectResponses(next_id);
+#if defined(OS_CHROMEOS) || !defined(NDEBUG)
+  EXPECT_FALSE(factory_raw_ptr_->host->enable_notifications());
+#else
+  EXPECT_TRUE(factory_raw_ptr_->host->enable_notifications());
+#endif
+  ++next_id;
+  WriteMessageToInputPipe(CreateDisconnectMessage(next_id));
+  VerifyDisconnectResponses(next_id);
+}
 
 // Verify non-Dictionary requests are rejected.
 TEST_F(It2MeNativeMessagingHostTest, WrongFormat) {

remoting/host/it2me_desktop_environment.cc

@@ -47,6 +47,7 @@ It2MeDesktopEnvironment::It2MeDesktopEnvironment(
   local_input_monitor_->StartMonitoringForClientSession(client_session_control);
 
   bool enable_user_interface = options.enable_user_interface();
+  bool enable_notifications = options.enable_notifications();
   // The host UI should be created on the UI thread.
 #if defined(OS_MACOSX)
   // Don't try to display any UI on top of the system's login screen as this
@@ -59,13 +60,22 @@ It2MeDesktopEnvironment::It2MeDesktopEnvironment(
   enable_user_interface = getuid() != 0;
 #endif  // defined(OS_MACOSX)
 
-  // Create the continue and disconnect windows.
+  // Create the continue window.  The implication of this window is that the
+  // session length will be limited.  If the user interface is disabled,
+  // then sessions will not have a maximum length enforced by the continue
+  // window timer.
   if (enable_user_interface) {
     continue_window_ = HostWindow::CreateContinueWindow();
     continue_window_.reset(new HostWindowProxy(
         caller_task_runner, ui_task_runner, std::move(continue_window_)));
     continue_window_->Start(client_session_control);
+  }
 
+  // Create the disconnect window on Mac/Windows/Linux or a tray notification
+  // on ChromeOS.  This has the effect of notifying the local user that
+  // someone has remotely connected to their machine and providing them with
+  // a disconnect button to terminate the connection.
+  if (enable_notifications) {
     disconnect_window_ = HostWindow::CreateDisconnectWindow();
     disconnect_window_.reset(new HostWindowProxy(
         caller_task_runner, ui_task_runner, std::move(disconnect_window_)));

remoting/test/it2me_cli_host.cc

@@ -41,7 +41,8 @@ constexpr char kCRDDebugLog[] = "_debug_log";
 // Connect message parameters:
 constexpr char kCRDConnectUserName[] = "userName";
 constexpr char kCRDConnectAuth[] = "authServiceWithToken";
-constexpr char kCRDConnectNoDialogs[] = "noDialogs";
+constexpr char kCRDConnectSuppressUserDialogs[] = "suppressUserDialogs";
+constexpr char kCRDConnectSuppressNotifications[] = "suppressNotifications";
 
 // CRD host states we care about:
 constexpr char kCRDStateKey[] = "state";
@@ -212,7 +213,8 @@ void It2MeCliHost::StartCRDHostAndGetCode(OAuthTokenGetter::Status status,
 
   connect_params.SetKey(kCRDConnectUserName, base::Value(user_email));
   connect_params.SetKey(kCRDConnectAuth, base::Value("oauth2:" + access_token));
-  connect_params.SetKey(kCRDConnectNoDialogs, base::Value(true));
+  connect_params.SetKey(kCRDConnectSuppressUserDialogs, base::Value(true));
+  connect_params.SetKey(kCRDConnectSuppressNotifications, base::Value(true));
   connect_params_ = std::move(connect_params);
 
   remote_connected_ = false;