Updated DeviceStatusReportRequest to contain new monitoring data.

chrome/browser/chromeos/policy/device_status_collector.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.
 
 #include "chrome/browser/chromeos/policy/device_status_collector.h"
 
 #include <limits>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/common/chrome_version_info.h"
 #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"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_type.h"
 #include "content/public/browser/browser_thread.h"
 #include "policy/proto/device_management_backend.pb.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ skipping 51 matching lines @@
       last_reported_day_(0),
       duration_for_last_reported_day_(0),
       geolocation_update_in_progress_(false),
       statistics_provider_(provider),
       report_version_info_(false),
       report_activity_times_(false),
       report_boot_mode_(false),
       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;
   idle_poll_timer_.Start(FROM_HERE,
                          TimeDelta::FromSeconds(kIdlePollIntervalSeconds),
                          this, &DeviceStatusCollector::CheckIdleState);
 
   cros_settings_ = chromeos::CrosSettings::Get();
 
   // Watch for changes to the individual policies that control what the status
   // reports contain.
   base::Closure callback =
       base::Bind(&DeviceStatusCollector::UpdateReportingSettings,
                  base::Unretained(this));
   version_info_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceVersionInfo, callback);
   activity_times_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceActivityTimes, callback);
   boot_mode_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceBootMode, callback);
   location_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceLocation, callback);
   network_interfaces_subscription_ = cros_settings_->AddSettingsObserver(
       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
   // reacquire the location on every user session change or browser crash.
   content::Geoposition position;
   std::string timestamp_str;
   int64 timestamp;
   const base::DictionaryValue* location =
       local_state_->GetDictionary(prefs::kDeviceLocation);
   if (location->GetDouble(kLatitude, &position.latitude) &&
@@ skipping 66 matching lines @@
     report_location_ = false;
   }
   if (!cros_settings_->GetBoolean(
       chromeos::kReportDeviceNetworkInterfaces, &report_network_interfaces_)) {
     report_network_interfaces_ = true;
   }
   if (!cros_settings_->GetBoolean(
       chromeos::kReportDeviceUsers, &report_users_)) {
     report_users_ = true;
   }
+  if (!cros_settings_->GetBoolean(

[pneubeck (no reviews), 2014/12/09 03:00:09]

IMO it would help a bit to drop a comment in front of these repeated 'if
(!cros_settings_->Get...)' that we report everything by default (if not
explicitly disabled).

DeviceLocation falls out of this pattern because it's not released and could be
moved to a separate block/be removed.

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

Done.

+      chromeos::kReportDeviceHardwareStatus, &report_hardware_status_)) {
+    report_hardware_status_ = true;
+  }
 
   if (report_location_) {
     ScheduleGeolocationUpdateRequest();
   } else {
     geolocation_update_timer_.Stop();
     position_ = content::Geoposition();
     local_state_->ClearPref(prefs::kDeviceLocation);
   }
 }
 
@@ skipping 169 matching lines @@
     const char* type_string;
     em::NetworkInterface::NetworkDeviceType type_constant;
   } kDeviceTypeMap[] = {
     { shill::kTypeEthernet,  em::NetworkInterface::TYPE_ETHERNET,  },
     { shill::kTypeWifi,      em::NetworkInterface::TYPE_WIFI,      },
     { shill::kTypeWimax,     em::NetworkInterface::TYPE_WIMAX,     },
     { shill::kTypeBluetooth, em::NetworkInterface::TYPE_BLUETOOTH, },
     { shill::kTypeCellular,  em::NetworkInterface::TYPE_CELLULAR,  },
   };
 
+  // Maps flimflam device connection status to proto enum constants.

[pneubeck (no reviews), 2014/12/09 03:00:10]

we shouldn't use the name flimflam of the previous incarnation of the network
manager anymore.
flimflam -> shill

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

Done.

