Updated DeviceStatusReportRequest to contain new monitoring data.

chrome/browser/chromeos/policy/device_status_collector_browsertest.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 "base/environment.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/testing_pref_service.h"
 #include "base/run_loop.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
 #include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/policy/stub_enterprise_install_attributes.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
 #include "chrome/common/pref_names.h"
 #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"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/geolocation_provider.h"
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "policy/proto/device_management_backend.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
@@ skipping 639 matching lines @@
     "aabbccddeeff", "", "",
     em::NetworkInterface::TYPE_WIFI },
   { "/device/bluetooth", shill::kTypeBluetooth, "bluetooth",
     "", "", "",
     em::NetworkInterface::TYPE_BLUETOOTH },
   { "/device/vpn", shill::kTypeVPN, "vpn",
     "", "", "",
     -1 },
 };
 
+// Fake network state.
+struct FakeNetworkState {
+  const char* name;
+  const char* device_path;
+  const char* type;
+  const int signal_strength;

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

nit: const not required

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

Done.

+  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

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

nit: "0" -> 0
for consistency.

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

Done.

+// 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 },
+};
+
 class DeviceStatusCollectorNetworkInterfacesTest
     : public DeviceStatusCollectorTest {
  protected:
   virtual void SetUp() override {
     chromeos::DBusThreadManager::Initialize();
     chromeos::NetworkHandler::Initialize();
     chromeos::ShillDeviceClient::TestInterface* test_device_client =
         chromeos::DBusThreadManager::Get()->GetShillDeviceClient()->
             GetTestInterface();
     test_device_client->ClearDevices();
@@ skipping 11 matching lines @@
             dev.device_path, shill::kMeidProperty,
             base::StringValue(dev.meid));
       }
       if (*dev.imei) {
         test_device_client->SetDeviceProperty(
             dev.device_path, shill::kImeiProperty,
             base::StringValue(dev.imei));
       }
     }
 
+    chromeos::ShillServiceClient::TestInterface* service_client =
+        chromeos::DBusThreadManager::Get()->GetShillServiceClient()->
+            GetTestInterface();
+    service_client->ClearServices();
+
+    // Now add services for every fake network.
+    for (size_t i = 0; i < arraysize(kFakeNetworks); ++i) {

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

should be possible to replace by

for (const FakeNetworkState& fake_network : kFakeNetworks) {
 ...

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

Done.

+      service_client->AddService(
+          kFakeNetworks[i].name,       /* service_path */
+          kFakeNetworks[i].name        /* guid */,
+          kFakeNetworks[i].name        /* name */,
+          kFakeNetworks[i].type        /* type */,
+          kFakeNetworks[i].connection_status,
+          true /* visible */);

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

would you want to test a not visible network as well, since you include them in
the report?

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

Added a test for this (we have a disconnected network and I've made it
non-visible).

+      service_client->SetServiceProperty(
+          kFakeNetworks[i].name, shill::kSignalStrengthProperty,
+          base::FundamentalValue(kFakeNetworks[i].signal_strength));
+      service_client->SetServiceProperty(
+          kFakeNetworks[i].name, shill::kDeviceProperty,
+          base::StringValue(kFakeNetworks[i].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(),
+        false,  // 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 {
     chromeos::NetworkHandler::Shutdown();
     chromeos::DBusThreadManager::Shutdown();
   }
 };
 
 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);
   GetStatus();
 
   int count = 0;
   for (size_t i = 0; i < arraysize(kFakeDevices); ++i) {
     const FakeDeviceData& dev = kFakeDevices[i];
     if (dev.expected_type == -1)
       continue;
 
     // Find the corresponding entry in reporting data.
     bool found_match = false;
     google::protobuf::RepeatedPtrField<em::NetworkInterface>::const_iterator
         iface;
     for (iface = status_.network_interface().begin();
          iface != status_.network_interface().end();
          ++iface) {
       // Check whether type, field presence and field values match.
       if (dev.expected_type == iface->type() &&
           iface->has_mac_address() == !!*dev.mac_address &&
           iface->has_meid() == !!*dev.meid &&
           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;
       }
     }
 
     EXPECT_TRUE(found_match) << "No matching interface for fake device " << i;
     count++;
   }
 
   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 (size_t i = 0; i < arraysize(kFakeNetworks); ++i) {

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

same here

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

Done.

+    const FakeNetworkState& state = kFakeNetworks[i];
+    bool found_match = false;
+    google::protobuf::RepeatedPtrField<em::NetworkState>::const_iterator
+        proto_state;
+    for (proto_state = status_.network_state().begin();

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

for (const em::NetworkState& proto_state : status_.network_state()) {
 ...

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

Done.

+         proto_state != status_.network_state().end();
+         ++proto_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 " << i
+                             << " (" << state.name << ")";
+  }
 }
 
 }  // namespace policy