Bug 29204

Summary: Compile KeyframeList.cpp and AnimationBase.cpp in winscw compiler.
Product: WebKit Reporter: Yongjun Zhang <yongjun.zhang>
Component: WebCore Misc.Assignee: Yongjun Zhang <yongjun.zhang>
Status: RESOLVED WONTFIX    
Severity: Normal CC: kling, laszlo.gombos
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: S60 Emulator   
OS: S60 3rd edition   
Bug Depends on:    
Bug Blocks: 27065    
Attachments:
Description Flags
Move setAnimation() and KeyframeList() ctor to respective cpp files. eric: review-

Yongjun Zhang
Reported 2009-09-11 12:34:04 PDT
KeyframeList(RenderObject*, const AtomicString&) is implemented in KeyframeList.h, and it calls KeyframeList::insert(float, PassRefPtr<RenderStyle>); the compiler tries to aggressively resolve RenderStyle::ref() in PassRefPtr<T>::PassRefPtr(T*) ctr, and it complains "illegal use of incomplete class" if RenderStyle.h is not included. In AnimationBase.h, setAnimation(..) is inlined and RefPtr<T>::operator=(T*) is called in this function which fails to resolve Animation::ref(). One idea is to not inline ref() call in PassRefPtr and RefPtr, as suggested by https://bugs.webkit.org/show_bug?id=28054#c15 . But this requires changes RefPtr & PassRefPtr and it is not ideal (see comment https://bugs.webkit.org/show_bug?id=28054#c17). It looks like only those two files uses inlined ref via RefPtr & PassRefPtr in header files. As a less intrusive way we can move the implementation of these two functions to respective cpp files.
Attachments
Move setAnimation() and KeyframeList() ctor to respective cpp files. (3.16 KB, patch)
2009-09-11 13:27 PDT, Yongjun Zhang 		eric: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yongjun Zhang
Comment 1 2009-09-11 12:37:24 PDT
sorry, the comment url should be: https://bugs.webkit.org/show_bug.cgi?id=28054#c15 and https://bugs.webkit.org/show_bug.cgi?id=28054#c17
Yongjun Zhang
Comment 2 2009-09-11 13:27:46 PDT
Created attachment 39472 [details] Move setAnimation() and KeyframeList() ctor to respective cpp files.
Eric Seidel (no email)
Comment 3 2009-09-11 16:45:28 PDT
Comment on attachment 39472 [details] Move setAnimation() and KeyframeList() ctor to respective cpp files. Why are these changes needed? And why are we supporting this broken compiler... ;) The number of fixes needed to make winscw work seems to be growing without bound. r-. We need more explanation for each of these (seemingly arbitrary) changes, and ideally links to compiler bugs you've filed with the winscw folks.
Yongjun Zhang
Comment 4 2009-09-11 20:57:50 PDT
(In reply to comment #3) > (From update of attachment 39472 [details]) Thanks a lot for the comment, Eric. > Why are these changes needed? And why are we supporting this broken compiler... Winscw compiler (based on Codewarrior) is used in building apps running in S60 emulator, so far that is the only available emulator compiler for us :-( > ;) The number of fixes needed to make winscw work seems to be growing without > bound. We fully understand the concern; and we tried hard to make sure the changes are minimum and really necessary. Good news is all the winscw patches are submitted for review and looks like there won't be any further patches for this issue any more. > r-. We need more explanation for each of these (seemingly arbitrary) changes, > and ideally links to compiler bugs you've filed with the winscw folks. There is a bug for it in winscw compiler bugzilla (https://xdabug001.ext.nokia.com/bugzilla/show_bug.cgi?id=9812). It is essentially the same issue as https://bugs.webkit.org/show_bug?id=28054 except it is caused by ref(), not deref().
Andreas Kling
Comment 5 2011-12-05 09:35:04 PST
WinSCW support is no longer an issue as Symbian support has been removed from WebKit trunk.
Note You need to log in before you can comment on or make changes to this bug.