Bug 95977 – webkit build for 64-bit Mac chromium

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Catalin badea]

Webkit yields compilation errors when trying to build chromium for the x86_64 architecture (Mac OS X 10.7).

[Catalin badea]

Created attachment 162501 [details]
Patch fix.

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Eric Seidel (no email)]

Comment on attachment 162501 [details]
Patch fix.

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

> Source/WebKit/chromium/public/mac/WebSubstringUtil.h:34
> +#include "../platform/WebCommon.h"

This seems like an odd way to include a header?  Are you sure this is proper form?  Maybe <platform/WebCommon.h> ?

> Source/WebKit/chromium/src/mac/WebSubstringUtil.mm:35
> +#import <Cocoa/Cocoa.h>

Why the re-org?

[Eric Seidel (no email)]

Comment on attachment 162501 [details]
Patch fix.

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

> Source/WebCore/platform/graphics/IntRect.h:236
> +#if (PLATFORM(MAC) || (PLATFORM(CHROMIUM) && OS(DARWIN))) \
> +        && !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES) \
> +        || (PLATFORM(QT) && USE(QTKIT))

Is Qt wrong here too?

[Catalin badea]

Thank you for the review.

(In reply to comment #3)
> (From update of attachment 162501 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162501&action=review
> 
> > Source/WebKit/chromium/public/mac/WebSubstringUtil.h:34
> > +#include "../platform/WebCommon.h"
> 
> This seems like an odd way to include a header?  Are you sure this is proper form?  Maybe <platform/WebCommon.h> ?

I've followed the headers in WebKit/chromium/public/mac/ as examples.

> > Source/WebKit/chromium/src/mac/WebSubstringUtil.mm:35
> > +#import <Cocoa/Cocoa.h>
> 
> Why the re-org?

Cocoa/Cocoa.h needs to be included first in order for the SGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES macro to be defined
before the contents of FloatSize.h & similar headers are being preprocessed ensuring that no extra constructors will be declared.
This also complies with header include ordering.

[Eric Seidel (no email)]

Comment on attachment 162501 [details]
Patch fix.

Seems reasonable.

[WebKit Review Bot]

Comment on attachment 162501 [details]
Patch fix.

Clearing flags on attachment: 162501

Committed r128085: <http://trac.webkit.org/changeset/128085>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.