Add tests for chromeos metrics provider full hardware class.

Bug: 856378
Change-Id: I81936e58a6e259e0dce7149d94b9d86288ef785b
Reviewed-on: https://chromium-review.googlesource.com/1116356
Commit-Queue: Jong Ahn <jongahn@google.com>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570924}

chrome/browser/metrics/chromeos_metrics_provider.cc

@@ -85,9 +85,6 @@ void IncrementPrefValue(const char* path) {
   pref->SetInteger(path, value + 1);
 }
 
-const base::Feature kUmaShortHWClass{"UmaShortHWClass",
-                                     base::FEATURE_ENABLED_BY_DEFAULT};
-
 // Called on a background thread to load hardware class information.
 std::string GetFullHardwareClassOnBackgroundThread() {
   std::string full_hardware_class;
@@ -98,6 +95,16 @@ std::string GetFullHardwareClassOnBackgroundThread() {
 
 }  // namespace
 
+namespace features {
+
+// Populates hardware class field in system_profile proto with the
+// short hardware class if enabled. If disabled, hardware class will have same
+// value as full hardware class.
+const base::Feature kUmaShortHWClass{"UmaShortHWClass",
+                                     base::FEATURE_ENABLED_BY_DEFAULT};
+
+}  // namespace features
+
 ChromeOSMetricsProvider::ChromeOSMetricsProvider()
     : registered_user_count_at_log_initialization_(false),
       user_count_at_log_initialization_(0),
@@ -140,7 +147,7 @@ ChromeOSMetricsProvider::GetEnrollmentStatus() {
 }
 
 void ChromeOSMetricsProvider::Init() {
-  if (base::FeatureList::IsEnabled(kUmaShortHWClass)) {
+  if (base::FeatureList::IsEnabled(features::kUmaShortHWClass)) {
     hardware_class_ =
         variations::VariationsFieldTrialCreator::GetShortHardwareClass();
   }
@@ -330,7 +337,7 @@ void ChromeOSMetricsProvider::SetBluetoothAdapter(
 void ChromeOSMetricsProvider::SetFullHardwareClass(
     base::Closure callback,
     std::string full_hardware_class) {
-  if (!base::FeatureList::IsEnabled(kUmaShortHWClass)) {
+  if (!base::FeatureList::IsEnabled(features::kUmaShortHWClass)) {
     DCHECK(hardware_class_.empty());
     hardware_class_ = full_hardware_class;
   }

chrome/browser/metrics/chromeos_metrics_provider.h

@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/metrics/perf/perf_provider_chromeos.h"
@@ -16,6 +17,10 @@ namespace device {
 class BluetoothAdapter;
 }
 
+namespace features {
+extern const base::Feature kUmaShortHWClass;
+}
+
 namespace metrics {
 class ChromeUserMetricsExtension;
 }

chrome/browser/metrics/chromeos_metrics_provider_unittest.cc

@@ -9,11 +9,13 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
-#include "chrome/browser/metrics/chromeos_metrics_provider.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager_client.h"
 #include "chromeos/login/login_state.h"
+#include "chromeos/system/fake_statistics_provider.h"
+#include "chromeos/system/statistics_provider.h"
 #include "components/user_manager/scoped_user_manager.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -45,6 +47,7 @@ using bluez::FakeBluetoothGattCharacteristicClient;
 using bluez::FakeBluetoothGattDescriptorClient;
 using bluez::FakeBluetoothGattServiceClient;
 using bluez::FakeBluetoothInputClient;
+
 using chromeos::DBusThreadManager;
 using chromeos::DBusThreadManagerSetter;
 using chromeos::PowerManagerClient;
@@ -92,6 +95,10 @@ class ChromeOSMetricsProviderTest : public testing::Test {
     fake_bluetooth_device_client_ = static_cast<FakeBluetoothDeviceClient*>(
         BluezDBusManager::Get()->GetBluetoothDeviceClient());
 
+    // Set statistic provider for hardware class tests.
+    chromeos::system::StatisticsProvider::SetTestProvider(
+        &fake_statistics_provider_);
+
     // Initialize the login state trackers.
     if (!chromeos::LoginState::IsInitialized())
       chromeos::LoginState::Initialize();
@@ -107,6 +114,8 @@ class ChromeOSMetricsProviderTest : public testing::Test {
  protected:
   FakeBluetoothAdapterClient* fake_bluetooth_adapter_client_;
   FakeBluetoothDeviceClient* fake_bluetooth_device_client_;
+  base::test::ScopedFeatureList scoped_feature_list_;
+  chromeos::system::ScopedFakeStatisticsProvider fake_statistics_provider_;
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
@@ -115,16 +124,16 @@ class ChromeOSMetricsProviderTest : public testing::Test {
 };
 
 // Wrapper around ChromeOSMetricsProvider that initializes
-// BluetoothAdapter in the constructor.
+// Bluetooth and hardware class in the constructor.
 class TestChromeOSMetricsProvider : public ChromeOSMetricsProvider {
  public:
   TestChromeOSMetricsProvider() {
-    InitTaskGetBluetoothAdapter(
-        base::Bind(&TestChromeOSMetricsProvider::GetBluetoothAdapterCallback,
-                   base::Unretained(this)));
+    AsyncInit(base::Bind(&TestChromeOSMetricsProvider::GetIdleCallback,
+                         base::Unretained(this)));
     base::RunLoop().Run();
   }
-  void GetBluetoothAdapterCallback() {
+
+  void GetIdleCallback() {
     ASSERT_TRUE(base::RunLoop::IsRunningOnCurrentThread());
     base::RunLoop::QuitCurrentWhenIdleDeprecated();
   }
@@ -282,3 +291,43 @@ TEST_F(ChromeOSMetricsProviderTest, BluetoothPairedDevices) {
   EXPECT_EQ(0x207D74U, device2.vendor_prefix());
   EXPECT_EQ(PairedDevice::VENDOR_ID_UNKNOWN, device2.vendor_id_source());
 }
+
+TEST_F(ChromeOSMetricsProviderTest, DisableUmaShortHwClass) {
+  const std::string expected_full_hw_class = "feature_disabled";
+  scoped_feature_list_.InitAndDisableFeature(features::kUmaShortHWClass);
+  fake_statistics_provider_.SetMachineStatistic("hardware_class",
+                                                expected_full_hw_class);
+
+  TestChromeOSMetricsProvider provider;
+  provider.OnDidCreateMetricsLog();
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+
+  ASSERT_TRUE(system_profile.has_hardware());
+  std::string proto_full_hw_class =
+      system_profile.hardware().full_hardware_class();
+
+  // If disabled, the two hardware classes should be equal to each other.
+  EXPECT_EQ(system_profile.hardware().hardware_class(), proto_full_hw_class);
+  EXPECT_EQ(expected_full_hw_class, proto_full_hw_class);
+}
+
+TEST_F(ChromeOSMetricsProviderTest, EnableUmaShortHwClass) {
+  const std::string expected_full_hw_class = "feature_enabled";
+  scoped_feature_list_.InitAndEnableFeature(features::kUmaShortHWClass);
+  fake_statistics_provider_.SetMachineStatistic("hardware_class",
+                                                expected_full_hw_class);
+
+  TestChromeOSMetricsProvider provider;
+  provider.OnDidCreateMetricsLog();
+  metrics::SystemProfileProto system_profile;
+  provider.ProvideSystemProfileMetrics(&system_profile);
+
+  ASSERT_TRUE(system_profile.has_hardware());
+  std::string proto_full_hw_class =
+      system_profile.hardware().full_hardware_class();
+
+  // If enabled, the two hardware strings should be different.
+  EXPECT_NE(system_profile.hardware().hardware_class(), proto_full_hw_class);
+  EXPECT_EQ(expected_full_hw_class, proto_full_hw_class);
+}