[CrOS MultiDevice] DeviceSync: Update GetSyncedDevices() return type.

Previously, GetSyncedDevices() returned an array of RemoteDevice
structs (in error cases, an empty list was returned). This was
problematic because a client of this service had no way of
differentiating between the case where there actually are no synced
devices and an error case.

This CL changes the return type to be a nullable array of RemoteDevice
structs; now, in error cases, null is returned instead of an empty list.

Bug: 824568, 752273
Change-Id: I6cfaa3933730beeb36082c8351d0f5b5ade0b086
Reviewed-on: https://chromium-review.googlesource.com/1020290
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553365}

chromeos/services/device_sync/device_sync_impl.cc

@@ -165,9 +165,8 @@ void DeviceSyncImpl::ForceSyncNow(ForceSyncNowCallback callback) {
 void DeviceSyncImpl::GetSyncedDevices(GetSyncedDevicesCallback callback) {
   if (status_ != Status::READY) {
     PA_LOG(WARNING) << "DeviceSyncImpl::GetSyncedDevices() invoked before "
-                    << "initialization was complete. Returning empty list of "
-                    << "synced devices.";
-    std::move(callback).Run(std::vector<cryptauth::RemoteDevice>());
+                    << "initialization was complete. Cannot return devices.";
+    std::move(callback).Run(base::nullopt);
     return;
   }
 

chromeos/services/device_sync/device_sync_service_unittest.cc

@@ -672,10 +672,8 @@ class DeviceSyncServiceTest : public testing::Test {
     EXPECT_FALSE(CallForceEnrollmentNow());
     EXPECT_FALSE(CallForceSyncNow());
 
-    // GetSyncedDevices() returns an empty list before initialization.
-    // TODO(khorimoto): Update it to return a null array instead of an empty
-    // list to differentiate between cases where there are actually no devices.
-    EXPECT_TRUE(CallGetSyncedDevices().empty());
+    // GetSyncedDevices() returns a null list before initialization.
+    EXPECT_FALSE(CallGetSyncedDevices());
 
     // SetSoftwareFeatureState() should return a struct with the special
     // kErrorNotInitialized error code.
@@ -735,7 +733,7 @@ class DeviceSyncServiceTest : public testing::Test {
     return last_force_sync_now_result_;
   }
 
-  const cryptauth::RemoteDeviceList& CallGetSyncedDevices() {
+  const base::Optional<cryptauth::RemoteDeviceList>& CallGetSyncedDevices() {
     base::RunLoop run_loop;
     device_sync_->GetSyncedDevices(
         base::BindOnce(&DeviceSyncServiceTest::OnGetSyncedDevicesCompleted,
@@ -827,7 +825,7 @@ class DeviceSyncServiceTest : public testing::Test {
 
   void OnGetSyncedDevicesCompleted(
       base::OnceClosure quit_closure,
-      const cryptauth::RemoteDeviceList& synced_devices) {
+      const base::Optional<cryptauth::RemoteDeviceList>& synced_devices) {
     last_synced_devices_result_ = synced_devices;
     std::move(quit_closure).Run();
   }
@@ -896,7 +894,7 @@ class DeviceSyncServiceTest : public testing::Test {
   bool device_already_enrolled_in_cryptauth_;
   bool last_force_enrollment_now_result_;
   bool last_force_sync_now_result_;
-  cryptauth::RemoteDeviceList last_synced_devices_result_;
+  base::Optional<cryptauth::RemoteDeviceList> last_synced_devices_result_;
   std::unique_ptr<base::Optional<std::string>>
       last_set_software_feature_state_response_;
   std::unique_ptr<std::pair<base::Optional<std::string>,
@@ -954,7 +952,7 @@ TEST_F(DeviceSyncServiceTest,
   CompleteConnectionToPrefService();
 
   // Initialization has not yet completed, so no devices should be available.
-  EXPECT_TRUE(CallGetSyncedDevices().empty());
+  EXPECT_FALSE(CallGetSyncedDevices());
 
   // Simulate enrollment failing.
   SimulateEnrollment(false /* success */);

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

@@ -126,9 +126,9 @@ interface DeviceSync {
   [Sync]
   ForceSyncNow() => (bool success);
 
-  // Returns all synced devices associated with the primary account.
-  [Sync]
-  GetSyncedDevices() => (array<RemoteDevice> devices);
+  // Returns all synced devices associated with the primary account. If this
+  // device has not yet registered with the back-end, no list is provided.
+  GetSyncedDevices() => (array<RemoteDevice>? devices);
 
   // Enables or disables the given software feature for the device with the
   // given public key. If |enabled| and |is_exclusive| are both true, this