Bug 1858977 - Don't throw on screen getters. r=smaug,webidl

The screen lifetime seems like it was a lot more complicated
in the past (looking at the comments in Window.webidl).

But nowadays it seems it can be more similar to the other
objects like VisualViewport etc.

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

dom/base/ScreenOrientation.cpp

@@ -825,14 +825,7 @@ ScreenOrientation::VisibleEventListener::HandleEvent(Event* aEvent) {
     return NS_OK;
   }
 
-  ErrorResult rv;
-  nsScreen* screen = win->GetScreen(rv);
-  if (NS_WARN_IF(rv.Failed())) {
-    return rv.StealNSResult();
-  }
-
-  MOZ_ASSERT(screen);
-  ScreenOrientation* orientation = screen->Orientation();
+  ScreenOrientation* orientation = win->Screen()->Orientation();
   MOZ_ASSERT(orientation);
 
   doc->RemoveSystemEventListener(u"visibilitychange"_ns, this, true);
@@ -847,12 +840,8 @@ ScreenOrientation::VisibleEventListener::HandleEvent(Event* aEvent) {
 
     nsCOMPtr<nsIRunnable> runnable =
         orientation->DispatchChangeEventAndResolvePromise();
-    rv = NS_DispatchToMainThread(runnable);
-    if (NS_WARN_IF(rv.Failed())) {
-      return rv.StealNSResult();
-    }
+    MOZ_TRY(NS_DispatchToMainThread(runnable));
   }
-
   return NS_OK;
 }
 

dom/base/nsGlobalWindowInner.cpp

@@ -2400,19 +2400,13 @@ VisualViewport* nsGlobalWindowInner::VisualViewport() {
   if (!mVisualViewport) {
     mVisualViewport = new mozilla::dom::VisualViewport(this);
   }
-
   return mVisualViewport;
 }
 
-nsScreen* nsGlobalWindowInner::GetScreen(ErrorResult& aError) {
+nsScreen* nsGlobalWindowInner::Screen() {
   if (!mScreen) {
-    mScreen = nsScreen::Create(this);
-    if (!mScreen) {
-      aError.Throw(NS_ERROR_UNEXPECTED);
-      return nullptr;
-    }
+    mScreen = new nsScreen(this);
   }
-
   return mScreen;
 }
 
@@ -2420,7 +2414,6 @@ nsHistory* nsGlobalWindowInner::GetHistory(ErrorResult& aError) {
   if (!mHistory) {
     mHistory = new nsHistory(this);
   }
-
   return mHistory;
 }
 
@@ -7278,11 +7271,7 @@ int16_t nsGlobalWindowInner::Orientation(CallerType aCallerType) {
           aCallerType, RFPTarget::ScreenOrientation)) {
     return 0;
   }
-  nsScreen* s = GetScreen(IgnoreErrors());
-  if (!s) {
-    return 0;
-  }
-  int16_t angle = AssertedCast<int16_t>(s->GetOrientationAngle());
+  int16_t angle = AssertedCast<int16_t>(Screen()->GetOrientationAngle());
   return angle <= 180 ? angle : angle - 360;
 }
 

dom/base/nsGlobalWindowInner.h

@@ -736,7 +736,7 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
   already_AddRefed<mozilla::dom::MediaQueryList> MatchMedia(
       const nsACString& aQuery, mozilla::dom::CallerType aCallerType,
       mozilla::ErrorResult& aError);
-  nsScreen* GetScreen(mozilla::ErrorResult& aError);
+  nsScreen* Screen();
   void MoveTo(int32_t aXPos, int32_t aYPos,
               mozilla::dom::CallerType aCallerType,
               mozilla::ErrorResult& aError);

dom/base/nsGlobalWindowOuter.cpp

