mojo VR interface simplified

device/vr/vr_service_impl_unittest.cc

Index: device/vr/vr_service_impl_unittest.cc
diff --git a/device/vr/vr_service_impl_unittest.cc b/device/vr/vr_service_impl_unittest.cc
index 300bf0dbfb960c8fc738333a2253a3fca33e7504..418078c814ef689174c932918032d204e20f6c0a 100644
--- a/device/vr/vr_service_impl_unittest.cc
+++ b/device/vr/vr_service_impl_unittest.cc
@@ -17,27 +17,10 @@ using ::testing::Mock;
 
 namespace device {
 
-class MockVRServiceClient : public VRServiceClient {
+class MockVRServiceClient : public mojom::VRServiceClient {
  public:
-  MOCK_METHOD1(OnDisplayChanged, void(const VRDisplay& display));
-  void OnDisplayChanged(VRDisplayPtr display) override {
-    OnDisplayChanged(*display);
-    last_display_ = std::move(display);
-  }
-
-  MOCK_METHOD1(OnExitPresent, void(uint32_t index));
-
-  MOCK_METHOD1(OnDisplayConnected, void(const VRDisplay& display));
-  void OnDisplayConnected(VRDisplayPtr display) override {
-    OnDisplayConnected(*display);
-    last_display_ = std::move(display);
-  }
-  void OnDisplayDisconnected(unsigned index) override {}
-
-  const VRDisplayPtr& LastDisplay() { return last_display_; }
-
- private:
-  VRDisplayPtr last_display_;
+  MOCK_METHOD1(GetDisplayClient,
+               void(mojom::VRDisplayClientPtr& display_client));

[dcheng, 2016/11/08 08:55:16]

I might be missing something, but I'm not seeing how this mock is used.

 };
 
 class VRServiceTestBinding {
@@ -47,26 +30,35 @@ class VRServiceTestBinding {
     service_impl_.reset(new VRServiceImpl());
     service_impl_->Bind(std::move(request));
 
-    VRServiceClientPtr client_ptr;
-    client_binding_.reset(new mojo::Binding<VRServiceClient>(
-        &mock_client_, mojo::GetProxy(&client_ptr)));
-    service_impl_->SetClient(std::move(client_ptr));
+    client_binding_.reset(new mojo::Binding<mojom::VRServiceClient>(

[dcheng, 2016/11/08 08:55:16]

Let's add a TODO to clean up this binding (this shouldn't need to be a
unique_ptr either).

It's also a bit unusual to have an external binding like this. I'd suggest just
not using gmock in this particular case.

[shaobo.yan, 2016/11/08 11:33:44]

Yes, and I'm preparing a patch to update a whole unittest. So I added a TODO for
this file.

+        mock_client_, mojo::GetProxy(&client_ptr_)));
   }
 
+  void SetClient() {
+    service_impl_->SetClient(
+        std::move(client_ptr_),
+        base::Bind(&device::VRServiceTestBinding::SetDeviceNum,
+                   base::Unretained(this)));
+  }
+
+  void SetDeviceNum(unsigned int device_num) { device_num_ = device_num; }

[dcheng, 2016/11/08 08:55:16]

SetNumberOfDevices

number_of_devices_?

+
   void Close() {
     service_ptr_.reset();
     service_impl_.reset();
   }
 
-  MockVRServiceClient& client() { return mock_client_; }
+  MockVRServiceClient* client() { return mock_client_; }
   VRServiceImpl* service() { return service_impl_.get(); }
 
  private:
+  mojom::VRServiceClientPtr client_ptr_;
   std::unique_ptr<VRServiceImpl> service_impl_;
-  mojo::InterfacePtr<VRService> service_ptr_;
+  mojo::InterfacePtr<mojom::VRService> service_ptr_;
 
-  MockVRServiceClient mock_client_;
-  std::unique_ptr<mojo::Binding<VRServiceClient>> client_binding_;
+  MockVRServiceClient* mock_client_;
+  std::unique_ptr<mojo::Binding<mojom::VRServiceClient>> client_binding_;
+  unsigned int device_num_;
 
   DISALLOW_COPY_AND_ASSIGN(VRServiceTestBinding);
 };
@@ -86,13 +78,14 @@ class VRServiceImplTest : public testing::Test {
   void TearDown() override { base::RunLoop().RunUntilIdle(); }
 
   std::unique_ptr<VRServiceTestBinding> BindService() {
-    return std::unique_ptr<VRServiceTestBinding>(new VRServiceTestBinding());
+    std::unique_ptr<VRServiceTestBinding> test_binding =
+        std::unique_ptr<VRServiceTestBinding>(new VRServiceTestBinding());
+    test_binding->SetClient();
+    return test_binding;
   }
 
   size_t ServiceCount() { return device_manager_->services_.size(); }
 
-  bool presenting() { return !!device_manager_->presenting_service_; }
-
   base::MessageLoop message_loop_;
   FakeVRDeviceProvider* provider_;
   std::unique_ptr<VRDeviceManager> device_manager_;
@@ -122,86 +115,4 @@ TEST_F(VRServiceImplTest, DeviceManagerRegistration) {
   EXPECT_EQ(0u, ServiceCount());
 }
 
-// Ensure that DeviceChanged calls are dispatched to all active services.
-TEST_F(VRServiceImplTest, DeviceChangedDispatched) {
-  std::unique_ptr<VRServiceTestBinding> service_1 = BindService();
-  std::unique_ptr<VRServiceTestBinding> service_2 = BindService();
-
-  EXPECT_CALL(service_1->client(), OnDisplayChanged(_));
-  EXPECT_CALL(service_2->client(), OnDisplayChanged(_));
-
-  std::unique_ptr<FakeVRDevice> device(new FakeVRDevice(provider_));
-  device_manager_->OnDeviceChanged(device->GetVRDevice());
-
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_EQ(device->id(), service_1->client().LastDisplay()->index);
-  EXPECT_EQ(device->id(), service_2->client().LastDisplay()->index);
-}
-
-// Ensure that presenting devices cannot be accessed by other services
-TEST_F(VRServiceImplTest, DevicePresentationIsolation) {
-  std::unique_ptr<VRServiceTestBinding> service_1 = BindService();
-  std::unique_ptr<VRServiceTestBinding> service_2 = BindService();
-
-  std::unique_ptr<FakeVRDevice> device(new FakeVRDevice(provider_));
-  provider_->AddDevice(device.get());
-
-  // Ensure the device manager has seen the fake device
-  device_manager_->GetVRDevices();
-
-  // When not presenting either service should be able to access the device
-  EXPECT_EQ(device.get(), VRDeviceManager::GetAllowedDevice(
-                              service_1->service(), device->id()));
-  EXPECT_EQ(device.get(), VRDeviceManager::GetAllowedDevice(
-                              service_2->service(), device->id()));
-
-  // Begin presenting to the fake device with service 1
-  EXPECT_TRUE(device_manager_->RequestPresent(service_1->service(),
-                                              device->id(), true));
-
-  EXPECT_TRUE(presenting());
-
-  // Service 2 should not be able to present to the device while service 1
-  // is still presenting.
-  EXPECT_FALSE(device_manager_->RequestPresent(service_2->service(),
-                                               device->id(), true));
-
-  // Only the presenting service should be able to access the device
-  EXPECT_EQ(device.get(), VRDeviceManager::GetAllowedDevice(
-                              service_1->service(), device->id()));
-  EXPECT_EQ(nullptr, VRDeviceManager::GetAllowedDevice(service_2->service(),
-                                                       device->id()));
-
-  // Service 2 should not be able to exit presentation to the device
-  device_manager_->ExitPresent(service_2->service(), device->id());
-  EXPECT_TRUE(presenting());
-
-  // Service 1 should be able to exit the presentation it initiated.
-  device_manager_->ExitPresent(service_1->service(), device->id());
-  EXPECT_FALSE(presenting());
-
-  // Once presention had ended both services should be able to access the device
-  EXPECT_EQ(device.get(), VRDeviceManager::GetAllowedDevice(
-                              service_1->service(), device->id()));
-  EXPECT_EQ(device.get(), VRDeviceManager::GetAllowedDevice(
-                              service_2->service(), device->id()));
-}
-
-// Ensure that DeviceChanged calls are dispatched to all active services.
-TEST_F(VRServiceImplTest, DeviceConnectedDispatched) {
-  std::unique_ptr<VRServiceTestBinding> service_1 = BindService();
-  std::unique_ptr<VRServiceTestBinding> service_2 = BindService();
-
-  EXPECT_CALL(service_1->client(), OnDisplayConnected(_));
-  EXPECT_CALL(service_2->client(), OnDisplayConnected(_));
-
-  std::unique_ptr<FakeVRDevice> device(new FakeVRDevice(provider_));
-  device_manager_->OnDeviceConnectionStatusChanged(device.get(), true);
-
-  base::RunLoop().RunUntilIdle();
-
-  EXPECT_EQ(device->id(), service_1->client().LastDisplay()->index);
-  EXPECT_EQ(device->id(), service_2->client().LastDisplay()->index);
-}
 }