Bug 175152 – Improve WebKitLegacy video fullscreen animation begin and end rects.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jeremy Jones]

Improve WebKitLegacy video fullscreen animation begin and end rects.

[Jeremy Jones]

rdar://problem/32840576

[Jeremy Jones]

Created attachment 317146 [details]
Patch

[Eric Carlson]

Comment on attachment 317146 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317146&action=review

> Source/WebCore/platform/mac/WebVideoFullscreenController.mm:236
> +static NSRect frameExpandedToRatioOfFrame(NSRect frameToExpand, NSRect frame)

Nit: NSRect&

> Source/WebCore/platform/mac/WebVideoFullscreenController.mm:284
>      NSRect frame = [self videoElementRect];
>      NSRect endFrame = [screen frame];
> -    constrainFrameToRatioOfFrame(&endFrame, &frame);
> +    frame = frameExpandedToRatioOfFrame(frame, endFrame);

Nit: this would be slightly cleaner as something like

NSRect frame = frameExpandedToRatioOfFrame([self videoElementRect], endFrame);

[Jeremy Jones]

(In reply to Eric Carlson from comment #3)
> Comment on attachment 317146 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317146&action=review
> 
> > Source/WebCore/platform/mac/WebVideoFullscreenController.mm:236
> > +static NSRect frameExpandedToRatioOfFrame(NSRect frameToExpand, NSRect frame)
> 
> Nit: NSRect&
> 

For small structs the cost of indirection might be higher than the cost of copying.

> > Source/WebCore/platform/mac/WebVideoFullscreenController.mm:284
> >      NSRect frame = [self videoElementRect];
> >      NSRect endFrame = [screen frame];
> > -    constrainFrameToRatioOfFrame(&endFrame, &frame);
> > +    frame = frameExpandedToRatioOfFrame(frame, endFrame);
> 
> Nit: this would be slightly cleaner as something like
> 
> NSRect frame = frameExpandedToRatioOfFrame([self videoElementRect],
> endFrame);

Done.

[Jeremy Jones]

Created attachment 317174 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 317174 [details]
Patch for landing.

Clearing flags on attachment: 317174

Committed r220248: <http://trac.webkit.org/changeset/220248>