Summary: | Add full screen animation code to WebFullScreenManager. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||
Component: | WebKit2 | Assignee: | Jer Noble <jer.noble> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, eric.carlson, jer.noble, sam | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 55993 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Jer Noble
2011-03-11 13:47:18 PST
Created attachment 85530 [details]
Patch
Created attachment 85539 [details]
Patch
Modified the linker settings for new files in the .xcodeproj.
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". (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! Committed r80923: <http://trac.webkit.org/changeset/80923> Committed r80924: <http://trac.webkit.org/changeset/80924> Committed r80925: <http://trac.webkit.org/changeset/80925> webkit-patch added the above commit messages incorrectly, probably due to #56241. The correct message is the last one: Committed r80925: <http://trac.webkit.org/changeset/80925> |