Fixes 2D-to-WebVR site transitions when browsing in VR.

device/vr/vr_service_impl.h

 // Copyright 2016 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 DEVICE_VR_VR_SERVICE_IMPL_H
 #define DEVICE_VR_VR_SERVICE_IMPL_H
 
 #include <memory>
 
 #include "base/macros.h"
 
 #include "device/vr/vr_device.h"
 #include "device/vr/vr_display_impl.h"
 #include "device/vr/vr_export.h"
 #include "device/vr/vr_service.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
 namespace device {
 
+// Browser process representation of a WebVR site session. Instantiated through
+// Mojo once the user navigates to a web site containing WebVR.

[mthiesse, 2017/03/13 22:17:29]

nit: s/navigates to a web site/loads a page/

[tiborg, 2017/03/13 23:00:58]

Done.

+// It instantiates a VRDisplayImpl for each newly connected VRDisplay and sends
+// the display's info to the browser process through its connected
+// mojom::VRServiceClient.
 class VRServiceImpl : public mojom::VRService {

[amp, 2017/03/13 22:26:05]

Not required for this change, but VRServiceImpl seems so generic as to be
useless (in fact rather confusing given the explanation comment above) and I
recommend we try to think of a better alternative (if that's not already being
done).

VRSession would make more sense to me.

[tiborg, 2017/03/13 23:00:58]

Yep, agreed. VRSession sounds good to me. Should be a follow-up CL. There will
be quite some renaming if done properly because you would also have to change
the Mojo interfaces etc.

[bajones, 2017/03/13 23:04:57]

May want to be careful with that. VRSession will have a specific meaning in the
upcoming spec refactor (an interface created from the VRDevice which enables
presentation and pose polling). Doesn't seem like this class maps to that
concept very well.

[amp, 2017/03/14 17:05:50]

Thanks for the clarification Brandon.  I thought I remembered something about VR
Session from the spec (which is why I had it in mind probably), but I agree we
should line up the naming and functionality correctly with the spec (so use
something other than VR session here if it doesn't line up).  Either way we
should handle this later and after some more discussion it sounds like.

  public:
   DEVICE_VR_EXPORT VRServiceImpl();
   DEVICE_VR_EXPORT ~VRServiceImpl() override;
 
   DEVICE_VR_EXPORT static void Create(
       mojo::InterfaceRequest<mojom::VRService> request);
 
-  mojom::VRServiceClient* client() { return client_.get(); }
-
-  DEVICE_VR_EXPORT VRDisplayImpl* GetVRDisplayImpl(VRDevice* device);
-
   // mojom::VRService implementation
+  // Adds this service to the VRDeviceManager.
   void SetClient(mojom::VRServiceClientPtr service_client,
                  const SetClientCallback& callback) override;
 
   bool listening_for_activate() { return listening_for_activate_; }
 
+  // Tell the render process that a new VR device is available.
+  void ConnectDevice(VRDevice* device);
+
  private:
   friend class FakeVRServiceClient;
   friend class VRDeviceManagerTest;
-  friend class VRDisplayImpl;
   friend class VRDisplayImplTest;
   friend class VRServiceImplTest;
 
-  void RemoveDevice(VRDevice* device);
-
   void SetListeningForActivate(bool listening) override;
+  void OnVRDisplayInfoCreated(VRDevice* device,
+                              mojom::VRDisplayInfoPtr display_info);
 
   std::map<VRDevice*, std::unique_ptr<VRDisplayImpl>> displays_;
 
   mojom::VRServiceClientPtr client_;
 
   bool listening_for_activate_;
 
+  base::WeakPtrFactory<VRServiceImpl> weak_ptr_factory_;
+
+  // Getter for testing.
+  VRDisplayImpl* GetVRDisplayImpl(VRDevice* device);
+
   DISALLOW_COPY_AND_ASSIGN(VRServiceImpl);
 };
 
 }  // namespace device
 
 #endif  // DEVICE_VR_VR_SERVICE_IMPL_H