[Webkit-unassigned] [Bug 23492] Separating the WebKitSystemInterface Calls

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=23492


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26954|review?                     |review-
               Flag|                            |




------- Comment #2 from darin at apple.com  2009-01-23 14:00 PDT -------
(From update of attachment 26954)
>  #include <CoreFoundation/CoreFoundation.h>
> +#if PLATFORM(CG)
>  #include <CoreGraphics/CoreGraphics.h>
> +#endif
>  #include <shlobj.h>
>  #include <shfolder.h>
>  #include <tchar.h>
> +#if PLATFORM(CG)
>  #include <WebKitSystemInterface/WebKitSystemInterface.h>
> +#endif
>  #include <wtf/HashMap.h>
>  #include <wtf/OwnArrayPtr.h>

Preferred WebKit style is to put all unconditional includes at the top of the
file in one section, then put the conditional includes in a separate block,
sorted separately.

> +// Link stubs for non-Safari clients
> +#if !PLATFORM(CG)
> +static void wkSetFontSmoothingLevel(int smoothingLevel) { notImplemented(); }
> +static void wkSetFontSmoothingContrast(float contrast) { notImplemented(); }
> +#endif

Why are these stubs needed? These are CG-specific calls. I think we should
instead have #if at the call sites. Since there's only one call to each
function, it seems straightforward to do it that way instead.

I don't think that non-Core-Graphics versions of this will have calls with this
same design, so I don't think we should have link stubs for them.

>  #include <CoreFoundation/CFString.h>
> +#if PLATFORM(CG)
>  #include <WebKitSystemInterface/WebKitSystemInterface.h>
> +#endif
>  #include <wtf/RetainPtr.h>

Same comment here.
>  
> +// Link stubs for non-Safari clients
> +#if !PLATFORM(CG)
> +#include <WebCore/NotImplemented.h>
> +static void wkAddFontsAtPath(CFStringRef) { notImplemented(); }
> +#endif

Same comment here. I don't think you need a stub for this Core Graphics
function call. You just need to put #if PLATFORM(CG) around the part of the
WebTextRenderer::registerPrivateFont function that creates a CFStringRef and
calls wkAddFontsAtPath.

I'm going to say review-, because I'd prefer not to create "stubs" unless
there's some real value there.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.