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+

Description Yong Li 2009-07-28 13:42:32 PDT
patch is comming
Comment 1 Yong Li 2009-07-28 13:45:50 PDT
Created attachment 33668 [details]
the patch
Comment 2 Yong Li 2009-07-28 13:51:53 PDT
Created attachment 33669 [details]
remove an unnecessary line
Comment 3 George Staikos 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?
Comment 4 Yong Li 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.
Comment 5 George Staikos 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?
Comment 6 Yong Li 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
Comment 7 Yong Li 2009-07-29 10:06:00 PDT
Created attachment 33719 [details]
remove unnecessary changes
Comment 8 Yong Li 2009-07-29 12:30:52 PDT
Created attachment 33731 [details]
add some comments for removed ASSERT
Comment 9 George Staikos 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.
Comment 10 George Staikos 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.
Comment 11 Yong Li 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.
Comment 12 Yong Li 2009-07-30 07:27:11 PDT
Created attachment 33777 [details]
more clean up
Comment 13 George Staikos 2009-07-30 11:09:02 PDT
Committed r46590