Bug 71994 – Fix IOSurface-related WebCoreSystemInterface defines.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Kevin Ollivier]

Fix IOSurface-related WebKitSystemInterface defines.

[Kevin Ollivier]

Created attachment 114426 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 114426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114426&action=review

> Source/WebCore/ChangeLog:3
> +        Fix IOSurface-related WebKitSystemInterface defines.

This patch changes WebCoreSystemInterface, not WebKitSystemInterface.

> Source/WebKit/mac/WebCoreSupport/WebSystemInterface.mm:123
> +#if PLATFORM(MAC) && USE(CA)

PLATFORM(MAC) without USE(CA) doesn't sound like a valid combination.

[Kevin Ollivier]

(In reply to comment #2)
> (From update of attachment 114426 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114426&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Fix IOSurface-related WebKitSystemInterface defines.
> 
> This patch changes WebCoreSystemInterface, not WebKitSystemInterface.
> 
> > Source/WebKit/mac/WebCoreSupport/WebSystemInterface.mm:123
> > +#if PLATFORM(MAC) && USE(CA)
> 
> PLATFORM(MAC) without USE(CA) doesn't sound like a valid combination.

I don't know if it's valid or not, someone on the Apple side would need to comment on that. However, right now it doesn't compile for !PLATFORM(MAC), such as for wx, which does use this library as well. The problem is that PLATFORM(MAC) && USE(CA) is checked when defining the IOSurfaceRef typedef, but not when using it. For the definition, see:

http://trac.webkit.org/browser/trunk/Source/WebCore/platform/mac/WebCoreSystemInterface.h#L60

IMHO we should at least make the platform and use checks consistent to ensure compilation works for non-Apple ports using WebCoreSystemInterface on Mac, then file a separate bug if PLATFORM(MAC) && USE(CA) should be shortened to just PLATFORM(MAC).

[Eric Seidel (no email)]

Comment on attachment 114426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114426&action=review

ap seems to be objecting, and this patch just seems confusing in its utility to me.

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

This will cause the CQ to fail this patch.

> Source/WebCore/platform/mac/WebCoreSystemInterface.h:259
> +#if PLATFORM(MAC) && USE(CA)
>  extern CGContextRef (*wkIOSurfaceContextCreate)(IOSurfaceRef surface, unsigned width, unsigned height, CGColorSpaceRef colorSpace);
> +#endif

Why do we have a PLATFORM(MAC) check inside mac/?

[Anders Carlsson]

Wx port is gone from WebKit, so this change is not needed.