From f44b2299146db3aba1a5a59cbe940abcd951e6aa Mon Sep 17 00:00:00 2001
From: Daosheng Mu <daoshengmu@gmail.com>
Date: Sun, 3 Feb 2019 07:19:58 +0000
Subject: [PATCH] Bug 1523926 - Fix open VR shmem mutex failed issue when
 without VR process. r=kip

MozReview-Commit-ID: 5P7D75wAWI7

Differential Revision: https://phabricator.services.mozilla.com/D18301

--HG--
extra : moz-landing-system : lando
---
 gfx/vr/gfxVRExternal.cpp     | 58 +++++++++++++++++++-----------------
 gfx/vr/gfxVRMutex.h          |  7 ++++-
 gfx/vr/service/VRService.cpp | 35 +++++++++++-----------
 3 files changed, 55 insertions(+), 45 deletions(-)

diff --git a/gfx/vr/gfxVRExternal.cpp b/gfx/vr/gfxVRExternal.cpp
index 1c85635eadeb..3f9022db596d 100644
--- a/gfx/vr/gfxVRExternal.cpp
+++ b/gfx/vr/gfxVRExternal.cpp
@@ -442,31 +442,41 @@ VRSystemManagerExternal::VRSystemManagerExternal(
   mEnumerationCompleted = false;
 #endif
   mDoShutdown = false;
+
+#if defined(XP_WIN)
+  mMutex = CreateMutex(
+    NULL,                   // default security descriptor
+    false,                  // mutex not owned
+    TEXT("mozilla::vr::ShmemMutex"));  // object name
+
+  if (mMutex == NULL) {
+    nsAutoCString msg;
+    msg.AppendPrintf("VRSystemManagerExternal CreateMutex error \"%lu\".",
+                      GetLastError());
+    NS_WARNING(msg.get());
+    MOZ_ASSERT(false);
+    return;
+  }
+  // At xpcshell extension tests, it creates multiple VRSystemManagerExternal instances
+  // in plug-contrainer.exe. It causes GetLastError() return `ERROR_ALREADY_EXISTS`.
+  // However, even though `ERROR_ALREADY_EXISTS`, it still returns the same mutex handle.
+  //
+  // https://docs.microsoft.com/en-us/windows/desktop/api/synchapi/nf-synchapi-createmutexa
+  MOZ_ASSERT(GetLastError() == 0 || GetLastError() == ERROR_ALREADY_EXISTS);
+#endif  // defined(XP_WIN)
 }
 
-VRSystemManagerExternal::~VRSystemManagerExternal() { CloseShmem(); }
+VRSystemManagerExternal::~VRSystemManagerExternal() {
+  CloseShmem();
+#if defined(XP_WIN)
+  if (mMutex) {
+    CloseHandle(mMutex);
+    mMutex = NULL;
+  }
+#endif
+}
 
 void VRSystemManagerExternal::OpenShmem() {
-#if defined(XP_WIN)
-  if (!mMutex) {
-     mMutex = CreateMutex(
-        NULL,                   // default security descriptor
-        false,                  // mutex not owned
-        TEXT("mozilla::vr::ShmemMutex"));  // object name
-
-    if (mMutex == NULL) {
-      nsAutoCString msg("VRService CreateMutex error \"%lu\".",
-                        GetLastError());
-      NS_WARNING(msg.get());
-      MOZ_ASSERT(false);
-      return;
-    }
-    else if (GetLastError() == ERROR_ALREADY_EXISTS) {
-      NS_WARNING("CreateMutex opened an existing mutex.");
-    }
-  }
-#endif  // defined(XP_WIN)
-
   if (mExternalShmem) {
     return;
 #if defined(MOZ_WIDGET_ANDROID)
@@ -570,12 +580,6 @@ void VRSystemManagerExternal::CheckForShutdown() {
 }
 
 void VRSystemManagerExternal::CloseShmem() {
-#if defined(XP_WIN)
-  if (mMutex) {
-    CloseHandle(mMutex);
-    mMutex = NULL;
-  }
-#endif
 #if !defined(MOZ_WIDGET_ANDROID)
   if (mSameProcess) {
     return;
diff --git a/gfx/vr/gfxVRMutex.h b/gfx/vr/gfxVRMutex.h
index f6dbb4a740eb..9728f7fac715 100644
--- a/gfx/vr/gfxVRMutex.h
+++ b/gfx/vr/gfxVRMutex.h
@@ -18,8 +18,9 @@ public:
     : mHandle(handle)
     , mStatus(false) {
 
-    DWORD dwWaitResult;
+    MOZ_ASSERT(mHandle);
 
+    DWORD dwWaitResult;
     dwWaitResult = WaitForSingleObject(
         mHandle,    // handle to mutex
         INFINITE);  // no time-out interval
@@ -43,6 +44,10 @@ public:
 
   ~WaitForMutex() {
     if (mHandle && !ReleaseMutex(mHandle)) {
+      nsAutoCString msg;
+      msg.AppendPrintf("WaitForMutex %d ReleaseMutex error \"%lu\".",
+                        mHandle, GetLastError());
+      NS_WARNING(msg.get());
       MOZ_ASSERT(false, "Failed to release mutex.");
     }
   }
diff --git a/gfx/vr/service/VRService.cpp b/gfx/vr/service/VRService.cpp
index ef3d76e7d066..621c01a2dd7f 100644
--- a/gfx/vr/service/VRService.cpp
+++ b/gfx/vr/service/VRService.cpp
@@ -103,6 +103,24 @@ void VRService::Refresh() {
 }
 
 void VRService::Start() {
+#if defined(XP_WIN)
+  if (!mMutex) {
+     mMutex = OpenMutex(
+        MUTEX_ALL_ACCESS,       // request full access
+        false,                  // handle not inheritable
+        TEXT("mozilla::vr::ShmemMutex"));  // object name
+
+    if (mMutex == NULL) {
+      nsAutoCString msg;
+      msg.AppendPrintf("VRService OpenMutex error \"%lu\".",
+                       GetLastError());
+      NS_WARNING(msg.get());
+      MOZ_ASSERT(false);
+    }
+    MOZ_ASSERT(GetLastError() == 0);
+  }
+#endif
+
   if (!mServiceThread) {
     /**
      * We must ensure that any time the service is re-started, that
@@ -166,23 +184,6 @@ void VRService::Stop() {
 }
 
 bool VRService::InitShmem() {
-#if defined(XP_WIN)
-  if (!mMutex) {
-     mMutex = OpenMutex(
-        MUTEX_ALL_ACCESS,       // request full access
-        false,                  // handle not inheritable
-        TEXT("mozilla::vr::ShmemMutex"));  // object name
-
-    if (mMutex == NULL) {
-      nsAutoCString msg("VRService OpenMutex error \"%lu\".",
-                        GetLastError());
-      NS_WARNING(msg.get());
-      MOZ_ASSERT(false);
-      return false;
-    }
-  }
-#endif
-
   if (!mVRProcessEnabled) {
     return true;
   }