Bug 29204 - Compile KeyframeList.cpp and AnimationBase.cpp in winscw compiler.
Summary: Compile KeyframeList.cpp and AnimationBase.cpp in winscw compiler.
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: S60 Emulator S60 3rd edition
: P2 Normal
Assignee: Yongjun Zhang
URL:
Keywords:
Depends on:
Blocks: 27065
  Show dependency treegraph
 
Reported: 2009-09-11 12:34 PDT by Yongjun Zhang
Modified: 2011-12-05 09:35 PST (History)
2 users (show)

See Also:


Attachments
Move setAnimation() and KeyframeList() ctor to respective cpp files. (3.16 KB, patch)
2009-09-11 13:27 PDT, Yongjun Zhang
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yongjun Zhang 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.
Comment 1 Yongjun Zhang 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
Comment 2 Yongjun Zhang 2009-09-11 13:27:46 PDT
Created attachment 39472 [details]
Move setAnimation() and KeyframeList() ctor to respective cpp files.
Comment 3 Eric Seidel (no email) 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.
Comment 4 Yongjun Zhang 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().
Comment 5 Andreas Kling 2011-12-05 09:35:04 PST
WinSCW support is no longer an issue as Symbian support has been removed from WebKit trunk.