Bug 56220 – Add full screen animation code to WebFullScreenManager.

• Component: WebKit2
• Status: RESOLVED FIXED

[Jer Noble]

Animation of the full screen element's CALayer must be done in the WebProcess, so add that code to WebFullScreenManager.

[Jer Noble]

Created attachment 85530 [details]
Patch

[Jer Noble]

Created attachment 85539 [details]
Patch

Modified the linker settings for new files in the .xcodeproj.

[Anders Carlsson]

Comment on attachment 85539 [details]
Patch

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

> Source/WebKit2/ChangeLog:8
> +        * WebProcess/FullScreen/WebFullScreenManager.cpp: Move functions into virtual base class.

I don't see a virtual base class here. Maybe this should say "Move virtual functions into base class"?

> Source/WebKit2/WebProcess/FullScreen/WebFullScreenManager.cpp:51
> +    

Extra newline.

> Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.h:38
> +typedef struct objc_object *id;

I don't think forward declaring id like this works with newer compilers. Could you forward declare WebFullScreenManagerAnimationListener instead and use it for the RetainPtr?

We have an OBJC_CLASS macro that is useful for forward declaring Objective-C classes.

> Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:88
> +    _manager = NULL;

= 0;

> Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:132
> +        m_rootLayer->setName("LayerTreeHost root layer");

This should say something else, maybe "Full screen root layer".

[Jer Noble]

(In reply to comment #3)
> (From update of attachment 85539 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=85539&action=review
> 
> > Source/WebKit2/ChangeLog:8
> > +        * WebProcess/FullScreen/WebFullScreenManager.cpp: Move functions into virtual base class.
> 
> I don't see a virtual base class here. Maybe this should say "Move virtual functions into base class"?

Yes, this was incredibly poorly written.  How about, "Make functions pure virtual to be implemented by a concrete subclass."?

> > Source/WebKit2/WebProcess/FullScreen/WebFullScreenManager.cpp:51
> > +    
> 
> Extra newline.

Deleted.

> > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.h:38
> > +typedef struct objc_object *id;
> 
> I don't think forward declaring id like this works with newer compilers. Could you forward declare WebFullScreenManagerAnimationListener instead and use it for the RetainPtr?
> 
> We have an OBJC_CLASS macro that is useful for forward declaring Objective-C classes.

OBJC_CLASS seems to work fine here.  Changed.

> > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:88
> > +    _manager = NULL;
> 
> = 0;

Changed.

> > Source/WebKit2/WebProcess/FullScreen/mac/WebFullScreenManagerMac.mm:132
> > +        m_rootLayer->setName("LayerTreeHost root layer");
> 
> This should say something else, maybe "Full screen root layer".

Suggestion taken.  Changed.

Thanks!

[Jer Noble]

Committed r80923: <http://trac.webkit.org/changeset/80923>

[Jer Noble]

Committed r80924: <http://trac.webkit.org/changeset/80924>

[Jer Noble]

Committed r80925: <http://trac.webkit.org/changeset/80925>

[Jer Noble]

webkit-patch added the above commit messages incorrectly, probably due to #56241.

[Jer Noble]

The correct message is the last one:

Committed r80925: <http://trac.webkit.org/changeset/80925>