Fix leak in VRServiceImpl

device/vr/vr_service_impl.cc

Index: device/vr/vr_service_impl.cc
diff --git a/device/vr/vr_service_impl.cc b/device/vr/vr_service_impl.cc
index 8be4a83e1660bb0aabeedd5429f67eec8559cfa0..49ba2199cd4ae6cdce8b51ebcf4e485f24767aff 100644
--- a/device/vr/vr_service_impl.cc
+++ b/device/vr/vr_service_impl.cc
@@ -9,19 +9,29 @@
 #include "base/bind.h"
 #include "device/vr/vr_device.h"
 #include "device/vr/vr_device_manager.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
 
 namespace device {
 
 VRServiceImpl::VRServiceImpl() : listening_for_activate_(false) {}
 
 VRServiceImpl::~VRServiceImpl() {
-  RemoveFromDeviceManager();
+  // Remove VRDisplayImpl in service/device pair.
+  // displays_ is a map which first element is a VRDevice pointer,

[dcheng, 2016/12/16 02:50:17]

first element => the key
second element => the value

[mthiesse, 2016/12/16 15:45:44]

Acknowledged.

+  // the second element is a VRDisplayImpl pointer.
+  for (DisplayImplMap::iterator it = displays_.begin(); it != displays_.end();

[dcheng, 2016/12/16 02:50:17]

Nit:

for (auto& it : displays_)
  it->first->RemoveDisplay(it->second.get());

Out of curiosity though, why can't VRDisplayImpl just remove itself from the
device on destruction? It has a pointer to the device, and it seems a bit nicer
than embedding that logic here.

[mthiesse, 2016/12/16 15:45:44]

Good point, done.

+       ++it) {
+    it->first->RemoveDisplay(it->second.get());
+  }
+
+  VRDeviceManager* device_manager = VRDeviceManager::GetInstance();
+  device_manager->RemoveService(this);
+  displays_.clear();
 }
 
-void VRServiceImpl::BindRequest(
-    mojo::InterfaceRequest<mojom::VRService> request) {
-  VRServiceImpl* service = new VRServiceImpl();
-  service->Bind(std::move(request));
+void VRServiceImpl::Create(mojo::InterfaceRequest<mojom::VRService> request) {
+  mojo::MakeStrongBinding(base::MakeUnique<VRServiceImpl>(),
+                          std::move(request));
 }
 
 // Gets a VRDisplayPtr unique to this service so that the associated page can
@@ -36,30 +46,6 @@ VRDisplayImpl* VRServiceImpl::GetVRDisplayImpl(VRDevice* device) {
   return display_impl;
 }
 
-void VRServiceImpl::Bind(mojo::InterfaceRequest<mojom::VRService> request) {
-  if (!binding_)
-    binding_.reset(
-        new mojo::Binding<mojom::VRService>(this, std::move(request)));
-  else
-    binding_->Bind(std::move(request));
-  binding_->set_connection_error_handler(base::Bind(
-      &VRServiceImpl::RemoveFromDeviceManager, base::Unretained(this)));
-}
-
-void VRServiceImpl::RemoveFromDeviceManager() {
-  // Remove VRDisplayImpl in service/device pair.
-  // displays_ is a map which first element is a VRDevice pointer,
-  // the second element is a VRDisplayImpl pointer.
-  for (DisplayImplMap::iterator it = displays_.begin(); it != displays_.end();
-       ++it) {
-    it->first->RemoveDisplay(it->second.get());
-  }
-
-  VRDeviceManager* device_manager = VRDeviceManager::GetInstance();
-  device_manager->RemoveService(this);
-  displays_.clear();
-}
-
 void VRServiceImpl::RemoveDevice(VRDevice* device) {
   displays_.erase(device);
 }