Integrates libjpeg-turbo into Chromium (Chromium side)...

Integrates libjpeg-turbo into Chromium (Chromium side)
This change is the Chromium-side change that integrates libjpeg-turbo into Chromium. (We need another change for WebKit.) It adds a GYP option 'libjpeg_turbo' so we can switch from libjpeg to libjpeg-turbo and use libjpeg-turbo when its value is 1. (Unfortunately, its value must be 0 for now to avoid build breaks until we land all changes required for libjpeg-turbo.)

BUG=48789
TEST=build Chromium.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=68453

[Mark Mentovai, 2010-11-29 19:14:17]

http://codereview.chromium.org/5292007/diff/3001/app/app.gyp
File app/app.gyp (right):

http://codereview.chromium.org/5292007/diff/3001/app/app.gyp#newcode91
app/app.gyp:91: ['libjpeg_turbo==1', {
I wonder if we could make this more abstract instead of having to write it in
each .gyp file that wants libjpeg.

Can we define a variable in common.gypi named libjpeg_gyp_path and have it point
to libjpeg.gyp in either libjpeg or libjpeg_turbo?

http://codereview.chromium.org/5292007/diff/3001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/5292007/diff/3001/build/common.gypi#newcode175
build/common.gypi:175: 'libjpeg_turbo%': '<(libjpeg_turbo)',
You may not need the variables-in-variables hack for the libjpeg_turbo.

Well, you don’t need it in this particular changelist as it stands right now.
You should be able to get away with getting rid of the definition of
libjpeg_turbo above and just making this one 'libjpeg_turbo%': 0. However, if
you can successfully implement my other suggestion about putting a
libjpeg_gyp_path variable into this file, you most likely will need to keep this
variables-in-variables hack.

[Mark Mentovai, 2010-11-30 15:06:01]

This will be much cleaner.

http://codereview.chromium.org/5292007/diff/17001/app/app.gyp
File app/app.gyp (right):

http://codereview.chromium.org/5292007/diff/17001/app/app.gyp#newcode36
app/app.gyp:36: '../third_party/<(libjpeg_gyp_path):libjpeg',
Do you need for this to be ../third_party/<(libjpeg_gyp_path):libjpeg?

You should be able to get away with <(libjpeg_gyp_path):libjpeg, if you define
libjpeg_gyp_path appropriately.

http://codereview.chromium.org/5292007/diff/17001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/5292007/diff/17001/build/common.gypi#newcode143
build/common.gypi:143: # The relative path from 'src/third_party' to the GYP
file of the JPEG
This doesn’t need to be relative to src/third_party. Since it ends in _path, it
ought to work to just be relative to this file (common.gypi). GYP will figure
the rest out.

http://codereview.chromium.org/5292007/diff/17001/build/common.gypi#newcode177
build/common.gypi:177: 'libjpeg_gyp_path%': '<(libjpeg_gyp_path)',
It still might be a good idea to have a condition to flip this to libjpeg-turbo
based on a use_libjpeg_turbo variable. I think it’s better to require the user
to set a variable to 1 than to require the user to set it to a specific path.

[Hironori Bono, 2010-12-03 09:32:16]

Greetings,

Thank you for your comments/suggestions.
I have updated this change to apply your comments. Is it possible to take
another look?

Regards,

Hironori Bono

http://codereview.chromium.org/5292007/diff/17001/app/app.gyp
File app/app.gyp (right):

http://codereview.chromium.org/5292007/diff/17001/app/app.gyp#newcode36
app/app.gyp:36: '../third_party/<(libjpeg_gyp_path):libjpeg',
On 2010/11/30 15:06:01, Mark Mentovai wrote:
> Do you need for this to be ../third_party/<(libjpeg_gyp_path):libjpeg?
> 
> You should be able to get away with <(libjpeg_gyp_path):libjpeg, if you define
> libjpeg_gyp_path appropriately.

Done.

http://codereview.chromium.org/5292007/diff/17001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/5292007/diff/17001/build/common.gypi#newcode143
build/common.gypi:143: # The relative path from 'src/third_party' to the GYP
file of the JPEG
On 2010/11/30 15:06:01, Mark Mentovai wrote:
> This doesn’t need to be relative to src/third_party.
> Since it ends in _path, it ought to work to just be
> relative to this file (common.gypi). GYP will figure
> the rest out.

Done. Thank you for noticing it. (Such comments are helpful because I'm a GYP
beginner.)

http://codereview.chromium.org/5292007/diff/17001/build/common.gypi#newcode177
build/common.gypi:177: 'libjpeg_gyp_path%': '<(libjpeg_gyp_path)',
On 2010/11/30 15:06:01, Mark Mentovai wrote:
> It still might be a good idea to have a condition to flip
> this to libjpeg-turbo based on a use_libjpeg_turbo
> variable. I think it’s better to require the user to set
> a variable to 1 than to require the user to set it to a
> specific path.

Done. I have added a 'use_libjpeg_turbo' variable and switch this
'libjpeg_gyp_path' according to the 'use_libjpeg_turbo' value. (I may be missing
something, though.)

[Mark Mentovai, 2010-12-03 16:18:09]

LGTM with these changes. Nice job.

http://codereview.chromium.org/5292007/diff/23001/app/app.gyp
File app/app.gyp (right):

http://codereview.chromium.org/5292007/diff/23001/app/app.gyp#newcode36
app/app.gyp:36: '<(libjpeg_gyp_path):libjpeg',
I’d just move these to the bottoms of their respective lists now, since they’re
no longer really sorted if you leave them where they are.

http://codereview.chromium.org/5292007/diff/23001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/5292007/diff/23001/build/common.gypi#newcode409
build/common.gypi:409: 'libjpeg_gyp_path%':
'../third_party/libjpeg_turbo/libjpeg.gyp',
These don’t need the % on them.

[Hironori Bono, 2010-12-07 07:28:27]

Greetings,

Thank you for your comments. I have applied your comments and landed the updated
one as r68453.

Regards,

Hironori Bono

http://codereview.chromium.org/5292007/diff/23001/app/app.gyp
File app/app.gyp (right):

http://codereview.chromium.org/5292007/diff/23001/app/app.gyp#newcode36
app/app.gyp:36: '<(libjpeg_gyp_path):libjpeg',
On 2010/12/03 16:18:09, Mark Mentovai wrote:
> I’d just move these to the bottoms of their respective lists now, since
they’re
> no longer really sorted if you leave them where they are.

Done, (including the other GYP files).

http://codereview.chromium.org/5292007/diff/23001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/5292007/diff/23001/build/common.gypi#newcode409
build/common.gypi:409: 'libjpeg_gyp_path%':
'../third_party/libjpeg_turbo/libjpeg.gyp',
On 2010/12/03 16:18:09, Mark Mentovai wrote:
> These don’t need the % on them.

Done. Thank you for noticing it.