[webkit-reviews] review granted: [Bug 132603] [iOS] Animate AVPlayerLayer into and out of full screen : [Attachment 230893] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 132603: [iOS] Animate AVPlayerLayer into and out of full screen
https://bugs.webkit.org/show_bug.cgi?id=132603

Attachment 230893: Patch
https://bugs.webkit.org/attachment.cgi?id=230893&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230893&action=review


> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:354
> +- (id)init

instancetype?

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:359
> +	   self.masksToBounds = YES;
> +	   self.videoLayerGravity = AVVideoLayerGravityResizeAspect;

Shouldn't use property syntax in the init method.

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:380
> +static FloatRect largestRectWithAspectRatioInsideRect(float aspectRatio,
FloatRect rect)
> +{
> +    if (aspectRatio > rect.size().aspectRatio()) {
> +	   float dy = rect.width() / aspectRatio - rect.height();
> +	   rect.inflateY(dy / 2);
> +    } else {
> +	   float dx = rect.height() * aspectRatio - rect.width();
> +	   rect.inflateX(dx / 2);
> +    }
> +    return rect;
> +}

Pretty sure we have code elsewhere that does aspect-ratio sizing.

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:392
> +    FloatRect rootBounds;
> +    CALayer *parent = self;
> +    while ((parent = [parent superlayer]))
> +	   rootBounds = [parent bounds];

This is weird. How do you know how far up you're walking? Which rect do you
actually want?