@@ -3037,7 +3037,7 @@ Navigator* nsGlobalWindowOuter::GetNavigator() {
 }
 
 nsScreen* nsGlobalWindowOuter::GetScreen() {
-  FORWARD_TO_INNER(GetScreen, (IgnoreErrors()), nullptr);
+  FORWARD_TO_INNER(Screen, (), nullptr);
 }
 
 void nsPIDOMWindowOuter::ActivateMediaComponents() {
@@ -3763,9 +3763,9 @@ void nsGlobalWindowOuter::CheckSecurityLeftAndTop(int32_t* aLeft, int32_t* aTop,
 
       // Get the screen dimensions
       // XXX This should use nsIScreenManager once it's fully fleshed out.
-      int32_t screenLeft = screen->GetAvailLeft(IgnoreErrors());
-      int32_t screenWidth = screen->GetAvailWidth(IgnoreErrors());
-      int32_t screenHeight = screen->GetAvailHeight(IgnoreErrors());
+      int32_t screenLeft = screen->AvailLeft();
+      int32_t screenWidth = screen->AvailWidth();
+      int32_t screenHeight = screen->AvailHeight();
 #if defined(XP_MACOSX)
       /* The mac's coordinate system is different from the assumed Windows'
          system. It offsets by the height of the menubar so that a window
@@ -3774,9 +3774,9 @@ void nsGlobalWindowOuter::CheckSecurityLeftAndTop(int32_t* aLeft, int32_t* aTop,
          the Avail... coordinates is overloaded. Here we allow a window
          to be placed at (0,0) because it does make sense to do so.
       */
-      int32_t screenTop = screen->GetTop(IgnoreErrors());
+      int32_t screenTop = screen->Top();
 #else
-      int32_t screenTop = screen->GetAvailTop(IgnoreErrors());
+      int32_t screenTop = screen->AvailTop();
 #endif
 
       if (aLeft) {

dom/base/nsScreen.cpp

@@ -14,28 +14,12 @@
 #include "nsCOMPtr.h"
 #include "nsIDocShellTreeItem.h"
 #include "nsLayoutUtils.h"
-#include "nsJSUtils.h"
 #include "nsDeviceContext.h"
 #include "mozilla/widget/ScreenManager.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
-/* static */
-already_AddRefed<nsScreen> nsScreen::Create(nsPIDOMWindowInner* aWindow) {
-  MOZ_ASSERT(aWindow);
-
-  if (!aWindow->GetDocShell()) {
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIScriptGlobalObject> sgo = do_QueryInterface(aWindow);
-  NS_ENSURE_TRUE(sgo, nullptr);
-
-  RefPtr<nsScreen> screen = new nsScreen(aWindow);
-  return screen.forget();
-}
-
 nsScreen::nsScreen(nsPIDOMWindowInner* aWindow)
     : DOMEventTargetHelper(aWindow),
       mScreenOrientation(new ScreenOrientation(aWindow, this)) {}
@@ -52,19 +36,15 @@ NS_IMPL_RELEASE_INHERITED(nsScreen, DOMEventTargetHelper)
 NS_IMPL_CYCLE_COLLECTION_INHERITED(nsScreen, DOMEventTargetHelper,
                                    mScreenOrientation)
 
-int32_t nsScreen::GetPixelDepth(ErrorResult& aRv) {
+int32_t nsScreen::PixelDepth() {
   // Return 24 to prevent fingerprinting.
   if (ShouldResistFingerprinting(RFPTarget::ScreenPixelDepth)) {
     return 24;
   }
-
   nsDeviceContext* context = GetDeviceContext();
-
-  if (!context) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return -1;
+  if (NS_WARN_IF(!context)) {
+    return 24;
   }
-
   return context->GetDepth();
 }
 
@@ -72,7 +52,6 @@ nsPIDOMWindowOuter* nsScreen::GetOuter() const {
   if (nsPIDOMWindowInner* inner = GetOwner()) {
     return inner->GetOuterWindow();
   }
-
   return nullptr;
 }
 
@@ -80,67 +59,62 @@ nsDeviceContext* nsScreen::GetDeviceContext() const {
   return nsLayoutUtils::GetDeviceContextForScreenInfo(GetOuter());
 }
 
-nsresult nsScreen::GetRect(CSSIntRect& aRect) {
+CSSIntRect nsScreen::GetRect() {
   // Return window inner rect to prevent fingerprinting.
   if (ShouldResistFingerprinting(RFPTarget::ScreenRect)) {
-    return GetWindowInnerRect(aRect);
+    return GetWindowInnerRect();
   }
 
   // Here we manipulate the value of aRect to represent the screen size,
   // if in RDM.
-  if (nsCOMPtr<nsPIDOMWindowInner> owner = GetOwner()) {
+  if (nsPIDOMWindowInner* owner = GetOwner()) {
     if (Document* doc = owner->GetExtantDoc()) {
       Maybe<CSSIntSize> deviceSize =
           nsGlobalWindowOuter::GetRDMDeviceSize(*doc);
       if (deviceSize.isSome()) {
         const CSSIntSize& size = deviceSize.value();
-        aRect.SetRect(0, 0, size.width, size.height);
-        return NS_OK;
+        return {0, 0, size.width, size.height};
       }
     }
   }
 
   nsDeviceContext* context = GetDeviceContext();
-
-  if (!context) {
-    return NS_ERROR_FAILURE;
+  if (NS_WARN_IF(!context)) {
+    return {};
   }
 
   nsRect r;
   context->GetRect(r);
-  aRect = CSSIntRect::FromAppUnitsRounded(r);
-  return NS_OK;
+  return CSSIntRect::FromAppUnitsRounded(r);
 }
 
-nsresult nsScreen::GetAvailRect(CSSIntRect& aRect) {
+CSSIntRect nsScreen::GetAvailRect() {
   // Return window inner rect to prevent fingerprinting.
   if (ShouldResistFingerprinting(RFPTarget::ScreenAvailRect)) {
-    return GetWindowInnerRect(aRect);
+    return GetWindowInnerRect();
   }
 
   // Here we manipulate the value of aRect to represent the screen size,
   // if in RDM.
-  if (nsCOMPtr<nsPIDOMWindowInner> owner = GetOwner()) {
+  if (nsPIDOMWindowInner* owner = GetOwner()) {
     if (Document* doc = owner->GetExtantDoc()) {
       Maybe<CSSIntSize> deviceSize =
           nsGlobalWindowOuter::GetRDMDeviceSize(*doc);
       if (deviceSize.isSome()) {
         const CSSIntSize& size = deviceSize.value();
-        aRect.SetRect(0, 0, size.width, size.height);
-        return NS_OK;
+        return {0, 0, size.width, size.height};
       }
     }
   }
 
   nsDeviceContext* context = GetDeviceContext();
-  if (!context) {
-    return NS_ERROR_FAILURE;
+  if (NS_WARN_IF(!context)) {
+    return {};
   }
 
   nsRect r;
   context->GetClientRect(r);
-  aRect = CSSIntRect::FromAppUnitsRounded(r);
-  return NS_OK;
+  return CSSIntRect::FromAppUnitsRounded(r);
 }
 
 uint16_t nsScreen::GetOrientationAngle() const {
@@ -185,46 +159,24 @@ void nsScreen::GetMozOrientation(nsString& aOrientation,
   }
 }
 
-bool nsScreen::MozLockOrientation(const nsAString& aOrientation,
-                                  ErrorResult& aRv) {
-  nsString orientation(aOrientation);
-  Sequence<nsString> orientations;
-  if (!orientations.AppendElement(orientation, fallible)) {
-    aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
-    return false;
-  }
-  return MozLockOrientation(orientations, aRv);
-}
-
-// This function is deprecated, use ScreenOrientation API instead.
-bool nsScreen::MozLockOrientation(const Sequence<nsString>& aOrientations,
-                                  ErrorResult& aRv) {
-  return false;
-}
-
-void nsScreen::MozUnlockOrientation() {}
-
 /* virtual */
 JSObject* nsScreen::WrapObject(JSContext* aCx,
                                JS::Handle<JSObject*> aGivenProto) {
   return Screen_Binding::Wrap(aCx, this, aGivenProto);
 }
 
-nsresult nsScreen::GetWindowInnerRect(CSSIntRect& aRect) {
-  aRect.x = 0;
-  aRect.y = 0;
+CSSIntRect nsScreen::GetWindowInnerRect() {
   nsCOMPtr<nsPIDOMWindowInner> win = GetOwner();
   if (!win) {
-    return NS_ERROR_FAILURE;
+    return {};
   }
   double width;
   double height;
-  nsresult rv = win->GetInnerWidth(&width);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = win->GetInnerHeight(&height);
-  NS_ENSURE_SUCCESS(rv, rv);
-  aRect.SizeTo(std::round(width), std::round(height));
-  return NS_OK;
+  if (NS_FAILED(win->GetInnerWidth(&width)) ||
+      NS_FAILED(win->GetInnerHeight(&height))) {
+    return {};
+  }
+  return {0, 0, int32_t(std::round(width)), int32_t(std::round(height))};
 }
 
 bool nsScreen::ShouldResistFingerprinting(RFPTarget aTarget) const {

dom/base/nsScreen.h

@@ -6,15 +6,11 @@
 #ifndef nsScreen_h___
 #define nsScreen_h___
 
-#include "mozilla/Attributes.h"
 #include "mozilla/dom/ScreenBinding.h"
 #include "mozilla/dom/ScreenLuminance.h"
 #include "mozilla/dom/ScreenOrientation.h"
 #include "mozilla/DOMEventTargetHelper.h"
-#include "mozilla/ErrorResult.h"
 #include "mozilla/StaticPrefs_media.h"
-#include "nsCOMPtr.h"
-#include "mozilla/gfx/Rect.h"
 #include "Units.h"
 
 class nsDeviceContext;
@@ -25,10 +21,8 @@ enum class RFPTarget : uint64_t;
 
 // Script "screen" object
 class nsScreen : public mozilla::DOMEventTargetHelper {
-  using ErrorResult = mozilla::ErrorResult;
-
  public:
-  static already_AddRefed<nsScreen> Create(nsPIDOMWindowInner* aWindow);
+  explicit nsScreen(nsPIDOMWindowInner* aWindow);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(nsScreen,
@@ -38,56 +32,18 @@ class nsScreen : public mozilla::DOMEventTargetHelper {
 
   nsPIDOMWindowOuter* GetOuter() const;
 
-  int32_t GetTop(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetRect(rect);
-    return rect.y;
-  }
+  int32_t Top() { return GetRect().y; }
+  int32_t Left() { return GetRect().x; }
+  int32_t Width() { return GetRect().Width(); }
+  int32_t Height() { return GetRect().Height(); }
 
-  int32_t GetLeft(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetRect(rect);
-    return rect.x;
-  }
+  int32_t AvailTop() { return GetAvailRect().y; }
+  int32_t AvailLeft() { return GetAvailRect().x; }
+  int32_t AvailWidth() { return GetAvailRect().Width(); }
+  int32_t AvailHeight() { return GetAvailRect().Height(); }
 
-  int32_t GetWidth(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetRect(rect);
-    return rect.Width();
-  }
-
-  int32_t GetHeight(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetRect(rect);
-    return rect.Height();
-  }
-
-  int32_t GetPixelDepth(ErrorResult& aRv);
-  int32_t GetColorDepth(ErrorResult& aRv) { return GetPixelDepth(aRv); }
-
-  int32_t GetAvailTop(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetAvailRect(rect);
-    return rect.y;
-  }
-
-  int32_t GetAvailLeft(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetAvailRect(rect);
-    return rect.x;
-  }
-
-  int32_t GetAvailWidth(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetAvailRect(rect);
-    return rect.Width();
-  }
-
-  int32_t GetAvailHeight(ErrorResult& aRv) {
-    mozilla::CSSIntRect rect;
-    aRv = GetAvailRect(rect);
-    return rect.Height();
-  }
+  int32_t PixelDepth();
+  int32_t ColorDepth() { return PixelDepth(); }
 
   // Media Capabilities extension
   mozilla::dom::ScreenColorGamut ColorGamut() const {
@@ -113,10 +69,12 @@ class nsScreen : public mozilla::DOMEventTargetHelper {
 
   IMPL_EVENT_HANDLER(mozorientationchange)
 
-  bool MozLockOrientation(const nsAString& aOrientation, ErrorResult& aRv);
-  bool MozLockOrientation(const mozilla::dom::Sequence<nsString>& aOrientations,
-                          ErrorResult& aRv);
-  void MozUnlockOrientation();
+  // This function is deprecated, use ScreenOrientation API instead.
+  bool MozLockOrientation(const nsAString&) { return false; };
+  bool MozLockOrientation(const mozilla::dom::Sequence<nsString>&) {
+    return false;
+  }
+  void MozUnlockOrientation() {}
 
   virtual JSObject* WrapObject(JSContext* aCx,
                                JS::Handle<JSObject*> aGivenProto) override;
@@ -128,12 +86,11 @@ class nsScreen : public mozilla::DOMEventTargetHelper {
 
  protected:
   nsDeviceContext* GetDeviceContext() const;
-  nsresult GetRect(mozilla::CSSIntRect& aRect);
-  nsresult GetAvailRect(mozilla::CSSIntRect& aRect);
-  nsresult GetWindowInnerRect(mozilla::CSSIntRect& aRect);
+  mozilla::CSSIntRect GetRect();
+  mozilla::CSSIntRect GetAvailRect();
+  mozilla::CSSIntRect GetWindowInnerRect();
 
  private:
-  explicit nsScreen(nsPIDOMWindowInner* aWindow);
   virtual ~nsScreen();
 
   bool ShouldResistFingerprinting(mozilla::RFPTarget aTarget) const;

dom/webidl/Screen.webidl

@@ -7,26 +7,16 @@
 interface Screen : EventTarget {
   // CSSOM-View
   // http://dev.w3.org/csswg/cssom-view/#the-screen-interface
-  [Throws]
   readonly attribute long availWidth;
-  [Throws]
   readonly attribute long availHeight;
-  [Throws]
   readonly attribute long width;
-  [Throws]
   readonly attribute long height;
-  [Throws]
   readonly attribute long colorDepth;
-  [Throws]
   readonly attribute long pixelDepth;
 
-  [Throws]
   readonly attribute long top;
-  [Throws]
   readonly attribute long left;
-  [Throws]
   readonly attribute long availTop;
-  [Throws]
   readonly attribute long availLeft;
 
   /**
@@ -42,17 +32,13 @@ interface Screen : EventTarget {
 
   /**
    * DEPRECATED, use ScreenOrientation API instead.
-   * Lock screen orientation to the specified type.
+   * Lock/unlock screen orientation to the specified type.
+   *
+   * FIXME(emilio): These do literally nothing, we should
+   * try to remove these.
    */
-  [Throws]
   boolean mozLockOrientation(DOMString orientation);
-  [Throws]
   boolean mozLockOrientation(sequence<DOMString> orientation);
-
-  /**
-   * DEPRECATED, use ScreenOrientation API instead.
-   * Unlock the screen orientation.
-   */
   undefined mozUnlockOrientation();
 };
 

dom/webidl/Window.webidl

@@ -314,10 +314,8 @@ dictionary ScrollToOptions : ScrollOptions {
 partial interface Window {
   //[Throws, NewObject, NeedsCallerType] MediaQueryList matchMedia(DOMString query);
   [Throws, NewObject, NeedsCallerType] MediaQueryList? matchMedia(UTF8String query);
-  // Per spec, screen is SameObject, but we don't actually guarantee that given
-  // nsGlobalWindow::Cleanup.  :(
-  //[SameObject, Replaceable, Throws] readonly attribute Screen screen;
-  [Replaceable, Throws] readonly attribute Screen screen;
+
+  [SameObject, Replaceable] readonly attribute Screen screen;
 
   // browsing context
   //[Throws] undefined moveTo(double x, double y);

testing/web-platform/tests/css/cssom-view/screen-detached-frame.html

@@ -0,0 +1,39 @@
+<!doctype html>
+<meta charset="utf-8">
+<link rel="help" href="https://drafts.csswg.org/cssom-view/#dom-window-screen">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1858977">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<iframe></iframe>
+<script>
+onload = function() {
+  test(() => {
+    let frame = document.querySelector("iframe");
+    let win = frame.contentWindow;
+    frame.remove();
+    assert_true(!!win.screen, "Window.screen should be available");
+    for (let prop of ["top", "left", "width", "height"]) {
+      let availProp = "avail" + prop[0].toUpperCase() + prop.substr(1);
+      if (prop == "width" || prop == "height") {
+        assert_true(prop in win.screen, prop + "must be implemented per spec")
+        assert_true(availProp in win.screen, availProp + "must be implemented per spec")
+      }
+      if (prop in win.screen) {
+        assert_equals(win.screen[prop], 0, prop);
+      }
+      if (availProp in win.screen) {
+        assert_equals(win.screen[availProp], 0, availProp);
+      }
+    }
+
+    // https://drafts.csswg.org/cssom-view/#dom-screen-colordepth
+    //   If the user agent does not know the color depth or does not want to
+    //   return it for privacy considerations, it should return 24.
+    for (let prop of ["colorDepth", "pixelDepth"]) {
+      assert_equals(win.screen[prop], 24, prop);
+    }
+  }, "Window.screen on detached frame");
+};
+</script>

xpfe/appshell/AppWindow.cpp

@@ -1114,17 +1114,8 @@ NS_IMETHODIMP AppWindow::GetAvailScreenSize(int32_t* aAvailWidth,
   RefPtr<nsScreen> screen = window->GetScreen();
   NS_ENSURE_STATE(screen);
 
-  ErrorResult rv;
-  *aAvailWidth = screen->GetAvailWidth(rv);
-  if (NS_WARN_IF(rv.Failed())) {
-    return rv.StealNSResult();
-  }
-
-  *aAvailHeight = screen->GetAvailHeight(rv);
-  if (NS_WARN_IF(rv.Failed())) {
-    return rv.StealNSResult();
-  }
-
+  *aAvailWidth = screen->AvailWidth();
+  *aAvailHeight = screen->AvailHeight();
   return NS_OK;
 }