Bug 27779

Summary: WINCE PORT: modified graphics files
Product: WebKit Reporter: Yong Li <yong.li.webkit>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: manyoso, staikos
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Other   
Bug Depends on:    
Bug Blocks: 23154    
Attachments:
Description Flags
the patch
none
remove an unnecessary line
none
remove unnecessary changes
none
add some comments for removed ASSERT
staikos: review-
more clean up staikos: review+

Yong Li
Reported 2009-07-28 13:42:32 PDT
patch is comming
Attachments
the patch (16.73 KB, patch)
2009-07-28 13:45 PDT, Yong Li 		no flags
DetailsFormatted DiffDiff
remove an unnecessary line (16.47 KB, patch)
2009-07-28 13:51 PDT, Yong Li 		no flags
DetailsFormatted DiffDiff
remove unnecessary changes (15.30 KB, patch)
2009-07-29 10:06 PDT, Yong Li 		no flags
DetailsFormatted DiffDiff
add some comments for removed ASSERT (15.41 KB, patch)
2009-07-29 12:30 PDT, Yong Li 		staikos: review-
DetailsFormatted DiffDiff
more clean up (14.73 KB, patch)
2009-07-30 07:27 PDT, Yong Li 		staikos: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Yong Li
Comment 1 2009-07-28 13:45:50 PDT
Created attachment 33668 [details] the patch
Yong Li
Comment 2 2009-07-28 13:51:53 PDT
Created attachment 33669 [details] remove an unnecessary line
George Staikos
Comment 3 2009-07-28 18:32:41 PDT
Comment on attachment 33669 [details] remove an unnecessary line A lot of the things conditionally removed in this patch look suspicious to me. Are you sure they're correct?
Yong Li
Comment 4 2009-07-29 06:54:48 PDT
(In reply to comment #3) > (From update of attachment 33669 [details]) > A lot of the things conditionally removed in this patch look suspicious to me. > Are you sure they're correct? Which things do you mean? I didn't see many things removed. We don't have PlatformGradient.
George Staikos
Comment 5 2009-07-29 07:18:00 PDT
Is it possible to refactor this into a PlatformGradient class that doesn't do any "platform" calls so we still fit within the framework?
Yong Li
Comment 6 2009-07-29 08:45:15 PDT
(In reply to comment #5) > Is it possible to refactor this into a PlatformGradient class that doesn't do > any "platform" calls so we still fit within the framework? Yes. We can leave a void* for it. just a waste of 4 bytes. I'll clean it up
Yong Li
Comment 7 2009-07-29 10:06:00 PDT
Created attachment 33719 [details] remove unnecessary changes
Yong Li
Comment 8 2009-07-29 12:30:52 PDT
Created attachment 33731 [details] add some comments for removed ASSERT
George Staikos
Comment 9 2009-07-29 12:37:10 PDT
Comment on attachment 33731 [details] add some comments for removed ASSERT Still more ifdef than I'd like to see but I think it's ok for now. Unfortunately WinCE doesn't make this easy.
George Staikos
Comment 10 2009-07-29 20:49:45 PDT
Comment on attachment 33731 [details] add some comments for removed ASSERT > +#if !PLATFORM(WINCE) || PLATFORM(QT) > PlatformGraphicsContext* platformContext() const; > +#endif > + > + const Font& font() const; > + void setFont(const Font&); I think we have a problem here. Why are these unconditionally added? In retrospect I think we need more cleanup.
Yong Li
Comment 11 2009-07-30 06:41:52 PDT
(In reply to comment #10) > (From update of attachment 33731 [details]) > > > +#if !PLATFORM(WINCE) || PLATFORM(QT) > > PlatformGraphicsContext* platformContext() const; > > +#endif > > + > > + const Font& font() const; > > + void setFont(const Font&); > > I think we have a problem here. Why are these unconditionally added? > > In retrospect I think we need more cleanup. Hm... we don't use them. probably it's a garbage from very old revision. I'll do do more cleanup.
Yong Li
Comment 12 2009-07-30 07:27:11 PDT
Created attachment 33777 [details] more clean up
George Staikos
Comment 13 2009-07-30 11:09:02 PDT
Committed r46590
Note You need to log in before you can comment on or make changes to this bug.