[Cocoa] Sometimes a YouTube video is incorrectly sized, and doesn't r…

…espond to window resize

https://bugs.webkit.org/show_bug.cgi?id=257628
rdar://109754524

Reviewed by Simon Fraser.

`-[WebAVPlayerLayer layoutSublayers]` will calculate a `targetVideoFrame` based on the current
videoLayer's bounds, the videoDimensions, and gravity. If this value matches the current video
frame, it bails out early without making any changes.

However, it doesn't save `targetVideoFrame` to `_targetVideoFrame`. If the gravity, videoDimensions,
or even the videoLayer's bounds change, and there's an existing pending call to `-resolveBounds`,
an incorrent frame will be set on the videoLayer (whatever the last successful call to
`-layoutSublayers` generated).

Rather than create a local `targetVideoFrame`, just save the results of calling
`-calculateTargetVideoFrame` to the `_targetVideoFrame` ivar. This ensures that subsequent calls to
`-resolveBounds` operate on the correct values.

Extensive drive-by fix: In investigating this bug, there were a number of times where logging which
included a frame's location would have been useful. But FloatPoint and FloatRect don't have logging
methods to convert their values to a String (only FloatSize does). So this patch adds those missing
logging methods and converts existing logging to log the whole rect.

* Source/WebCore/platform/cocoa/WebAVPlayerLayer.mm:
(-[WebAVPlayerLayer layoutSublayers]):
(-[WebAVPlayerLayer resolveBounds]):
* Source/WebCore/platform/graphics/FloatPoint.cpp:
(WebCore::FloatPoint::toJSONObject const):
(WebCore::FloatPoint::toJSONString const):
* Source/WebCore/platform/graphics/FloatPoint.h:
(WTF::LogArgument<WebCore::FloatPoint>::toString):
* Source/WebCore/platform/graphics/FloatRect.cpp:
(WebCore::FloatRect::toJSONObject const):
(WebCore::FloatRect::toJSONString const):
* Source/WebCore/platform/graphics/FloatRect.h:
(WTF::LogArgument<WebCore::FloatRect>::toString):
* Source/WebCore/platform/graphics/avfoundation/objc/VideoLayerManagerObjC.mm:
(WebCore::VideoLayerManagerObjC::setVideoLayer):
* Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.mm:
(WebKit::VideoFullscreenModelContext::setVideoLayerFrame):

Canonical link: https://commits.webkit.org/264832@main

Source/WebCore/platform/cocoa/WebAVPlayerLayer.mm

@@ -218,7 +218,7 @@ - (void)layoutSublayers
     }
 
     FloatRect sourceVideoFrame = self.videoSublayer.bounds;
-    FloatRect targetVideoFrame = [self calculateTargetVideoFrame];
+    _targetVideoFrame = [self calculateTargetVideoFrame];
 
     float videoAspectRatio = self.videoDimensions.width / self.videoDimensions.height;
 
@@ -233,34 +233,32 @@ - (void)layoutSublayers
         // The initial resize will have an empty videoLayerFrame, which makes
         // the subsequent calculations incorrect. When this happens, just do
         // the synchronous resize step instead.
-        _targetVideoFrame = targetVideoFrame;
         OBJC_INFO_LOG(OBJC_LOGIDENTIFIER, "sourceVideoFrame is empty; calling -resolveBounds");
         [self resolveBounds];
         return;
     }
 
-    if (sourceVideoFrame == targetVideoFrame && CGAffineTransformIsIdentity([_videoSublayer affineTransform])) {
-        OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame (", _targetVideoFrame.size(), ") is equal to sourceVideoFrame, and affineTransform is identity, bailing");
+    if (sourceVideoFrame == _targetVideoFrame && CGAffineTransformIsIdentity([_videoSublayer affineTransform])) {
+        OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame (", _targetVideoFrame, ") is equal to sourceVideoFrame, and affineTransform is identity, bailing");
         return;
     }
 
     // CALayer -frame will take the -position, -bounds, and -affineTransform into account,
     // so bail out if the current -frame is essentially equal to the targetVideoFrame.
-    if (areFramesEssentiallyEqualWithTolerance(self.videoSublayer.frame, targetVideoFrame)) {
-        OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame (", _targetVideoFrame.size(), ") is essentially equal to videoSublayer.frame, bailing");
+    if (areFramesEssentiallyEqualWithTolerance(self.videoSublayer.frame, _targetVideoFrame)) {
+        OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame (", _targetVideoFrame, ") is essentially equal to videoSublayer.frame, bailing");
         return;
     }
 
-    CGAffineTransform transform = CGAffineTransformMakeScale(targetVideoFrame.width() / sourceVideoFrame.width(), targetVideoFrame.height() / sourceVideoFrame.height());
+    CGAffineTransform transform = CGAffineTransformMakeScale(_targetVideoFrame.width() / sourceVideoFrame.width(), _targetVideoFrame.height() / sourceVideoFrame.height());
 
     [CATransaction begin];
     [_videoSublayer setAnchorPoint:CGPointMake(0.5, 0.5)];
     [_videoSublayer setAffineTransform:transform];
     [_videoSublayer setPosition:CGPointMake(CGRectGetMidX(self.bounds), CGRectGetMidY(self.bounds))];
     [CATransaction commit];
 
-    _targetVideoFrame = targetVideoFrame;
-    OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame: ", targetVideoFrame.size(), ", transform: [", transform.a, ", ", transform.d, "]");
+    OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, "self.bounds: ", FloatRect(self.bounds), ", targetVideoFrame: ", _targetVideoFrame, ", transform: [", transform.a, ", ", transform.d, "]");
 
     NSTimeInterval animationDuration = [CATransaction animationDuration];
     RunLoop::main().dispatch([self, strongSelf = retainPtr(self), animationDuration] {
@@ -279,15 +277,15 @@ - (void)resolveBounds
     }
 
     if (areFramesEssentiallyEqualWithTolerance(self.videoSublayer.frame, _targetVideoFrame) && CGAffineTransformIsIdentity([_videoSublayer affineTransform])) {
-        OBJC_INFO_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame (", _targetVideoFrame.size(), ") is equal to videoSublayer.bounds, and affineTransform is identity, bailing");
+        OBJC_INFO_LOG(OBJC_LOGIDENTIFIER, "targetVideoFrame (", _targetVideoFrame, ") is equal to videoSublayer.bounds, and affineTransform is identity, bailing");
         return;
     }
 
     [CATransaction begin];
     [CATransaction setAnimationDuration:0];
     [CATransaction setDisableActions:YES];
 
-    OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, _targetVideoFrame.size());
+    OBJC_DEBUG_LOG(OBJC_LOGIDENTIFIER, _targetVideoFrame);
 
     if (_fullscreenModel) {
         FloatRect targetVideoBounds { { }, _targetVideoFrame.size() };

Source/WebCore/platform/graphics/FloatPoint.cpp

@@ -95,4 +95,19 @@ TextStream& operator<<(TextStream& ts, const FloatPoint& p)
     return ts << "(" << TextStream::FormatNumberRespectingIntegers(p.x()) << "," << TextStream::FormatNumberRespectingIntegers(p.y()) << ")";
 }
 
+Ref<JSON::Object> FloatPoint::toJSONObject() const
+{
+    auto object = JSON::Object::create();
+
+    object->setDouble("x"_s, m_x);
+    object->setDouble("y"_s, m_y);
+
+    return object;
+}
+
+String FloatPoint::toJSONString() const
+{
+    return toJSONObject()->toJSONString();
+}
+
 }

