Win 10 Inactive toast experiment metrics and storage modifications.

chrome/installer/util/experiment_storage_unittest.cc

+// Copyright 2017 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/installer/util/experiment_storage.h"
+
+#include "base/base64.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/test_reg_util_win.h"
+#include "chrome/install_static/install_details.h"
+#include "chrome/installer/util/experiment.h"
+#include "chrome/installer/util/experiment_metrics.h"
+#include "chrome/installer/util/google_update_settings.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace installer {
+
+class ExperimentStorageTest : public ::testing::Test {
+ protected:
+  void TestExperimentMetrics(ExperimentMetrics* metrics) {
+    metrics->SetState(ExperimentMetrics::kGroupAssigned);
+    metrics->toast_location = ExperimentMetrics::kOverTaskbarPin;
+    metrics->toast_count = 1;
+    metrics->first_toast_offset = 30;
+    metrics->toast_hour = 3;
+    metrics->last_used_bucket = 2;
+    metrics->display_time_bucket = 11;
+    metrics->session_length_bucket = 36;
+  }
+
+  void SetUp() {
+    const install_static::InstallDetails& details =

[grt (UTC plus 2), 2017/05/24 13:43:39]

please make this a parameterized test with a bool parameter for user vs. system
level, and use install_static::ScopedInstallDetails in the test fixture to test
both variants (see GoogleUpdateWinTest for an example of this).

[nikunjb, 2017/05/30 21:45:02]

Done.

+        install_static::InstallDetails::Get();
+    HKEY root = details.system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+    registry_util::RegistryOverrideManager override_manager;

[grt (UTC plus 2), 2017/05/24 13:43:39]

this needs to be a member variable of the class so that the override is alive
for the actual test.

[nikunjb, 2017/05/30 21:45:01]

Done.

+    ASSERT_NO_FATAL_FAILURE(override_manager.OverrideRegistry(root));
+    GoogleUpdateSettings::SetGoogleUpdateIntegrationForTesting(true);

[grt (UTC plus 2), 2017/05/24 13:43:39]

i propose getting rid of this in favor of moving the kUseGoogleUpdateIntegration
check from {Set,Read}ExperimentLabels into
chrome/browser/metrics/variations/chrome_variations_service_client.cc. i think
this will allow TestLoadStoreMetrics and TestLoadStoreExperiment to work in
Chromium builds. wdyt?

[nikunjb, 2017/05/30 21:45:02]

SGTM.

+  }
+
+  void TearDown() {
+    GoogleUpdateSettings::SetGoogleUpdateIntegrationForTesting(false);
+  }
+};
+
+TEST_F(ExperimentStorageTest, TestEncodeDecodeMetrics) {
+  ExperimentMetrics metrics;
+  TestExperimentMetrics(&metrics);
+  base::string16 encoded_metrics(ExperimentStorage::EncodeMetrics(metrics));
+  ExperimentMetrics decoded_metrics;
+  ASSERT_TRUE(
+      ExperimentStorage::DecodeMetrics(encoded_metrics, &decoded_metrics));
+  ASSERT_EQ(encoded_metrics, base::ASCIIToUTF16("5BIMD4IA"));

[grt (UTC plus 2), 2017/05/24 13:43:39]

move this up so it immediately follows line 48, and change it to EXPECT_EQ

[nikunjb, 2017/05/30 21:45:02]

Done.

+  base::string16 decoded_metrics_str(
+      ExperimentStorage::EncodeMetrics(decoded_metrics));
+  ASSERT_EQ(encoded_metrics, decoded_metrics_str);

[grt (UTC plus 2), 2017/05/24 13:43:38]

EXPECT rather than ASSERT, and inline the call to
ExperimentStorage::EncodeMetrics(decoded_metrics).

[nikunjb, 2017/05/30 21:45:01]

Done.

+}
+
+TEST_F(ExperimentStorageTest, TestReadWriteParticipation) {
+#if defined(GOOGLE_CHROME_BUILD)

[grt (UTC plus 2), 2017/05/24 13:43:38]

i don't think this is needed here

[nikunjb, 2017/05/30 21:45:02]

Done.

+  ExperimentStorage storage;
+  ExperimentStorage::Participation expected =
+      ExperimentStorage::Participation::kIsParticipating;
+  ASSERT_TRUE(storage.AcquireLock()->WriteParticipation(expected));
+  ExperimentStorage::Participation p;
+  ASSERT_TRUE(storage.AcquireLock()->ReadParticipation(&p));
+  ASSERT_EQ(p, expected);
+#endif
+}
+
+TEST_F(ExperimentStorageTest, TestLoadStoreExperiment) {
+#if defined(GOOGLE_CHROME_BUILD)
+  Experiment experiment;
+  ExperimentMetrics metrics;
+  TestExperimentMetrics(&metrics);
+  experiment.InitializeFromMetrics(metrics);
+  ExperimentStorage storage;
+  ASSERT_TRUE(storage.AcquireLock()->StoreExperiment(experiment));
+  Experiment stored_experiment;
+  ASSERT_TRUE(storage.AcquireLock()->LoadExperiment(&stored_experiment));
+  ASSERT_EQ(stored_experiment.state(), ExperimentMetrics::kGroupAssigned);
+#endif
+}
+
+TEST_F(ExperimentStorageTest, TestLoadStoreMetrics) {
+#if defined(GOOGLE_CHROME_BUILD)
+  ExperimentStorage storage;
+  ExperimentMetrics metrics;
+  TestExperimentMetrics(&metrics);
+  ASSERT_TRUE(storage.AcquireLock()->StoreMetrics(metrics));
+  ExperimentMetrics stored_metrics;
+  ASSERT_TRUE(storage.AcquireLock()->LoadMetrics(&stored_metrics));
+  base::string16 encoded_metrics(ExperimentStorage::EncodeMetrics(metrics));
+  base::string16 encoded_stored_metrics(
+      ExperimentStorage::EncodeMetrics(stored_metrics));
+  ASSERT_EQ(encoded_stored_metrics, encoded_metrics);
+  ASSERT_EQ(encoded_stored_metrics, base::ASCIIToUTF16("5BIMD4IA"));
+#endif
+}
+}  // namespace installer

[grt (UTC plus 2), 2017/05/24 13:43:38]

nit: blank line before this