Ensure VRService is notified when sessions stop.

crrev.com/c/2076431 revealed an underlying race condition from the
"TestDeviceServiceDisconnect" test case, where there is essentially a
race between when the IsolatedVRDeviceProvider receives the disconnect
from the underlying device provider and when the BrowserXrRuntime would
receive it's own disconnect from the underlying device.

If the IsolatedVRDeviceProvider wins the race, the BrowserXRRuntime may
be removed before it can notify the VRService that it (and the session
it is responsible for) is being ended. Because the renderer process has
its own pipe to the device process, it is notified of the disconnect
(and ended session), even if the Browser doesn't till it about this
termination. The end result is that the only state that doesn't end up
eventually getting cleaned up is that of the session metrics helper,
which thinks that a session is still active, and may hit a DCHECK when
attempting to start a new session.

Change-Id: I5a448130cab0bc245cd4f3e6ef77e44512a89652
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081528
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Auto-Submit: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746992}

chrome/browser/vr/service/browser_xr_runtime.cc

@@ -589,4 +589,15 @@ void BrowserXRRuntime::OnImmersiveSessionError() {
   StopImmersiveSession(base::DoNothing());
 }
 
+void BrowserXRRuntime::BeforeRuntimeRemoved() {
+  DVLOG(1) << __func__ << ": id=" << id_;
+
+  // If the device process crashes or otherwise gets removed, it's a race as to
+  // whether or not our mojo interface to the device gets reset before we're
+  // deleted as the result of the device provider being destroyed.
+  // Since this no-ops if we don't have an active immersive session, try to end
+  // any immersive session we may be currently responsible for.
+  StopImmersiveSession(base::DoNothing());
+}
+
 }  // namespace vr

chrome/browser/vr/service/browser_xr_runtime.h

@@ -103,6 +103,10 @@ class BrowserXRRuntime : public device::mojom::XRRuntimeEventListener {
   }
   device::mojom::XRDeviceId GetId() const { return id_; }
 
+  // Called to allow the runtime to conduct any cleanup it needs to do before it
+  // is removed.
+  void BeforeRuntimeRemoved();
+
  private:
   // device::XRRuntimeEventListener
   void OnDisplayInfoChanged(

chrome/browser/vr/service/xr_runtime_manager.cc

@@ -431,6 +431,7 @@ void XRRuntimeManager::AddRuntime(
 }
 
 void XRRuntimeManager::RemoveRuntime(device::mojom::XRDeviceId id) {
+  DVLOG(1) << __func__ << " id: " << id;
   TRACE_EVENT_INSTANT1("xr", "RemoveRuntime", TRACE_EVENT_SCOPE_THREAD, "id",
                        id);
 
@@ -438,6 +439,10 @@ void XRRuntimeManager::RemoveRuntime(device::mojom::XRDeviceId id) {
   auto it = runtimes_.find(id);
   DCHECK(it != runtimes_.end());
 
+  // Give the runtime a chance to clean itself up before notifying services
+  // that it was removed.
+  it->second->BeforeRuntimeRemoved();
+
   // Remove the runtime from runtimes_ before notifying services that it was
   // removed, since they will query for runtimes in RuntimesChanged.
   std::unique_ptr<BrowserXRRuntime> removed_runtime = std::move(it->second);