[DeviceSync v2] Add FeatureStatusChange enum to DeviceSync mojo service

This CL prepares the way for the addition of SetFeatureStatus() to the
DeviceSync mojo service. See http://crrev/c/1831010.

Bug: 951969
Change-Id: I488984c741ed7050bef2a78163ef6e93f7aee2b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895887Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712711}

chromeos/services/device_sync/BUILD.gn

@@ -4,6 +4,12 @@
 
 assert(is_chromeos, "Non-ChromeOS builds cannot depend on //chromeos")
 
+static_library("feature_status_change") {
+  sources = [
+    "feature_status_change.h",
+  ]
+}
+
 static_library("device_sync") {
   sources = [
     "async_execution_time_metrics_logger.cc",
@@ -135,6 +141,7 @@ static_library("device_sync") {
   ]
 
   public_deps = [
+    ":feature_status_change",
     "//chromeos/components/multidevice",
     "//chromeos/components/multidevice/logging",
     "//chromeos/services/device_sync/public/mojom",
@@ -271,6 +278,7 @@ source_set("unit_tests") {
 
   deps = [
     ":device_sync",
+    ":feature_status_change",
     ":test_support",
     "//base",
     "//base/test:test_support",

chromeos/services/device_sync/cryptauth_feature_status_setter.h

@@ -10,6 +10,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "chromeos/components/multidevice/software_feature.h"
+#include "chromeos/services/device_sync/feature_status_change.h"
 #include "chromeos/services/device_sync/network_request_error.h"
 
 namespace chromeos {
@@ -23,20 +24,6 @@ namespace device_sync {
 // with the CryptAuth v1 DeviceSync interface.
 class CryptAuthFeatureStatusSetter {
  public:
-  enum class FeatureStatusChange {
-    // Enables a feature on a device and disables the feature on all other
-    // devices associated with the account.
-    kEnableExclusively,
-
-    // Enables a feature on a device; other devices on the account are
-    // unaffected.
-    kEnableNonExclusively,
-
-    // Disables a feature on a device; other devices on the account are
-    // unaffected.
-    kDisable
-  };
-
   CryptAuthFeatureStatusSetter() = default;
   virtual ~CryptAuthFeatureStatusSetter() = default;
 

chromeos/services/device_sync/cryptauth_feature_status_setter_impl.h

@@ -17,6 +17,7 @@
 #include "base/timer/timer.h"
 #include "chromeos/components/multidevice/software_feature.h"
 #include "chromeos/services/device_sync/cryptauth_feature_status_setter.h"
+#include "chromeos/services/device_sync/feature_status_change.h"
 #include "chromeos/services/device_sync/network_request_error.h"
 #include "chromeos/services/device_sync/proto/cryptauth_client_app_metadata.pb.h"
 #include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h"

chromeos/services/device_sync/cryptauth_feature_status_setter_impl_unittest.cc

@@ -16,6 +16,7 @@
 #include "chromeos/services/device_sync/cryptauth_client.h"
 #include "chromeos/services/device_sync/cryptauth_key_bundle.h"
 #include "chromeos/services/device_sync/fake_cryptauth_gcm_manager.h"
+#include "chromeos/services/device_sync/feature_status_change.h"
 #include "chromeos/services/device_sync/mock_cryptauth_client.h"
 #include "chromeos/services/device_sync/network_request_error.h"
 #include "chromeos/services/device_sync/proto/cryptauth_client_app_metadata.pb.h"
@@ -153,10 +154,9 @@ class DeviceSyncCryptAuthFeatureStatusSetterImplTest
         .WillByDefault(testing::Return(kAccessTokenUsed));
   }
 
-  void SetFeatureStatus(
-      const std::string& device_id,
-      multidevice::SoftwareFeature feature,
-      CryptAuthFeatureStatusSetter::FeatureStatusChange status_change) {
+  void SetFeatureStatus(const std::string& device_id,
+                        multidevice::SoftwareFeature feature,
+                        FeatureStatusChange status_change) {
     feature_status_setter_->SetFeatureStatus(
         device_id, feature, status_change,
         base::BindOnce(&DeviceSyncCryptAuthFeatureStatusSetterImplTest::
@@ -286,22 +286,21 @@ TEST_F(DeviceSyncCryptAuthFeatureStatusSetterImplTest, Test) {
   // sequentially.
   SetFeatureStatus("device_id_1",
                    multidevice::SoftwareFeature::kSmartLockClient,
-                   CryptAuthFeatureStatusSetter::FeatureStatusChange::kDisable);
-  SetFeatureStatus(
-      "device_id_2", multidevice::SoftwareFeature::kInstantTetheringHost,
-      CryptAuthFeatureStatusSetter::FeatureStatusChange::kEnableNonExclusively);
-  SetFeatureStatus(
-      "device_id_3", multidevice::SoftwareFeature::kSmartLockHost,
-      CryptAuthFeatureStatusSetter::FeatureStatusChange::kEnableExclusively);
+                   FeatureStatusChange::kDisable);
+  SetFeatureStatus("device_id_2",
+                   multidevice::SoftwareFeature::kInstantTetheringHost,
+                   FeatureStatusChange::kEnableNonExclusively);
+  SetFeatureStatus("device_id_3", multidevice::SoftwareFeature::kSmartLockHost,
+                   FeatureStatusChange::kEnableExclusively);
   SetFeatureStatus("device_id_4",
                    multidevice::SoftwareFeature::kInstantTetheringClient,
-                   CryptAuthFeatureStatusSetter::FeatureStatusChange::kDisable);
+                   FeatureStatusChange::kDisable);
   SetFeatureStatus("device_id_5",
                    multidevice::SoftwareFeature::kInstantTetheringClient,
-                   CryptAuthFeatureStatusSetter::FeatureStatusChange::kDisable);
-  SetFeatureStatus(
-      "device_id_6", multidevice::SoftwareFeature::kBetterTogetherHost,
-      CryptAuthFeatureStatusSetter::FeatureStatusChange::kEnableNonExclusively);
+                   FeatureStatusChange::kDisable);
+  SetFeatureStatus("device_id_6",
+                   multidevice::SoftwareFeature::kBetterTogetherHost,
+                   FeatureStatusChange::kEnableNonExclusively);
 
   // base::nullopt indicates a success.
   std::vector<base::Optional<NetworkRequestError>> expected_results;

chromeos/services/device_sync/feature_status_change.h

+// Copyright 2019 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.
+
+#ifndef CHROMEOS_SERVICES_DEVICE_SYNC_FEATURE_STATUS_CHANGE_H_
+#define CHROMEOS_SERVICES_DEVICE_SYNC_FEATURE_STATUS_CHANGE_H_
+
+namespace chromeos {
+
+namespace device_sync {
+
+// The target status change for a device's SoftwareFeature.
+enum class FeatureStatusChange {
+  // Enables a feature on a device and disables the feature on all other devices
+  // associated with the account.
+  kEnableExclusively = 0,
+
+  // Enables a feature on a device; other devices on the account are unaffected.
+  kEnableNonExclusively = 1,
+
+  // Disables a feature on a device; other devices on the account are
+  // unaffected.
+  kDisable = 2
+};
+
+}  // namespace device_sync
+
+}  // namespace chromeos
+
+#endif  //  CHROMEOS_SERVICES_DEVICE_SYNC_FEATURE_STATUS_CHANGE_H_

chromeos/services/device_sync/public/mojom/BUILD.gn

@@ -26,6 +26,8 @@ source_set("unit_tests") {
     ":mojom",
     "//base",
     "//base/test:test_support",
+    "//chromeos/services/device_sync:feature_status_change",
+    "//chromeos/services/device_sync/proto",
     "//mojo/public/cpp/test_support:test_utils",
     "//testing/gtest",
   ]

chromeos/services/device_sync/public/mojom/device_sync.mojom

@@ -42,6 +42,20 @@ enum NetworkRequestResult {
   kUnknown
 };
 
+enum FeatureStatusChange {
+  // Enables a feature on a device and disables the feature on all other
+  // devices associated with the account.
+  kEnableExclusively,
+
+  // Enables a feature on a device; other devices on the account are
+  // unaffected.
+  kEnableNonExclusively,
+
+  // Disables a feature on a device; other devices on the account are
+  // unaffected.
+  kDisable
+};
+
 struct FindEligibleDevicesResponse {
   array<chromeos.multidevice.mojom.RemoteDevice> eligible_devices;
   array<chromeos.multidevice.mojom.RemoteDevice> ineligible_devices;

chromeos/services/device_sync/public/mojom/device_sync.typemap

@@ -4,8 +4,10 @@
 
 mojom = "//chromeos/services/device_sync/public/mojom/device_sync.mojom"
 
-public_headers =
-    [ "//chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h" ]
+public_headers = [
+  "//chromeos/services/device_sync/feature_status_change.h",
+  "//chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h",
+]
 
 traits_headers = [
   "//chromeos/services/device_sync/public/mojom/device_sync_mojom_traits.h",
@@ -16,4 +18,12 @@ sources = [
   "//chromeos/services/device_sync/public/mojom/device_sync_mojom_traits.h",
 ]
 
-type_mappings = [ "chromeos.device_sync.mojom.ConnectivityStatus=cryptauthv2::ConnectivityStatus" ]
+public_deps = [
+  "//chromeos/services/device_sync:feature_status_change",
+  "//chromeos/services/device_sync/proto",
+]
+
+type_mappings = [
+  "chromeos.device_sync.mojom.ConnectivityStatus=cryptauthv2::ConnectivityStatus",
+  "chromeos.device_sync.mojom.FeatureStatusChange=chromeos::device_sync::FeatureStatusChange",
+]

chromeos/services/device_sync/public/mojom/device_sync_mojom_traits.cc

@@ -43,4 +43,41 @@ bool EnumTraits<chromeos::device_sync::mojom::ConnectivityStatus,
   return false;
 }
 
+chromeos::device_sync::mojom::FeatureStatusChange
+EnumTraits<chromeos::device_sync::mojom::FeatureStatusChange,
+           chromeos::device_sync::FeatureStatusChange>::
+    ToMojom(chromeos::device_sync::FeatureStatusChange input) {
+  switch (input) {
+    case chromeos::device_sync::FeatureStatusChange::kEnableExclusively:
+      return chromeos::device_sync::mojom::FeatureStatusChange::
+          kEnableExclusively;
+    case chromeos::device_sync::FeatureStatusChange::kEnableNonExclusively:
+      return chromeos::device_sync::mojom::FeatureStatusChange::
+          kEnableNonExclusively;
+    case chromeos::device_sync::FeatureStatusChange::kDisable:
+      return chromeos::device_sync::mojom::FeatureStatusChange::kDisable;
+  }
+}
+
+bool EnumTraits<chromeos::device_sync::mojom::FeatureStatusChange,
+                chromeos::device_sync::FeatureStatusChange>::
+    FromMojom(chromeos::device_sync::mojom::FeatureStatusChange input,
+              chromeos::device_sync::FeatureStatusChange* out) {
+  switch (input) {
+    case chromeos::device_sync::mojom::FeatureStatusChange::kEnableExclusively:
+      *out = chromeos::device_sync::FeatureStatusChange::kEnableExclusively;
+      return true;
+    case chromeos::device_sync::mojom::FeatureStatusChange::
+        kEnableNonExclusively:
+      *out = chromeos::device_sync::FeatureStatusChange::kEnableNonExclusively;
+      return true;
+    case chromeos::device_sync::mojom::FeatureStatusChange::kDisable:
+      *out = chromeos::device_sync::FeatureStatusChange::kDisable;
+      return true;
+  }
+
+  NOTREACHED();
+  return false;
+}
+
 }  // namespace mojo

chromeos/services/device_sync/public/mojom/device_sync_mojom_traits.h

@@ -5,11 +5,13 @@
 #ifndef CHROMEOS_SERVICES_DEVICE_SYNC_PUBLIC_MOJOM_DEVICE_SYNC_MOJOM_TRAITS_H_
 #define CHROMEOS_SERVICES_DEVICE_SYNC_PUBLIC_MOJOM_DEVICE_SYNC_MOJOM_TRAITS_H_
 
+#include "chromeos/services/device_sync/feature_status_change.h"
 #include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h"
 #include "chromeos/services/device_sync/public/mojom/device_sync.mojom.h"
 #include "mojo/public/cpp/bindings/enum_traits.h"
 
 namespace mojo {
+
 template <>
 class EnumTraits<chromeos::device_sync::mojom::ConnectivityStatus,
                  cryptauthv2::ConnectivityStatus> {
@@ -20,6 +22,16 @@ class EnumTraits<chromeos::device_sync::mojom::ConnectivityStatus,
                         cryptauthv2::ConnectivityStatus* out);
 };
 
+template <>
+class EnumTraits<chromeos::device_sync::mojom::FeatureStatusChange,
+                 chromeos::device_sync::FeatureStatusChange> {
+ public:
+  static chromeos::device_sync::mojom::FeatureStatusChange ToMojom(
+      chromeos::device_sync::FeatureStatusChange input);
+  static bool FromMojom(chromeos::device_sync::mojom::FeatureStatusChange input,
+                        chromeos::device_sync::FeatureStatusChange* out);
+};
+
 }  // namespace mojo
 
-#endif  // CHROMEOS_SERVICES_DEVICE_SYNC_PUBLIC_MOJOM_DEVICE_SYNC_MOJOM_TRAITS_H_
+#endif  // CHROMEOS_SERVICES_DEVICE_SYNC_PUBLIC_MOJOM_DEVICE_SYNC_MOJOM_TRAITS_H_
\ No newline at end of file

chromeos/services/device_sync/public/mojom/device_sync_mojom_traits_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "chromeos/services/device_sync/public/mojom/device_sync_mojom_traits.h"
 
+#include "chromeos/services/device_sync/feature_status_change.h"
 #include "chromeos/services/device_sync/proto/cryptauth_devicesync.pb.h"
 #include "chromeos/services/device_sync/public/mojom/device_sync.mojom.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
@@ -27,3 +28,25 @@ TEST(DeviceSyncMojomTraitsTest, ConnectivityStatus) {
     EXPECT_EQ(status_in, status_out);
   }
 }
+
+TEST(DeviceSyncMojomTraitsTest, FeatureStatusChange) {
+  static constexpr chromeos::device_sync::FeatureStatusChange
+      kTestFeatureStatusChanges[] = {
+          chromeos::device_sync::FeatureStatusChange::kEnableExclusively,
+          chromeos::device_sync::FeatureStatusChange::kEnableNonExclusively,
+          chromeos::device_sync::FeatureStatusChange::kDisable};
+
+  for (auto status_in : kTestFeatureStatusChanges) {
+    chromeos::device_sync::FeatureStatusChange status_out;
+
+    chromeos::device_sync::mojom::FeatureStatusChange serialized_status =
+        mojo::EnumTraits<
+            chromeos::device_sync::mojom::FeatureStatusChange,
+            chromeos::device_sync::FeatureStatusChange>::ToMojom(status_in);
+    ASSERT_TRUE(
+        (mojo::EnumTraits<chromeos::device_sync::mojom::FeatureStatusChange,
+                          chromeos::device_sync::FeatureStatusChange>::
+             FromMojom(serialized_status, &status_out)));
+    EXPECT_EQ(status_in, status_out);
+  }
+}