Bug 95977

Summary:

webkit build for 64-bit Mac chromium

Product:

WebKit

Reporter:

Catalin badea <badea>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dglazkov, eric, fishd, jamesr, mark, mmaerean, ossy, thakis, tkent+wkapi, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.7

Attachments:

Description	Flags
Patch fix.	none

Catalin badea

Reported 2012-09-06 05:42:50 PDT

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

Attachments
Patch fix. (5.83 KB, patch) 2012-09-06 06:56 PDT, Catalin badea	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Catalin badea

Comment 1 2012-09-06 06:56:04 PDT

Created attachment 162501 [details] Patch fix.

WebKit Review Bot

Comment 2 2012-09-10 02:30:07 PDT

Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org 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 3 2012-09-10 09:35:43 PDT

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 4 2012-09-10 09:36:01 PDT

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

Comment 5 2012-09-10 10:07:47 PDT

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 6 2012-09-10 11:22:17 PDT

Comment on attachment 162501 [details] Patch fix. Seems reasonable.

WebKit Review Bot

Comment 7 2012-09-10 11:48:45 PDT

Comment on attachment 162501 [details] Patch fix. Clearing flags on attachment: 162501 Committed r128085: <http://trac.webkit.org/changeset/128085>

WebKit Review Bot

Comment 8 2012-09-10 11:48:49 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.