Bug 46316 – patch for FreeBSD support with Chromium

• Component: Platform
• Status: RESOLVED FIXED

Created attachment 68459 [details]
FreeBSD ifdefs

This patch adds further ifdefs so that FreeBSD can use Webkit for Chromium too.  It applies against webkit/trunk/WebKit/chromium in the svn repo.

[Tony Chang]

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

> features.gypi:-94
> -        ['OS=="win" or OS=="linux" or OS=="mac"', {
> -          'feature_defines': [
> -           'WTF_USE_ACCELERATED_COMPOSITING=1',
> -           'ENABLE_3D_RENDERING=1',
> -          ],

I think this is fine, but adding jamesr to be sure.

[James Robinson]

Comment on attachment 68459 [details]
FreeBSD ifdefs

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

> features.gypi:88
> +        'WTF_USE_ACCELERATED_COMPOSITING=1',
> +        'ENABLE_3D_RENDERING=1',

Alphabetize these, please.  Otherwise this move is fine.

Created attachment 68464 [details]
WebCore patch for FreeBSD

another patch, this time against WebCore: webkit/trunk/WebCore from the svn repo

[Tony Chang]

Comment on attachment 68464 [details]
WebCore patch for FreeBSD

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

> platform/Scrollbar.cpp:43
> -#if PLATFORM(CHROMIUM) && OS(LINUX) || PLATFORM(GTK)
> +#if PLATFORM(CHROMIUM) && (OS(LINUX) || OS(FREEBSD)) || PLATFORM(GTK)

Nit: Can you add () around (PLATFORM(CHROMIUM) && (OS(LINUX) || OS(FREEBSD))) so the order is clear?

[Tony Chang]

A couple workflow comments:
- Please open a separate bug per patch.
- Please set patches to review? if you want it to be reviewed.

Created attachment 68468 [details]
alphabetized the change to features.gypi

Created attachment 68469 [details]
WebCore patch for FreeBSD

I added the parentheses asked for.  I was originally going to open a separate ticket for the second patch, since these patches were generated from two separate checkouts in Chromium, but then I thought you might prefer them both together since they come from a common WebKit repo.

[Tony Chang]

I will commit these for you.

[Tony Chang]

Comment on attachment 68468 [details]
alphabetized the change to features.gypi

Sorry, I'm dumb.  These changes need a ChangeLog entry.  You can run prepare-ChangeLog or use webkit-patch upload to generate a ChangeLog.

Created attachment 68474 [details]
final WebCore patch with ChangeLog

Added a ChangeLog entry

Created attachment 68478 [details]
final WebKit/chromium patch for FreeBSD

Added to the ChangeLog

[Tony Chang]

cc'ing darin for guidance.

Ruben has requested that we don't include his full name in the ChangeLog.  Is it OK for me to commit the patch for him?

[Darin Adler]

(In reply to comment #12)
> Ruben has requested that we don't include his full name in the ChangeLog.  Is it OK for me to commit the patch for him?

It’s ugly but OK. If he says that’s the name he wants to go by I think we’re OK.

[James Robinson]

Committed r68112: <http://trac.webkit.org/changeset/68112>

[James Robinson]

Combined into a single patch, generated proper ChangeLogs and landed as http://trac.webkit.org/changeset/68112.