Bug 1696568 - Ensure skeleton UI window is always consumed if created r=emalysz

If we error out in, say, DrawSkeletonUI, the window we created will be orphaned
and left to sit there indefinitely. This patch fixes that by separating the
error from the consume result.

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

mozglue/misc/PreXULSkeletonUI.cpp

@@ -162,7 +162,7 @@ static int sWindowWidth;
 static int sWindowHeight;
 static double sCSSToDevPixelScaling;
 
-static Maybe<PreXULSkeletonUIError> sDisabledReason;
+static Maybe<PreXULSkeletonUIError> sErrorReason;
 
 static const int kAnimationCSSPixelsPerFrame = 21;
 static const int kAnimationCSSExtraWindowSize = 300;
@@ -248,9 +248,9 @@ Result<UniquePtr<wchar_t[]>, PreXULSkeletonUIError> GetBinaryPath() {
 // don't mess with registry values in these scenarios that we may use in
 // other scenarios in which the skeleton UI is actually enabled.
 static bool PreXULSkeletonUIDisallowed() {
-  return sDisabledReason.isSome() &&
-         (*sDisabledReason == PreXULSkeletonUIError::Cmdline ||
-          *sDisabledReason == PreXULSkeletonUIError::EnvVars);
+  return sErrorReason.isSome() &&
+         (*sErrorReason == PreXULSkeletonUIError::Cmdline ||
+          *sErrorReason == PreXULSkeletonUIError::EnvVars);
 }
 
 static UniquePtr<wchar_t, LoadedCoTaskMemFreeDeleter> GetKnownFolderPath(
@@ -2072,13 +2072,13 @@ void CreateAndStorePreXULSkeletonUI(HINSTANCE hInstance, int argc,
   auto result = CreateAndStorePreXULSkeletonUIImpl(hInstance, argc, argv);
 
   if (result.isErr()) {
-    sDisabledReason.emplace(result.unwrapErr());
+    sErrorReason.emplace(result.unwrapErr());
   }
 }
 
 bool WasPreXULSkeletonUIMaximized() { return sMaximized; }
 
-Result<HWND, PreXULSkeletonUIError> ConsumePreXULSkeletonUIHandle() {
+HWND ConsumePreXULSkeletonUIHandle() {
   // NOTE: we need to make sure that everything that runs here is a no-op if
   // it failed to be set, which is a possibility. If anything fails to be set
   // we don't want to clean everything up right away, because if we have a
@@ -2102,18 +2102,13 @@ Result<HWND, PreXULSkeletonUIError> ConsumePreXULSkeletonUIHandle() {
   delete sAnimatedRects;
   sAnimatedRects = nullptr;
 
-  // Regardless of whether we're disabled or not, we want to do the cleanup
-  // above. It should generally be a no-op if we've been disabled, but it's
-  // possible it wasn't.
-  if (sDisabledReason.isSome()) {
-    return Err(*sDisabledReason);
-  }
-  if (!result) {
-    return Err(PreXULSkeletonUIError::Unknown);
-  }
   return result;
 }
 
+Maybe<PreXULSkeletonUIError> GetPreXULSkeletonUIErrorReason() {
+  return sErrorReason;
+}
+
 Result<Ok, PreXULSkeletonUIError> PersistPreXULSkeletonUIValues(
     const SkeletonUISettings& settings) {
   if (!sPreXULSkeletonUIEnabled) {

mozglue/misc/PreXULSkeletonUI.h

@@ -9,6 +9,7 @@
 
 #include <windows.h>
 #include "mozilla/EnumSet.h"
+#include "mozilla/Maybe.h"
 #include "mozilla/Result.h"
 #include "mozilla/Types.h"
 #include "mozilla/Vector.h"
@@ -156,7 +157,8 @@ enum class PreXULSkeletonUIProgress : uint32_t {
 
 MFBT_API void CreateAndStorePreXULSkeletonUI(HINSTANCE hInstance, int argc,
                                              char** argv);
-MFBT_API Result<HWND, PreXULSkeletonUIError> ConsumePreXULSkeletonUIHandle();
+MFBT_API HWND ConsumePreXULSkeletonUIHandle();
+MFBT_API Maybe<PreXULSkeletonUIError> GetPreXULSkeletonUIErrorReason();
 MFBT_API bool WasPreXULSkeletonUIMaximized();
 MFBT_API Result<Ok, PreXULSkeletonUIError> PersistPreXULSkeletonUIValues(
     const SkeletonUISettings& settings);

widget/windows/nsWindow.cpp

@@ -897,15 +897,16 @@ nsresult nsWindow::Create(nsIWidget* aParent, nsNativeWidget aNativeParent,
   if (aInitData->mWindowType == eWindowType_toplevel && !aParent &&
       !sFirstTopLevelWindowCreated) {
     sFirstTopLevelWindowCreated = true;
-    auto skeletonUIResult = ConsumePreXULSkeletonUIHandle();
-    if (skeletonUIResult.isErr()) {
+    mWnd = ConsumePreXULSkeletonUIHandle();
+    auto skeletonUIError = GetPreXULSkeletonUIErrorReason();
+    if (skeletonUIError) {
       nsAutoString errorString(
-          GetPreXULSkeletonUIErrorString(skeletonUIResult.unwrapErr()));
+          GetPreXULSkeletonUIErrorString(skeletonUIError.value()));
       Telemetry::ScalarSet(
           Telemetry::ScalarID::STARTUP_SKELETON_UI_DISABLED_REASON,
           errorString);
-    } else {
-      mWnd = skeletonUIResult.unwrap();
+    }
+    if (mWnd) {
       MOZ_ASSERT(style == kPreXULSkeletonUIWindowStyle,
                  "The skeleton UI window style should match the expected "
                  "style for the first window created");