Bug 1674863 Part 1 - Improve type safety for APIs computing video's intrinsic size and ratio. r=emilio

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

dom/html/HTMLVideoElement.cpp

@@ -106,31 +106,32 @@ void HTMLVideoElement::UpdateMediaSize(const nsIntSize& aSize) {
   }
 }
 
-nsresult HTMLVideoElement::GetVideoSize(nsIntSize* size) {
+Maybe<CSSIntSize> HTMLVideoElement::GetVideoSize() const {
   if (!mMediaInfo.HasVideo()) {
-    return NS_ERROR_FAILURE;
+    return Nothing();
   }
 
   if (mDisableVideo) {
-    return NS_ERROR_FAILURE;
+    return Nothing();
   }
 
+  CSSIntSize size;
   switch (mMediaInfo.mVideo.mRotation) {
     case VideoInfo::Rotation::kDegree_90:
     case VideoInfo::Rotation::kDegree_270: {
-      size->width = mMediaInfo.mVideo.mDisplay.height;
-      size->height = mMediaInfo.mVideo.mDisplay.width;
+      size.width = mMediaInfo.mVideo.mDisplay.height;
+      size.height = mMediaInfo.mVideo.mDisplay.width;
       break;
     }
     case VideoInfo::Rotation::kDegree_0:
     case VideoInfo::Rotation::kDegree_180:
     default: {
-      size->height = mMediaInfo.mVideo.mDisplay.height;
-      size->width = mMediaInfo.mVideo.mDisplay.width;
+      size.height = mMediaInfo.mVideo.mDisplay.height;
+      size.width = mMediaInfo.mVideo.mDisplay.width;
       break;
     }
   }
-  return NS_OK;
+  return Some(size);
 }
 
 void HTMLVideoElement::Invalidate(bool aImageSizeChanged,

dom/html/HTMLVideoElement.h

@@ -10,6 +10,7 @@
 #include "mozilla/Attributes.h"
 #include "mozilla/dom/HTMLMediaElement.h"
 #include "mozilla/StaticPrefs_media.h"
+#include "Units.h"
 
 namespace mozilla {
 
@@ -53,9 +54,7 @@ class HTMLVideoElement final : public HTMLMediaElement {
 
   virtual void UnbindFromTree(bool aNullParent = true) override;
 
-  // Set size with the current video frame's height and width.
-  // If there is no video frame, returns NS_ERROR_FAILURE.
-  nsresult GetVideoSize(nsIntSize* size);
+  mozilla::Maybe<mozilla::CSSIntSize> GetVideoSize() const;
 
   virtual void UpdateMediaSize(const nsIntSize& aSize) override;
 

layout/generic/AspectRatio.h

@@ -10,6 +10,7 @@
 /* The aspect ratio of a box, in a "width / height" format. */
 
 #include "mozilla/Attributes.h"
+#include "mozilla/gfx/BaseSize.h"
 #include "nsCoord.h"
 #include <algorithm>
 #include <limits>
@@ -31,6 +32,11 @@ struct AspectRatio {
     return AspectRatio(aWidth / aHeight);
   }
 
+  template <typename T, typename Sub>
+  static AspectRatio FromSize(const gfx::BaseSize<T, Sub>& aSize) {
+    return FromSize(aSize.Width(), aSize.Height());
+  }
+
   explicit operator bool() const { return mRatio != 0.0f; }
 
   nscoord ApplyTo(nscoord aCoord) const {

layout/generic/nsIFrame.h

@@ -468,6 +468,9 @@ struct IntrinsicSize {
   IntrinsicSize(nscoord aWidth, nscoord aHeight)
       : width(Some(aWidth)), height(Some(aHeight)) {}
 
+  explicit IntrinsicSize(const nsSize& aSize)
+      : IntrinsicSize(aSize.Width(), aSize.Height()) {}
+
   mozilla::Maybe<nsSize> ToSize() const {
     return width && height ? Some(nsSize(*width, *height)) : Nothing();
   }

layout/generic/nsVideoFrame.cpp

@@ -175,8 +175,8 @@ already_AddRefed<Layer> nsVideoFrame::BuildLayer(
   nsRect area = GetContentRectRelativeToSelf() + aItem->ToReferenceFrame();
   HTMLVideoElement* element = static_cast<HTMLVideoElement*>(GetContent());
 
-  nsIntSize videoSizeInPx;
-  if (NS_FAILED(element->GetVideoSize(&videoSizeInPx)) || area.IsEmpty()) {
+  Maybe<CSSIntSize> videoSizeInPx = element->GetVideoSize();
+  if (videoSizeInPx.isNothing() || area.IsEmpty()) {
     return nullptr;
   }
 
@@ -191,15 +191,8 @@ already_AddRefed<Layer> nsVideoFrame::BuildLayer(
     return nullptr;
   }
 
-  // Convert video size from pixel units into app units, to get an aspect-ratio
-  // (which has to be represented as a nsSize) and an IntrinsicSize that we
-  // can pass to ComputeObjectRenderRect.
-  auto aspectRatio =
-      AspectRatio::FromSize(videoSizeInPx.width, videoSizeInPx.height);
-  IntrinsicSize intrinsicSize(
-      nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.width),
-      nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.height));
-
+  const auto aspectRatio = AspectRatio::FromSize(*videoSizeInPx);
+  const IntrinsicSize intrinsicSize(CSSPixel::ToAppUnits(*videoSizeInPx));
   nsRect dest = nsLayoutUtils::ComputeObjectDestRect(
       area, intrinsicSize, aspectRatio, StylePosition());
 
@@ -426,8 +419,8 @@ class nsDisplayVideo : public nsPaintedDisplayItem {
     HTMLVideoElement* element =
         static_cast<HTMLVideoElement*>(Frame()->GetContent());
 
-    nsIntSize videoSizeInPx;
-    if (NS_FAILED(element->GetVideoSize(&videoSizeInPx)) || area.IsEmpty()) {
+    Maybe<CSSIntSize> videoSizeInPx = element->GetVideoSize();
+    if (videoSizeInPx.isNothing() || area.IsEmpty()) {
       return true;
     }
 
@@ -444,15 +437,8 @@ class nsDisplayVideo : public nsPaintedDisplayItem {
       return true;
     }
 
-    // Convert video size from pixel units into app units, to get an
-    // aspect-ratio (which has to be represented as a nsSize) and an
-    // IntrinsicSize that we can pass to ComputeObjectRenderRect.
-    IntrinsicSize intrinsicSize(
-        nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.width),
-        nsPresContext::CSSPixelsToAppUnits(videoSizeInPx.height));
-    auto aspectRatio =
-        AspectRatio::FromSize(videoSizeInPx.width, videoSizeInPx.height);
-
+    const auto aspectRatio = AspectRatio::FromSize(*videoSizeInPx);
+    const IntrinsicSize intrinsicSize(CSSPixel::ToAppUnits(*videoSizeInPx));
     nsRect dest = nsLayoutUtils::ComputeObjectDestRect(
         area, intrinsicSize, aspectRatio, Frame()->StylePosition());
 
@@ -646,26 +632,24 @@ AspectRatio nsVideoFrame::GetIntrinsicRatio() const {
     return AspectRatio();
   }
 
-  // 'contain:size' replaced elements have intrinsic size 0,0.
+  // 'contain:size' replaced elements have no intrinsic ratio.
   if (StyleDisplay()->IsContainSize()) {
     return AspectRatio();
   }
 
-  nsIntSize size(REPLACED_ELEM_FALLBACK_PX_WIDTH,
-                 REPLACED_ELEM_FALLBACK_PX_HEIGHT);
-
   HTMLVideoElement* element = static_cast<HTMLVideoElement*>(GetContent());
-  if (NS_SUCCEEDED(element->GetVideoSize(&size))) {
-    return AspectRatio::FromSize(size.width, size.height);
+  if (Maybe<CSSIntSize> size = element->GetVideoSize()) {
+    return AspectRatio::FromSize(*size);
   }
 
   if (ShouldDisplayPoster()) {
     if (Maybe<nsSize> imgSize = PosterImageSize()) {
-      return AspectRatio::FromSize(imgSize->width, imgSize->height);
+      return AspectRatio::FromSize(*imgSize);
     }
   }
 
-  return AspectRatio::FromSize(size.width, size.height);
+  return AspectRatio::FromSize(REPLACED_ELEM_FALLBACK_PX_WIDTH,
+                               REPLACED_ELEM_FALLBACK_PX_HEIGHT);
 }
 
 bool nsVideoFrame::ShouldDisplayPoster() const {
@@ -697,18 +681,19 @@ nsSize nsVideoFrame::GetVideoIntrinsicSize(gfxContext* aRenderingContext) {
     return nsSize(0, 0);
   }
 
-  nsIntSize size(REPLACED_ELEM_FALLBACK_PX_WIDTH,
-                 REPLACED_ELEM_FALLBACK_PX_HEIGHT);
-
   HTMLVideoElement* element = static_cast<HTMLVideoElement*>(GetContent());
-  if (NS_FAILED(element->GetVideoSize(&size)) && ShouldDisplayPoster()) {
+  if (Maybe<CSSIntSize> size = element->GetVideoSize()) {
+    return CSSPixel::ToAppUnits(*size);
+  }
+
+  if (ShouldDisplayPoster()) {
     if (Maybe<nsSize> imgSize = PosterImageSize()) {
       return *imgSize;
     }
   }
 
-  return nsSize(nsPresContext::CSSPixelsToAppUnits(size.width),
-                nsPresContext::CSSPixelsToAppUnits(size.height));
+  return CSSPixel::ToAppUnits(CSSIntSize(REPLACED_ELEM_FALLBACK_PX_WIDTH,
+                                         REPLACED_ELEM_FALLBACK_PX_HEIGHT));
 }
 
 void nsVideoFrame::UpdatePosterSource(bool aNotify) {
@@ -756,11 +741,11 @@ bool nsVideoFrame::HasVideoElement() const {
 }
 
 bool nsVideoFrame::HasVideoData() const {
-  if (!HasVideoElement()) return false;
-  HTMLVideoElement* element = static_cast<HTMLVideoElement*>(GetContent());
-  nsIntSize size(0, 0);
-  element->GetVideoSize(&size);
-  return size != nsIntSize(0, 0);
+  if (!HasVideoElement()) {
+    return false;
+  }
+  auto* element = static_cast<HTMLVideoElement*>(GetContent());
+  return element->GetVideoSize().isSome();
 }
 
 void nsVideoFrame::UpdateTextTrack() {