Bug 1754813 - Make Hal::ScreenOrientation an enum class. r=gsvelto,m_kato,geckoview-reviewers

This adds proper IPC validation too.

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

docshell/base/BrowsingContext.cpp

@@ -400,7 +400,7 @@ already_AddRefed<BrowsingContext> BrowsingContext::CreateDetached(
   fields.mDefaultLoadFlags =
       inherit ? inherit->GetDefaultLoadFlags() : nsIRequest::LOAD_NORMAL;
 
-  fields.mOrientationLock = mozilla::hal::eScreenOrientation_None;
+  fields.mOrientationLock = mozilla::hal::ScreenOrientation::None;
 
   fields.mUseGlobalHistory = inherit ? inherit->GetUseGlobalHistory() : false;
 

docshell/base/SyncedContextInlines.h

@@ -12,6 +12,7 @@
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/ContentChild.h"
 #include "nsReadableUtils.h"
+#include "mozilla/HalIPCUtils.h"
 
 namespace mozilla {
 namespace dom {

docshell/base/nsDocShell.cpp

@@ -4886,7 +4886,7 @@ void nsDocShell::ActivenessMaybeChanged() {
       // Update orientation when the top-level browsing context becomes active.
       if (isActive && mBrowsingContext->IsTop()) {
         // We only care about the top-level browsing context.
-        uint16_t orientation = mBrowsingContext->GetOrientationLock();
+        auto orientation = mBrowsingContext->GetOrientationLock();
         ScreenOrientation::UpdateActiveOrientationLock(orientation);
       }
 
@@ -9459,13 +9459,13 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   // lock the orientation of the document to the document's default
   // orientation. We don't explicitly check for a top-level browsing context
   // here because orientation is only set on top-level browsing contexts.
-  if (mBrowsingContext->GetOrientationLock() != hal::eScreenOrientation_None) {
+  if (mBrowsingContext->GetOrientationLock() != hal::ScreenOrientation::None) {
     MOZ_ASSERT(mBrowsingContext->IsTop());
     MOZ_ALWAYS_SUCCEEDS(
-        mBrowsingContext->SetOrientationLock(hal::eScreenOrientation_None));
+        mBrowsingContext->SetOrientationLock(hal::ScreenOrientation::None));
     if (mBrowsingContext->IsActive()) {
       ScreenOrientation::UpdateActiveOrientationLock(
-          hal::eScreenOrientation_None);
+          hal::ScreenOrientation::None);
     }
   }
 

dom/base/ScreenOrientation.cpp

@@ -36,13 +36,13 @@ NS_IMPL_RELEASE_INHERITED(ScreenOrientation, DOMEventTargetHelper)
 static OrientationType InternalOrientationToType(
     hal::ScreenOrientation aOrientation) {
   switch (aOrientation) {
-    case hal::eScreenOrientation_PortraitPrimary:
+    case hal::ScreenOrientation::PortraitPrimary:
       return OrientationType::Portrait_primary;
-    case hal::eScreenOrientation_PortraitSecondary:
+    case hal::ScreenOrientation::PortraitSecondary:
       return OrientationType::Portrait_secondary;
-    case hal::eScreenOrientation_LandscapePrimary:
+    case hal::ScreenOrientation::LandscapePrimary:
       return OrientationType::Landscape_primary;
-    case hal::eScreenOrientation_LandscapeSecondary:
+    case hal::ScreenOrientation::LandscapeSecondary:
       return OrientationType::Landscape_secondary;
     default:
       MOZ_CRASH("Bad aOrientation value");
@@ -53,13 +53,13 @@ static hal::ScreenOrientation OrientationTypeToInternal(
     OrientationType aOrientation) {
   switch (aOrientation) {
     case OrientationType::Portrait_primary:
-      return hal::eScreenOrientation_PortraitPrimary;
+      return hal::ScreenOrientation::PortraitPrimary;
     case OrientationType::Portrait_secondary:
-      return hal::eScreenOrientation_PortraitSecondary;
+      return hal::ScreenOrientation::PortraitSecondary;
     case OrientationType::Landscape_primary:
-      return hal::eScreenOrientation_LandscapePrimary;
+      return hal::ScreenOrientation::LandscapePrimary;
     case OrientationType::Landscape_secondary:
-      return hal::eScreenOrientation_LandscapeSecondary;
+      return hal::ScreenOrientation::LandscapeSecondary;
     default:
       MOZ_CRASH("Bad aOrientation value");
   }
@@ -154,7 +154,7 @@ ScreenOrientation::LockOrientationTask::~LockOrientationTask() = default;
 
 bool ScreenOrientation::LockOrientationTask::OrientationLockContains(
     OrientationType aOrientationType) {
-  return mOrientationLock & OrientationTypeToInternal(aOrientationType);
+  return bool(mOrientationLock & OrientationTypeToInternal(aOrientationType));
 }
 
 NS_IMETHODIMP
@@ -179,7 +179,7 @@ ScreenOrientation::LockOrientationTask::Run() {
     return NS_OK;
   }
 
-  if (mOrientationLock == hal::eScreenOrientation_None) {
+  if (mOrientationLock == hal::ScreenOrientation::None) {
     mScreenOrientation->UnlockDeviceOrientation();
     mPromise->MaybeResolveWithUndefined();
     mDocument->ClearOrientationPendingPromise();
@@ -211,7 +211,7 @@ ScreenOrientation::LockOrientationTask::Run() {
 
   BrowsingContext* bc = mDocument->GetBrowsingContext();
   if (OrientationLockContains(bc->GetCurrentOrientationType()) ||
-      (mOrientationLock == hal::eScreenOrientation_Default &&
+      (mOrientationLock == hal::ScreenOrientation::Default &&
        bc->GetCurrentOrientationAngle() == 0)) {
     // Orientation lock will not cause an orientation change.
     mPromise->MaybeResolveWithUndefined();
@@ -223,37 +223,37 @@ ScreenOrientation::LockOrientationTask::Run() {
 
 already_AddRefed<Promise> ScreenOrientation::Lock(
     OrientationLockType aOrientation, ErrorResult& aRv) {
-  hal::ScreenOrientation orientation = hal::eScreenOrientation_None;
+  hal::ScreenOrientation orientation = hal::ScreenOrientation::None;
 
   switch (aOrientation) {
     case OrientationLockType::Any:
-      orientation = hal::eScreenOrientation_PortraitPrimary |
+      orientation = hal::ScreenOrientation::PortraitPrimary |
-                    hal::eScreenOrientation_PortraitSecondary |
+                    hal::ScreenOrientation::PortraitSecondary |
-                    hal::eScreenOrientation_LandscapePrimary |
+                    hal::ScreenOrientation::LandscapePrimary |
-                    hal::eScreenOrientation_LandscapeSecondary;
+                    hal::ScreenOrientation::LandscapeSecondary;
       break;
     case OrientationLockType::Natural:
-      orientation |= hal::eScreenOrientation_Default;
+      orientation |= hal::ScreenOrientation::Default;
       break;
     case OrientationLockType::Landscape:
-      orientation = hal::eScreenOrientation_LandscapePrimary |
+      orientation = hal::ScreenOrientation::LandscapePrimary |
-                    hal::eScreenOrientation_LandscapeSecondary;
+                    hal::ScreenOrientation::LandscapeSecondary;
       break;
     case OrientationLockType::Portrait:
-      orientation = hal::eScreenOrientation_PortraitPrimary |
+      orientation = hal::ScreenOrientation::PortraitPrimary |
-                    hal::eScreenOrientation_PortraitSecondary;
+                    hal::ScreenOrientation::PortraitSecondary;
       break;
     case OrientationLockType::Portrait_primary:
-      orientation = hal::eScreenOrientation_PortraitPrimary;
+      orientation = hal::ScreenOrientation::PortraitPrimary;
       break;
     case OrientationLockType::Portrait_secondary:
-      orientation = hal::eScreenOrientation_PortraitSecondary;
+      orientation = hal::ScreenOrientation::PortraitSecondary;
       break;
     case OrientationLockType::Landscape_primary:
-      orientation = hal::eScreenOrientation_LandscapePrimary;
+      orientation = hal::ScreenOrientation::LandscapePrimary;
       break;
     case OrientationLockType::Landscape_secondary:
-      orientation = hal::eScreenOrientation_LandscapeSecondary;
+      orientation = hal::ScreenOrientation::LandscapeSecondary;
       break;
     default:
       NS_WARNING("Unexpected orientation type");
@@ -401,7 +401,7 @@ RefPtr<MozPromise<bool, bool, false>> ScreenOrientation::LockDeviceOrientation(
 }
 
 void ScreenOrientation::Unlock(ErrorResult& aRv) {
-  RefPtr<Promise> p = LockInternal(hal::eScreenOrientation_None, aRv);
+  RefPtr<Promise> p = LockInternal(hal::ScreenOrientation::None, aRv);
 }
 
 void ScreenOrientation::UnlockDeviceOrientation() {
@@ -517,10 +517,10 @@ void ScreenOrientation::Notify(const hal::ScreenConfiguration& aConfiguration) {
   }
 
   hal::ScreenOrientation orientation = aConfiguration.orientation();
-  if (orientation != hal::eScreenOrientation_PortraitPrimary &&
+  if (orientation != hal::ScreenOrientation::PortraitPrimary &&
-      orientation != hal::eScreenOrientation_PortraitSecondary &&
+      orientation != hal::ScreenOrientation::PortraitSecondary &&
-      orientation != hal::eScreenOrientation_LandscapePrimary &&
+      orientation != hal::ScreenOrientation::LandscapePrimary &&
-      orientation != hal::eScreenOrientation_LandscapeSecondary) {
+      orientation != hal::ScreenOrientation::LandscapeSecondary) {
     // The platform may notify of some other values from
     // an orientation lock, but we only care about real
     // changes to screen orientation which result in one of
@@ -559,7 +559,7 @@ void ScreenOrientation::Notify(const hal::ScreenConfiguration& aConfiguration) {
 
 void ScreenOrientation::UpdateActiveOrientationLock(
     hal::ScreenOrientation aOrientation) {
-  if (aOrientation == hal::eScreenOrientation_None) {
+  if (aOrientation == hal::ScreenOrientation::None) {
     hal::UnlockScreenOrientation();
   } else {
     hal::LockScreenOrientation(aOrientation)

gfx/layers/ipc/LayersMessages.ipdlh

@@ -28,7 +28,7 @@ using mozilla::TimeDuration from "mozilla/TimeStamp.h";
 using class mozilla::TimeStamp from "mozilla/TimeStamp.h";
 using mozilla::ScreenRotation from "mozilla/WidgetUtils.h";
 using nsCSSPropertyID from "nsCSSPropertyID.h";
-using hal::ScreenOrientation from "mozilla/HalScreenConfiguration.h";
+using hal::ScreenOrientation from "mozilla/HalIPCUtils.h";
 using struct mozilla::layers::TextureInfo from "mozilla/layers/CompositorTypes.h";
 using mozilla::CSSPoint from "Units.h";
 using mozilla::CSSRect from "Units.h";

hal/HalIPCUtils.h

@@ -0,0 +1,23 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set sw=2 ts=8 et ft=cpp : */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_HalIPCUtils_h
+#define mozilla_HalIPCUtils_h
+
+#include "HalScreenConfiguration.h"
+
+#include "ipc/EnumSerializer.h"
+
+namespace IPC {
+
+template <>
+struct ParamTraits<mozilla::hal::ScreenOrientation>
+    : public BitFlagsEnumSerializer<mozilla::hal::ScreenOrientation,
+                                    mozilla::hal::kAllScreenOrientationBits> {};
+
+}  // namespace IPC
+
+#endif

hal/HalScreenConfiguration.h

@@ -8,27 +8,30 @@
 #define mozilla_HalScreenConfiguration_h
 
 #include "mozilla/Observer.h"
+#include "mozilla/TypedEnumBits.h"
 
-namespace mozilla {
+namespace mozilla::hal {
-namespace hal {
 
 // Make sure that any change to ScreenOrientation values are also made in
 // mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoScreenOrientation.java
-typedef uint32_t ScreenOrientation;
+enum class ScreenOrientation : uint32_t {
+  None = 0,
+  PortraitPrimary = 1u << 0,
+  PortraitSecondary = 1u << 1,
+  LandscapePrimary = 1u << 2,
+  LandscapeSecondary = 1u << 3,
+  // Default will use the natural orientation for the device, it could be
+  // PortraitPrimary or LandscapePrimary depends on display resolution
+  Default = 1u << 4,
+};
 
-static const ScreenOrientation eScreenOrientation_None = 0;
+constexpr auto kAllScreenOrientationBits = ScreenOrientation((1 << 5) - 1);
-static const ScreenOrientation eScreenOrientation_PortraitPrimary = 1u << 0;
+
-static const ScreenOrientation eScreenOrientation_PortraitSecondary = 1u << 1;
+MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ScreenOrientation);
-static const ScreenOrientation eScreenOrientation_LandscapePrimary = 1u << 2;
-static const ScreenOrientation eScreenOrientation_LandscapeSecondary = 1u << 3;
-// eScreenOrientation_Default will use the natural orientation for the deivce,
-// it could be PortraitPrimary or LandscapePrimary depends on display resolution
-static const ScreenOrientation eScreenOrientation_Default = 1u << 4;
 
 class ScreenConfiguration;
-typedef Observer<ScreenConfiguration> ScreenConfigurationObserver;
+using ScreenConfigurationObserver = Observer<ScreenConfiguration>;
 
-}  // namespace hal
+}  // namespace mozilla::hal
-}  // namespace mozilla
 
 #endif  // mozilla_HalScreenConfiguration_h

hal/android/AndroidHal.cpp

@@ -16,10 +16,7 @@
 using namespace mozilla::dom;
 using namespace mozilla::hal;
 
-namespace java = mozilla::java;
+namespace mozilla::hal_impl {
-
-namespace mozilla {
-namespace hal_impl {
 
 void Vibrate(const nsTArray<uint32_t>& pattern, WindowIdentifier&&) {
   // Ignore the WindowIdentifier parameter; it's here only because hal::Vibrate,
@@ -102,47 +99,56 @@ void GetCurrentScreenConfiguration(ScreenConfiguration* aScreenConfiguration) {
   aScreenConfiguration->angle() = bridge->GetScreenAngle();
 }
 
+static bool IsSupportedScreenOrientation(hal::ScreenOrientation aOrientation) {
+  // The Android backend only supports these orientations.
+  static constexpr ScreenOrientation kSupportedOrientations[] = {
+      ScreenOrientation::PortraitPrimary,
+      ScreenOrientation::PortraitSecondary,
+      ScreenOrientation::PortraitPrimary | ScreenOrientation::PortraitSecondary,
+      ScreenOrientation::LandscapePrimary,
+      ScreenOrientation::LandscapeSecondary,
+      ScreenOrientation::LandscapePrimary |
+          ScreenOrientation::LandscapeSecondary,
+      ScreenOrientation::PortraitPrimary |
+          ScreenOrientation::PortraitSecondary |
+          ScreenOrientation::LandscapePrimary |
+          ScreenOrientation::LandscapeSecondary,
+      ScreenOrientation::Default,
+  };
+  for (auto supportedOrientation : kSupportedOrientations) {
+    if (aOrientation == supportedOrientation) {
+      return true;
+    }
+  }
+  return false;
+}
+
 RefPtr<MozPromise<bool, bool, false>> LockScreenOrientation(
     const hal::ScreenOrientation& aOrientation) {
-  switch (aOrientation) {
+  using LockPromise = MozPromise<bool, bool, false>;
-    // The Android backend only supports these orientations.
+
-    case eScreenOrientation_PortraitPrimary:
+  if (!IsSupportedScreenOrientation(aOrientation)) {
-    case eScreenOrientation_PortraitSecondary:
-    case eScreenOrientation_PortraitPrimary |
-        eScreenOrientation_PortraitSecondary:
-    case eScreenOrientation_LandscapePrimary:
-    case eScreenOrientation_LandscapeSecondary:
-    case eScreenOrientation_LandscapePrimary |
-        eScreenOrientation_LandscapeSecondary:
-    case eScreenOrientation_PortraitPrimary |
-        eScreenOrientation_PortraitSecondary |
-        eScreenOrientation_LandscapePrimary |
-        eScreenOrientation_LandscapeSecondary:
-    case eScreenOrientation_Default: {
-      java::GeckoRuntime::LocalRef runtime = java::GeckoRuntime::GetInstance();
-      if (runtime != NULL) {
-        auto result = runtime->LockScreenOrientation(aOrientation);
-        auto geckoResult = java::GeckoResult::LocalRef(std::move(result));
-        return geckoResult
-                   ? MozPromise<bool, bool, false>::FromGeckoResult(geckoResult)
-                   : MozPromise<bool, bool, false>::CreateAndReject(false,
-                                                                    __func__);
-      } else {
-        return MozPromise<bool, bool, false>::CreateAndReject(false, __func__);
-      }
-    }
-    default:
     NS_WARNING("Unsupported screen orientation type");
-      return MozPromise<bool, bool, false>::CreateAndReject(false, __func__);
+    return LockPromise::CreateAndReject(false, __func__);
   }
+
+  java::GeckoRuntime::LocalRef runtime = java::GeckoRuntime::GetInstance();
+  if (!runtime) {
+    return LockPromise::CreateAndReject(false, __func__);
+  }
+  auto result = runtime->LockScreenOrientation(uint32_t(aOrientation));
+  auto geckoResult = java::GeckoResult::LocalRef(std::move(result));
+  if (!geckoResult) {
+    return LockPromise::CreateAndReject(false, __func__);
+  }
+  return LockPromise::FromGeckoResult(geckoResult);
 }
 
 void UnlockScreenOrientation() {
   java::GeckoRuntime::LocalRef runtime = java::GeckoRuntime::GetInstance();
-  if (runtime != NULL) {
+  if (runtime) {
     runtime->UnlockScreenOrientation();
   }
 }
 
-}  // namespace hal_impl
+}  // namespace mozilla::hal_impl
-}  // namespace mozilla

hal/fallback/FallbackScreenConfiguration.h

@@ -20,8 +20,8 @@ inline void GetCurrentScreenConfiguration(
   aScreenConfiguration->orientation() =
       aScreenConfiguration->rect().Width() >=
               aScreenConfiguration->rect().Height()
-          ? hal::eScreenOrientation_LandscapePrimary
+          ? hal::ScreenOrientation::LandscapePrimary
-          : hal::eScreenOrientation_PortraitPrimary;
+          : hal::ScreenOrientation::PortraitPrimary;
 }
 
 }  // namespace fallback

hal/moz.build

@@ -11,6 +11,7 @@ EXPORTS.mozilla += [
     "Hal.h",
     "HalBatteryInformation.h",
     "HalImpl.h",
+    "HalIPCUtils.h",
     "HalNetworkInformation.h",
     "HalSandbox.h",
     "HalScreenConfiguration.h",

hal/sandbox/PHal.ipdl

@@ -10,7 +10,7 @@ include protocol PBrowser;
 include "mozilla/dom/ReferrerInfoUtils.h";
 include "mozilla/GfxMessageUtils.h";
 
-using hal::ScreenOrientation from "mozilla/HalScreenConfiguration.h";
+using hal::ScreenOrientation from "mozilla/HalIPCUtils.h";
 using mozilla::hal::SensorType from "mozilla/HalSensor.h";
 using mozilla::hal::WakeLockControl from "mozilla/HalTypes.h";
 using mozilla::hal::ProcessPriority from "mozilla/HalTypes.h";

widget/Screen.cpp

@@ -60,7 +60,7 @@ mozilla::dom::ScreenDetails Screen::ToScreenDetails() {
 mozilla::hal::ScreenConfiguration Screen::ToScreenConfiguration() {
   return mozilla::hal::ScreenConfiguration(
       nsIntRect(mRect.x, mRect.y, mRect.width, mRect.height),
-      hal::eScreenOrientation_None, 0, mColorDepth, mPixelDepth);
+      hal::ScreenOrientation::None, 0, mColorDepth, mPixelDepth);
 }
 
 NS_IMETHODIMP

widget/android/AndroidBridge.cpp

@@ -427,12 +427,12 @@ nsAndroidBridge::GetDispatcherByName(const char* aName,
 
 nsAndroidBridge::~nsAndroidBridge() {}
 
-uint32_t AndroidBridge::GetScreenOrientation() {
+hal::ScreenOrientation AndroidBridge::GetScreenOrientation() {
   ALOG_BRIDGE("AndroidBridge::GetScreenOrientation");
 
   int16_t orientation = java::GeckoAppShell::GetScreenOrientation();
 
-  return static_cast<hal::ScreenOrientation>(orientation);
+  return hal::ScreenOrientation(orientation);
 }
 
 uint16_t AndroidBridge::GetScreenAngle() {

widget/android/AndroidBridge.h

@@ -31,6 +31,7 @@ class AutoLocalJNIFrame;
 namespace hal {
 class BatteryInformation;
 class NetworkInformation;
+enum class ScreenOrientation : uint32_t;
 }  // namespace hal
 
 class AndroidBridge final {
@@ -79,11 +80,7 @@ class AndroidBridge final {
 
   void GetCurrentNetworkInformation(hal::NetworkInformation* aNetworkInfo);
 
-  // These methods don't use a ScreenOrientation because it's an
+  hal::ScreenOrientation GetScreenOrientation();
-  // enum and that would require including the header which requires
-  // include IPC headers which requires including basictypes.h which
-  // requires a lot of changes...
-  uint32_t GetScreenOrientation();
   uint16_t GetScreenAngle();
 
   nsresult GetProxyForURI(const nsACString& aSpec, const nsACString& aScheme,