+  static const struct {
+    const char* type_string;

[pneubeck (no reviews), 2014/12/09 03:00:09]

type_string -> state_string

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

Done.

+    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) {
     // Determine the type enum constant for |device|.
     size_t type_idx = 0;
     for (; type_idx < arraysize(kDeviceTypeMap); ++type_idx) {
       if ((*device)->type() == kDeviceTypeMap[type_idx].type_string)
         break;
     }
 
     // If the type isn't in |kDeviceTypeMap|, the interface is not relevant for
     // reporting. This filters out VPN devices.
     if (type_idx >= arraysize(kDeviceTypeMap))
       continue;
 
     em::NetworkInterface* interface = request->add_network_interface();
     interface->set_type(kDeviceTypeMap[type_idx].type_constant);
     if (!(*device)->mac_address().empty())
       interface->set_mac_address((*device)->mac_address());
     if (!(*device)->meid().empty())
       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(),
+      false,  // configured_only

[pneubeck (no reviews), 2014/12/09 03:00:09]

just verify with your intent here:
this will list _all_ networks. Which means any network that is configured or
that is visible, i.e. also visible but not configured networks.

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

Hmmm. It's probably better to be a little tighter and only report configured
networks. Changed this, good catch.

+      false,  // visible_only,
+      0,      // no limit to number of results
+      &state_list);
+
+  chromeos::NetworkStateHandler::NetworkStateList::const_iterator state;
+  for (state = state_list.begin(); state != state_list.end(); ++state) {

[ygorshenin1, 2014/12/05 15:55:41]

nit: you can just write:
for (const auto& state : state_list) { ... }

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

Done.

+    // Determine the connection state and signal strength for |state|.
+    em::NetworkState::ConnectionState connection_state_enum =
+        em::NetworkState::UNKNOWN;
+    std::string connection_state_string = (*state)->connection_state();

[pneubeck (no reviews), 2014/12/09 03:00:09]

nit: const, use constructor instead of assignment

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

Done.

+    for (size_t i = 0; i < arraysize(kConnectionStateMap); ++i) {
+      if (connection_state_string == kConnectionStateMap[i].type_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());
   }
 }
 
 void DeviceStatusCollector::GetUsers(em::DeviceStatusReportRequest* request) {
   policy::BrowserPolicyConnectorChromeOS* connector =
       g_browser_process->platform_part()->browser_policy_connector_chromeos();
   const user_manager::UserList& users =
       user_manager::UserManager::Get()->GetUsers();
   user_manager::UserList::const_iterator user;
   for (user = users.begin(); user != users.end(); ++user) {
     // Only users with gaia accounts (regular) are reported.
     if (!(*user)->HasGaiaAccount())
       continue;
 
     em::DeviceUser* device_user = request->add_user();
     const std::string& email = (*user)->email();
     if (connector->GetUserAffiliation(email) == USER_AFFILIATION_MANAGED) {
       device_user->set_type(em::DeviceUser::USER_TYPE_MANAGED);
       device_user->set_email(email);
     } else {
       device_user->set_type(em::DeviceUser::USER_TYPE_UNMANAGED);
       // Do not report the email address of unmanaged users.
     }
   }
 }
 
-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) {

[pneubeck (no reviews), 2014/12/09 03:00:10]

why is this empty function necessary?
if it will be filled eventually better just punt this part of the CL.

[Andrew T Wilson (Slow), 2014/12/11 00:09:54]

I'd like to keep this in place because I'd like to keep the surrounding logic
(checking the report_hardware_status_ setting, etc in this CL). I've added a
TODO here.

+
 }
 
 bool DeviceStatusCollector::GetDeviceStatus(
     em::DeviceStatusReportRequest* status) {
   if (report_activity_times_)
     GetActivityTimes(status);
 
   if (report_version_info_)
     GetVersionInfo(status);
 
   if (report_boot_mode_)
     GetBootMode(status);
 
   if (report_location_)
     GetLocation(status);
 
   if (report_network_interfaces_)
     GetNetworkInterfaces(status);
 
   if (report_users_) {
     GetUsers(status);
   }
 
+  if (report_hardware_status_)
+    GetHardwareStatus(status);
+
   return true;
 }
 
 bool DeviceStatusCollector::GetSessionStatus(
     em::SessionStatusReportRequest* status) {
   return false;
 }
 
 void DeviceStatusCollector::OnSubmittedSuccessfully() {
   TrimStoredActivityPeriods(last_reported_day_, duration_for_last_reported_day_,
@@ skipping 60 matching lines @@
     location.SetDouble(kSpeed, position.speed);
     location.SetString(kTimestamp,
         base::Int64ToString(position.timestamp.ToInternalValue()));
     local_state_->Set(prefs::kDeviceLocation, location);
   }
 
   ScheduleGeolocationUpdateRequest();
 }
 
 }  // namespace policy