Source/WebCore/platform/graphics/FloatPoint.h

@@ -193,6 +193,9 @@ class FloatPoint {
     WEBCORE_EXPORT FloatPoint matrixTransform(const TransformationMatrix&) const;
     WEBCORE_EXPORT FloatPoint matrixTransform(const AffineTransform&) const;
 
+    WEBCORE_EXPORT String toJSONString() const;
+    WEBCORE_EXPORT Ref<JSON::Object> toJSONObject() const;
+
 private:
     float m_x { 0 };
     float m_y { 0 };
@@ -307,3 +310,15 @@ WEBCORE_EXPORT WTF::TextStream& operator<<(WTF::TextStream&, const FloatPoint&);
 
 }
 
+namespace WTF {
+
+template<typename Type> struct LogArgument;
+template <>
+struct LogArgument<WebCore::FloatPoint> {
+    static String toString(const WebCore::FloatPoint& point)
+    {
+        return point.toJSONString();
+    }
+};
+
+} // namespace WTF

Source/WebCore/platform/graphics/FloatRect.cpp

@@ -281,4 +281,19 @@ TextStream& operator<<(TextStream& ts, const FloatRect &r)
     return ts << r.location() << " " << r.size();
 }
 
+Ref<JSON::Object> FloatRect::toJSONObject() const
+{
+    auto object = JSON::Object::create();
+
+    object->setObject("location"_s, m_location.toJSONObject());
+    object->setObject("size"_s, m_size.toJSONObject());
+
+    return object;
+}
+
+String FloatRect::toJSONString() const
+{
+    return toJSONObject()->toJSONString();
+}
+
 }

Source/WebCore/platform/graphics/FloatRect.h

@@ -239,6 +239,9 @@ class FloatRect {
     static FloatRect infiniteRect();
     bool isInfinite() const;
 
+    WEBCORE_EXPORT String toJSONString() const;
+    WEBCORE_EXPORT Ref<JSON::Object> toJSONObject() const;
+
 private:
     FloatPoint m_location;
     FloatSize m_size;
@@ -322,3 +325,16 @@ WEBCORE_EXPORT id makeNSArrayElement(const FloatRect&);
 #endif
 
 }
+
+namespace WTF {
+
+template<typename Type> struct LogArgument;
+template <>
+struct LogArgument<WebCore::FloatRect> {
+    static String toString(const WebCore::FloatRect& rect)
+    {
+        return rect.toJSONString();
+    }
+};
+
+} // namespace WTF

Source/WebCore/platform/graphics/avfoundation/objc/VideoLayerManagerObjC.mm

@@ -63,7 +63,7 @@
 
 void VideoLayerManagerObjC::setVideoLayer(PlatformLayer *videoLayer, FloatSize contentSize)
 {
-    ALWAYS_LOG(LOGIDENTIFIER, contentSize.width(), ", ", contentSize.height());
+    ALWAYS_LOG(LOGIDENTIFIER, contentSize);
 
     m_videoLayer = videoLayer;
     [m_videoLayer web_disableAllActions];

Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.mm

@@ -220,7 +220,7 @@ - (BOOL)prefersStatusBarHidden
 
 void VideoFullscreenModelContext::setVideoLayerFrame(WebCore::FloatRect frame)
 {
-    ALWAYS_LOG_IF_POSSIBLE(LOGIDENTIFIER, frame.size());
+    ALWAYS_LOG_IF_POSSIBLE(LOGIDENTIFIER, frame);
     if (m_manager)
         m_manager->setVideoLayerFrame(m_contextId, frame);
 }