Updated DeviceStatusReportRequest to contain new monitoring data.

Added code to DeviceStatusCollector to populate new NetworkState fields.

BUG=430908

Review URL: https://codereview.chromium.org/776803003

Cr-Commit-Position: refs/heads/master@{#308393}

chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc

@@ -614,6 +614,7 @@ void OwnerSettingsServiceChromeOS::UpdateDeviceSettings(
     //   kReportDeviceVersionInfo
     //   kReportDeviceNetworkInterfaces
     //   kReportDeviceUsers
+    //   kReportDeviceHardwareStatus
     //   kScreenSaverExtensionId
     //   kScreenSaverTimeout
     //   kServiceAccountIdentity

chrome/browser/chromeos/policy/device_status_collector.cc

@@ -23,6 +23,7 @@
 #include "chrome/common/pref_names.h"
 #include "chromeos/network/device_state.h"
 #include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/system/statistics_provider.h"
@@ -93,6 +94,7 @@ DeviceStatusCollector::DeviceStatusCollector(
       report_location_(false),
       report_network_interfaces_(false),
       report_users_(false),
+      report_hardware_status_(false),
       weak_factory_(this) {
   if (location_update_requester)
     location_update_requester_ = *location_update_requester;
@@ -119,6 +121,8 @@ DeviceStatusCollector::DeviceStatusCollector(
       chromeos::kReportDeviceNetworkInterfaces, callback);
   users_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceUsers, callback);
+  hardware_status_subscription_ = cros_settings_->AddSettingsObserver(
+      chromeos::kReportDeviceHardwareStatus, callback);
 
   // The last known location is persisted in local state. This makes location
   // information available immediately upon startup and avoids the need to
@@ -187,6 +191,8 @@ void DeviceStatusCollector::UpdateReportingSettings() {
                  weak_factory_.GetWeakPtr()))) {
     return;
   }
+
+  // All reporting settings default to 'enabled'.
   if (!cros_settings_->GetBoolean(
       chromeos::kReportDeviceVersionInfo, &report_version_info_)) {
     report_version_info_ = true;
@@ -199,10 +205,6 @@ void DeviceStatusCollector::UpdateReportingSettings() {
       chromeos::kReportDeviceBootMode, &report_boot_mode_)) {
     report_boot_mode_ = true;
   }
-  if (!cros_settings_->GetBoolean(
-      chromeos::kReportDeviceLocation, &report_location_)) {
-    report_location_ = false;
-  }
   if (!cros_settings_->GetBoolean(
       chromeos::kReportDeviceNetworkInterfaces, &report_network_interfaces_)) {
     report_network_interfaces_ = true;
@@ -211,6 +213,17 @@ void DeviceStatusCollector::UpdateReportingSettings() {
       chromeos::kReportDeviceUsers, &report_users_)) {
     report_users_ = true;
   }
+  if (!cros_settings_->GetBoolean(
+      chromeos::kReportDeviceHardwareStatus, &report_hardware_status_)) {
+    report_hardware_status_ = true;
+  }
+
+  // Device location reporting is disabled by default because it is
+  // not launched yet.
+  if (!cros_settings_->GetBoolean(
+      chromeos::kReportDeviceLocation, &report_location_)) {
+    report_location_ = false;
+  }
 
   if (report_location_) {
     ScheduleGeolocationUpdateRequest();
@@ -388,7 +401,7 @@ void DeviceStatusCollector::GetLocation(
 
 void DeviceStatusCollector::GetNetworkInterfaces(
     em::DeviceStatusReportRequest* request) {
-  // Maps flimflam device type strings to proto enum constants.
+  // Maps shill device type strings to proto enum constants.
   static const struct {
     const char* type_string;
     em::NetworkInterface::NetworkDeviceType type_constant;
@@ -400,9 +413,29 @@ void DeviceStatusCollector::GetNetworkInterfaces(
     { shill::kTypeCellular,  em::NetworkInterface::TYPE_CELLULAR,  },
   };
 
+  // Maps shill device connection status to proto enum constants.
+  static const struct {
+    const char* state_string;
+    em::NetworkState::ConnectionState state_constant;
+  } kConnectionStateMap[] = {
+    { shill::kStateIdle,              em::NetworkState::IDLE },
+    { shill::kStateCarrier,           em::NetworkState::CARRIER },
+    { shill::kStateAssociation,       em::NetworkState::ASSOCIATION },
+    { shill::kStateConfiguration,     em::NetworkState::CONFIGURATION },
+    { shill::kStateReady,             em::NetworkState::READY },
+    { shill::kStatePortal,            em::NetworkState::PORTAL },
+    { shill::kStateOffline,           em::NetworkState::OFFLINE },
+    { shill::kStateOnline,            em::NetworkState::ONLINE },
+    { shill::kStateDisconnect,        em::NetworkState::DISCONNECT },
+    { shill::kStateFailure,           em::NetworkState::FAILURE },
+    { shill::kStateActivationFailure,
+        em::NetworkState::ACTIVATION_FAILURE },
+  };
+
   chromeos::NetworkStateHandler::DeviceStateList device_list;
-  chromeos::NetworkHandler::Get()->network_state_handler()->GetDeviceList(
-      &device_list);
+  chromeos::NetworkStateHandler* network_state_handler =
+      chromeos::NetworkHandler::Get()->network_state_handler();
+  network_state_handler->GetDeviceList(&device_list);
 
   chromeos::NetworkStateHandler::DeviceStateList::const_iterator device;
   for (device = device_list.begin(); device != device_list.end(); ++device) {
@@ -426,6 +459,37 @@ void DeviceStatusCollector::GetNetworkInterfaces(
       interface->set_meid((*device)->meid());
     if (!(*device)->imei().empty())
       interface->set_imei((*device)->imei());
+    if (!(*device)->path().empty())
+      interface->set_device_path((*device)->path());
+  }
+
+  // Walk the various networks and store their state in the status report.
+  chromeos::NetworkStateHandler::NetworkStateList state_list;
+  network_state_handler->GetNetworkListByType(
+      chromeos::NetworkTypePattern::Default(),
+      true,  // configured_only
+      false,  // visible_only,
+      0,      // no limit to number of results
+      &state_list);
+
+  for (const chromeos::NetworkState* state: state_list) {
+    // Determine the connection state and signal strength for |state|.
+    em::NetworkState::ConnectionState connection_state_enum =
+        em::NetworkState::UNKNOWN;
+    const std::string connection_state_string(state->connection_state());
+    for (size_t i = 0; i < arraysize(kConnectionStateMap); ++i) {
+      if (connection_state_string == kConnectionStateMap[i].state_string) {
+        connection_state_enum = kConnectionStateMap[i].state_constant;
+        break;
+      }
+    }
+
+    // Copy fields from NetworkState into the status report.
+    em::NetworkState* proto_state = request->add_network_state();
+    proto_state->set_connection_state(connection_state_enum);
+    proto_state->set_signal_strength(state->signal_strength());
+    if (!state->device_path().empty())
+      proto_state->set_device_path(state->device_path());
   }
 }
 
@@ -452,12 +516,9 @@ void DeviceStatusCollector::GetUsers(em::DeviceStatusReportRequest* request) {
   }
 }
 
-void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
-  // TODO(mnissler): Remove once the old cloud policy stack is retired. The old
-  // stack doesn't support reporting successful submissions back to here, so
-  // just assume whatever ends up in |request| gets submitted successfully.
-  GetDeviceStatus(request);
-  OnSubmittedSuccessfully();
+void DeviceStatusCollector::GetHardwareStatus(
+    em::DeviceStatusReportRequest* status) {
+  // TODO(atwilson): Fill in hardware status fields.
 }
 
 bool DeviceStatusCollector::GetDeviceStatus(
@@ -481,6 +542,9 @@ bool DeviceStatusCollector::GetDeviceStatus(
     GetUsers(status);
   }
 
+  if (report_hardware_status_)
+    GetHardwareStatus(status);
+
   return true;
 }
 

chrome/browser/chromeos/policy/device_status_collector.h

@@ -59,8 +59,6 @@ class DeviceStatusCollector : public CloudPolicyClient::StatusProvider {
       LocationUpdateRequester* location_update_requester);
   virtual ~DeviceStatusCollector();
 
-  void GetStatus(enterprise_management::DeviceStatusReportRequest* request);
-
   // CloudPolicyClient::StatusProvider:
   virtual bool GetDeviceStatus(
       enterprise_management::DeviceStatusReportRequest* status) override;
@@ -123,6 +121,8 @@ class DeviceStatusCollector : public CloudPolicyClient::StatusProvider {
       enterprise_management::DeviceStatusReportRequest* request);
   void GetUsers(
       enterprise_management::DeviceStatusReportRequest* request);
+  void GetHardwareStatus(
+      enterprise_management::DeviceStatusReportRequest* request);
 
   // Update the cached values of the reporting settings.
   void UpdateReportingSettings();
@@ -176,6 +176,7 @@ class DeviceStatusCollector : public CloudPolicyClient::StatusProvider {
   bool report_location_;
   bool report_network_interfaces_;
   bool report_users_;
+  bool report_hardware_status_;
 
   scoped_ptr<chromeos::CrosSettings::ObserverSubscription>
       version_info_subscription_;
@@ -189,6 +190,8 @@ class DeviceStatusCollector : public CloudPolicyClient::StatusProvider {
       network_interfaces_subscription_;
   scoped_ptr<chromeos::CrosSettings::ObserverSubscription>
       users_subscription_;
+  scoped_ptr<chromeos::CrosSettings::ObserverSubscription>
+      hardware_status_subscription_;
 
   base::WeakPtrFactory<DeviceStatusCollector> weak_factory_;
 

chrome/browser/chromeos/policy/device_status_collector_browsertest.cc

@@ -23,7 +23,10 @@
 #include "chrome/test/base/testing_browser_process.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
+#include "chromeos/dbus/shill_service_client.h"
 #include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/settings/cros_settings_provider.h"
 #include "chromeos/system/fake_statistics_provider.h"
@@ -683,6 +686,51 @@ static const FakeDeviceData kFakeDevices[] = {
     -1 },
 };
 
+// Fake network state.
+struct FakeNetworkState {
+  const char* name;
+  const char* device_path;
+  const char* type;
+  int signal_strength;
+  const char* connection_status;
+  int expected_state;
+};
+
+// List of fake networks - primarily used to make sure that signal strength
+// and connection state are properly populated in status reports. Note that
+// by convention shill will not report a signal strength of 0 for a visible
+// network, so we use 1 below.
+static const FakeNetworkState kFakeNetworks[] = {
+  { "offline", "/device/wifi", shill::kTypeWifi, 35,
+    shill::kStateOffline, em::NetworkState::OFFLINE },
+  { "ethernet", "/device/ethernet", shill::kTypeEthernet, 0,
+    shill::kStateOnline, em::NetworkState::ONLINE },
+  { "wifi", "/device/wifi", shill::kTypeWifi, 23, shill::kStatePortal,
+    em::NetworkState::PORTAL },
+  { "idle", "/device/cellular1", shill::kTypeCellular, 0, shill::kStateIdle,
+    em::NetworkState::IDLE },
+  { "carrier", "/device/cellular1", shill::kTypeCellular, 0,
+    shill::kStateCarrier, em::NetworkState::CARRIER },
+  { "association", "/device/cellular1", shill::kTypeCellular, 0,
+    shill::kStateAssociation, em::NetworkState::ASSOCIATION },
+  { "config", "/device/cellular1", shill::kTypeCellular, 0,
+    shill::kStateConfiguration, em::NetworkState::CONFIGURATION },
+  { "ready", "/device/cellular1", shill::kTypeCellular, 0, shill::kStateReady,
+    em::NetworkState::READY },
+  { "disconnect", "/device/wifi", shill::kTypeWifi, 1,
+    shill::kStateDisconnect, em::NetworkState::DISCONNECT },
+  { "failure", "/device/wifi", shill::kTypeWifi, 1, shill::kStateFailure,
+    em::NetworkState::FAILURE },
+  { "activation-failure", "/device/cellular1", shill::kTypeCellular, 0,
+    shill::kStateActivationFailure, em::NetworkState::ACTIVATION_FAILURE },
+  { "unknown", "", shill::kTypeWifi, 1, "unknown", em::NetworkState::UNKNOWN },
+};
+
+static const FakeNetworkState kUnconfiguredNetwork = {
+  "unconfigured", "/device/unconfigured", shill::kTypeWifi, 35,
+  shill::kStateOffline, em::NetworkState::OFFLINE
+};
+
 class DeviceStatusCollectorNetworkInterfacesTest
     : public DeviceStatusCollectorTest {
  protected:
@@ -714,8 +762,64 @@ class DeviceStatusCollectorNetworkInterfacesTest
       }
     }
 
+    chromeos::ShillServiceClient::TestInterface* service_client =
+        chromeos::DBusThreadManager::Get()->GetShillServiceClient()->
+            GetTestInterface();
+    service_client->ClearServices();
+
+    // Now add services for every fake network.
+    for (const FakeNetworkState& fake_network : kFakeNetworks) {
+      // Shill forces non-visible networks to report a disconnected state.
+      bool is_visible =
+          fake_network.connection_status != shill::kStateDisconnect;
+      service_client->AddService(
+          fake_network.name,       /* service_path */
+          fake_network.name        /* guid */,
+          fake_network.name        /* name */,
+          fake_network.type        /* type */,
+          fake_network.connection_status,
+          is_visible);
+      service_client->SetServiceProperty(
+          fake_network.name, shill::kSignalStrengthProperty,
+          base::FundamentalValue(fake_network.signal_strength));
+      service_client->SetServiceProperty(
+          fake_network.name, shill::kDeviceProperty,
+          base::StringValue(fake_network.device_path));
+      // Set the profile so this shows up as a configured network.
+      service_client->SetServiceProperty(
+          fake_network.name, shill::kProfileProperty,
+          base::StringValue(fake_network.name));
+    }
+
+    // Now add an unconfigured network - it should not show up in the
+    // reported list of networks because it doesn't have a profile specified.
+    service_client->AddService(
+        kUnconfiguredNetwork.name,       /* service_path */
+        kUnconfiguredNetwork.name        /* guid */,
+        kUnconfiguredNetwork.name        /* name */,
+        kUnconfiguredNetwork.type        /* type */,
+        kUnconfiguredNetwork.connection_status,
+        true /* visible */);
+    service_client->SetServiceProperty(
+        kUnconfiguredNetwork.name, shill::kSignalStrengthProperty,
+        base::FundamentalValue(kUnconfiguredNetwork.signal_strength));
+    service_client->SetServiceProperty(
+        kUnconfiguredNetwork.name, shill::kDeviceProperty,
+        base::StringValue(kUnconfiguredNetwork.device_path));
+
     // Flush out pending state updates.
     base::RunLoop().RunUntilIdle();
+
+    chromeos::NetworkStateHandler::NetworkStateList state_list;
+    chromeos::NetworkStateHandler* network_state_handler =
+        chromeos::NetworkHandler::Get()->network_state_handler();
+    network_state_handler->GetNetworkListByType(
+        chromeos::NetworkTypePattern::Default(),
+        true,  // configured_only
+        false,  // visible_only,
+        0,      // no limit to number of results
+        &state_list);
+    ASSERT_EQ(arraysize(kFakeNetworks), state_list.size());
   }
 
   virtual void TearDown() override {
@@ -728,11 +832,13 @@ TEST_F(DeviceStatusCollectorNetworkInterfacesTest, NetworkInterfaces) {
   // Interfaces should be reported by default.
   GetStatus();
   EXPECT_LT(0, status_.network_interface_size());
+  EXPECT_LT(0, status_.network_state_size());
 
   // No interfaces should be reported if the policy is off.
   cros_settings_->SetBoolean(chromeos::kReportDeviceNetworkInterfaces, false);
   GetStatus();
   EXPECT_EQ(0, status_.network_interface_size());
+  EXPECT_EQ(0, status_.network_state_size());
 
   // Switch the policy on and verify the interface list is present.
   cros_settings_->SetBoolean(chromeos::kReportDeviceNetworkInterfaces, true);
@@ -758,7 +864,8 @@ TEST_F(DeviceStatusCollectorNetworkInterfacesTest, NetworkInterfaces) {
           iface->has_imei() == !!*dev.imei &&
           iface->mac_address() == dev.mac_address &&
           iface->meid() == dev.meid &&
-          iface->imei() == dev.imei) {
+          iface->imei() == dev.imei &&
+          iface->device_path() == dev.device_path) {
         found_match = true;
         break;
       }
@@ -769,6 +876,24 @@ TEST_F(DeviceStatusCollectorNetworkInterfacesTest, NetworkInterfaces) {
   }
 
   EXPECT_EQ(count, status_.network_interface_size());
+
+  // Now make sure network state list is correct.
+  EXPECT_EQ(arraysize(kFakeNetworks),
+            static_cast<size_t>(status_.network_state_size()));
+  for (const FakeNetworkState& state : kFakeNetworks) {
+    bool found_match = false;
+    for (const em::NetworkState& proto_state : status_.network_state()) {
+      // Make sure every item has a matching entry in the proto.
+      if (proto_state.has_device_path() == (strlen(state.device_path) > 0) &&
+          proto_state.signal_strength() == state.signal_strength &&
+          proto_state.connection_state() == state.expected_state) {
+        found_match = true;
+        break;
+      }
+    }
+    EXPECT_TRUE(found_match) << "No matching state for fake network "
+                             << " (" << state.name << ")";
+  }
 }
 
 }  // namespace policy

chrome/browser/chromeos/policy/proto/chrome_device_policy.proto

@@ -97,6 +97,7 @@ message DeviceReportingProto {
   optional bool report_location = 4;
   optional bool report_network_interfaces = 5;
   optional bool report_users = 6;
+  optional bool report_hardware_status = 7;
 }
 
 message EphemeralUsersEnabledProto {

chrome/browser/chromeos/settings/device_settings_provider.cc

@@ -67,6 +67,7 @@ const char* const kKnownSettings[] = {
   kReportDeviceLocation,
   kReportDeviceNetworkInterfaces,
   kReportDeviceUsers,
+  kReportDeviceHardwareStatus,
   kReportDeviceVersionInfo,
   kScreenSaverExtensionId,
   kScreenSaverTimeout,
@@ -367,6 +368,11 @@ void DecodeReportingPolicies(
           kReportDeviceUsers,
           reporting_policy.report_users());
     }
+    if (reporting_policy.has_report_hardware_status()) {
+      new_values_cache->SetBoolean(
+          kReportDeviceHardwareStatus,
+          reporting_policy.report_hardware_status());
+    }
   }
 }
 

chromeos/settings/cros_settings_names.cc

@@ -95,6 +95,11 @@ const char kReportDeviceNetworkInterfaces[] =
 // status reports to the device management server.
 const char kReportDeviceUsers[] = "cros.device_status.report_users";
 
+// Determines whether the device reports hardware status (CPU utilization,
+// disk space, etc) in device status reports to the device management server.
+const char kReportDeviceHardwareStatus[] =
+    "cros.device_status.report_hardware_status";
+
 // A list of dictionaries, each detailing one extension to install as part of
 // the AppPack and including the following fields:
 // "extension-id": ID of the extension to install

chromeos/settings/cros_settings_names.h

@@ -56,6 +56,7 @@ CHROMEOS_EXPORT extern const char kReportDeviceBootMode[];
 CHROMEOS_EXPORT extern const char kReportDeviceLocation[];
 CHROMEOS_EXPORT extern const char kReportDeviceNetworkInterfaces[];
 CHROMEOS_EXPORT extern const char kReportDeviceUsers[];
+CHROMEOS_EXPORT extern const char kReportDeviceHardwareStatus[];
 
 CHROMEOS_EXPORT extern const char kAppPack[];
 CHROMEOS_EXPORT extern const char kAppPackKeyExtensionId[];

components/policy/proto/device_management_backend.proto

@@ -480,6 +480,40 @@ message NetworkInterface {
   // IMEI (if applicable) of the corresponding network device. 15-16 decimal
   // digits encoded as ASCII string. Example: 355402040158759.
   optional string imei = 4;
+
+  // The device path associated with this network interface.
+  optional string device_path = 5;
+}
+
+
+// Information about configured/visible networks - this is separate from
+// NetworkInterface because a configured network may not be associated with
+// any specific interface, or may be visible across multiple interfaces.
+message NetworkState {
+  // The current state of this network.
+  enum ConnectionState {
+    IDLE = 0;
+    CARRIER = 1;
+    ASSOCIATION = 2;
+    CONFIGURATION = 3;
+    READY = 4;
+    PORTAL = 5;
+    OFFLINE = 6;
+    ONLINE = 7;
+    DISCONNECT = 8;
+    FAILURE = 9;
+    ACTIVATION_FAILURE = 10;
+    UNKNOWN = 11;
+  }
+
+  // For networks associated with a device, the path of the device.
+  optional string device_path = 1;
+
+  // Current state of this connection as reported by shill.
+  optional ConnectionState connection_state = 2;
+
+  // For wireless networks, the signal_strength in dBm.
+  optional int32 signal_strength = 3;
 }
 
 // Details about a device user.
@@ -500,6 +534,13 @@ message DeviceUser {
   optional string email = 2;
 }
 
+// Information about a single disk volume.
+message VolumeInfo {
+  optional string volume_id = 1;
+  optional int64 storage_used = 2;
+  optional int64 storage_free = 3;
+}
+
 // Report device level status.
 message DeviceStatusReportRequest {
   // The OS version reported by the device is a platform version
@@ -530,18 +571,57 @@ message DeviceStatusReportRequest {
 
   // List of recent device users, in descending order by last login time.
   repeated DeviceUser user = 9;
+
+  // Disk space + other info about mounted/connected volumes.
+  repeated VolumeInfo volume_info = 10;
+
+  // List of visible/configured networks
+  repeated NetworkState network_state = 11;
+
+  // Samples of CPU utilization (0-100), sampled once every 60 seconds.
+  repeated int32 cpu_utilization_pct = 12;
+
+  // RAM free (as reported on a per-process level, unreliable due to GC).
+  optional int64 system_ram_free = 13;
+  optional int64 system_ram_used = 14;
+}
+
+// Provides status information for an installed app/extension.
+message AppStatus {
+  // ID of the installed app/extension
+  required string app_id = 1;
+
+  // Currently installed version of the app.
+  required string extension_version = 2;
+
+  // Self-reported status summary (via chrome.reporting APIs)
+  optional string status = 3;
+
+  // If true, the application is currently in a self-reported error state.
+  optional bool error = 4;
 }
 
 // Report session (a user on one device) level status.
 message SessionStatusReportRequest {
   // Installed apps for this user on this device.
-  repeated string installed_app_id = 1;
+  // No longer used -- use installed_apps instead.
+  repeated string installed_app_id = 1 [deprecated = true];
 
   // Installed extensions for this user on this device.
-  repeated string installed_extension_id = 2;
+  // No longer used -- use installed_extensions instead.
+  repeated string installed_extension_id = 2 [deprecated = true];
 
   // One stat per app for top 30 apps.
   repeated InstallableLaunch app_launch_stat = 3;
+
+  // If this is a kiosk session, this is the device local account ID.
+  optional string device_local_account_id = 4;
+
+  // Information about installed apps for this user on this device.
+  repeated AppStatus installed_apps = 5;
+
+  // Information about installed extensions for this user on this device.
+  repeated AppStatus installed_extensions = 6;
 }
 
 // Response from DMServer to update devices' status.