mash: Add SystemTrayClient interface, use to show date settings

ash/mus/system_tray_delegate_mus.cc

 // Copyright 2016 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 "ash/mus/system_tray_delegate_mus.h"
 
 #include "ash/common/system/tray/system_tray_notifier.h"
 #include "ash/common/wm_shell.h"
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
+#include "services/shell/public/cpp/connector.h"
 
 namespace ash {
 namespace {
 
 SystemTrayDelegateMus* g_instance = nullptr;
 
 }  // namespace
 
-SystemTrayDelegateMus::SystemTrayDelegateMus()
-    : hour_clock_type_(base::GetHourClockType()) {
+SystemTrayDelegateMus::SystemTrayDelegateMus(shell::Connector* connector)
+    : connector_(connector),
+      hour_clock_type_(base::GetHourClockType()),
+      weak_factory_(this) {
+  DCHECK(connector_);
+  // Don't make an initial connection to exe:chrome. Do it on demand.
+
   DCHECK(!g_instance);
   g_instance = this;
 }
 
 SystemTrayDelegateMus::~SystemTrayDelegateMus() {
   DCHECK_EQ(this, g_instance);
   g_instance = nullptr;
 }
 
 // static
 SystemTrayDelegateMus* SystemTrayDelegateMus::Get() {
   return g_instance;
 }
 
+void SystemTrayDelegateMus::ConnectToSystemTrayClient() {
+  // Check if already connected.

[sky, 2016/09/26 15:23:14]

optional: IMO this comment and 43 document exactly what the code does and don't
provide much value.

[James Cook, 2016/09/26 18:01:37]

Removed this one and the one on line 46.

+  if (system_tray_client_.is_bound())
+    return;
+
+  // Connect to chrome browser.
+  connector_->ConnectToInterface("exe:chrome", &system_tray_client_);

[sky, 2016/09/26 15:23:14]

What you have here certainly works. Consider though that one nice thing about
the explicit SetClient call you had before is that it means ash doesn't care who
is providing the client interface and doesn't need to establish an out bound
connection like this.

[James Cook, 2016/09/26 18:01:37]

As discussed offline, keeping this so commands on the system_tray_client_
interface get queued if the user takes an action before chrome is up.

+
+  // Tolerate chrome crashing and coming back up.
+  system_tray_client_.set_connection_error_handler(
+      base::Bind(&SystemTrayDelegateMus::OnClientConnectionError,
+                 weak_factory_.GetWeakPtr()));

[sky, 2016/09/26 15:23:14]

As this owns system_tray_client_ you shouldn't need a weakptr here.

[James Cook, 2016/09/26 18:01:37]

Done.

+}
+
+void SystemTrayDelegateMus::OnClientConnectionError() {
+  system_tray_client_.reset();
+}
+
 base::HourClockType SystemTrayDelegateMus::GetHourClockType() const {
   return hour_clock_type_;
 }
 
+void SystemTrayDelegateMus::ShowDateSettings() {
+  ConnectToSystemTrayClient();
+  system_tray_client_->ShowDateSettings();
+}
+
 void SystemTrayDelegateMus::SetUse24HourClock(bool use_24_hour) {
   hour_clock_type_ = use_24_hour ? base::k24HourClock : base::k12HourClock;
   WmShell::Get()->system_tray_notifier()->NotifyDateFormatChanged();
 }
 
 }  // namespace ash