Use libjpeg-turbo by default....

Use libjpeg-turbo by default.
This change sets the 'use_libjpeg_turbo' flag to 1 so Chrome can use libjpeg-turbo by default on Win, Mac, and Linux.

BUG=48789
TEST=build Chromium without errors and pass all existing tests.

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

[Hironori Bono, 2011-01-31 07:48:13]

Greetings,

I would like to use libjpeg-turbo instead of libjpeg on all platforms and see
whether it causes compatibility problems. Even though I have verified it works
good on all platforms, I will revert it when it causes compatibility problems on
Chromium.

Regards,

Hironori Bono

[evanm, 2011-01-31 16:59:05]

LGTM on the Linux side.  May actually fix a longstanding bug of ours.

I wonder if color conversion is an issue on the Mac?

[abarth-chromium, 2011-01-31 17:05:05]

> I wonder if color conversion is an issue on the Mac?

Does this change require re-baselining any LayoutTest image results?  If so,
we'll need to do that carefully to make sure we don't regress various things,
including color correction.

[Mark Mentovai, 2011-01-31 17:09:56]

Color correction shouldn’t be any different with this change.

Still needs work.

http://codereview.chromium.org/6332003/diff/2001/chrome/app/framework.order
File chrome/app/framework.order (right):

http://codereview.chromium.org/6332003/diff/2001/chrome/app/framework.order#n...
chrome/app/framework.order:67: # ASM functions in libjpeg-turbo.
Nope. These shouldn’t be exported at all. Take a look at the build to figure out
why they’re being exported, and reconfigure it properly so that these won’t leak
out.

[Mark Mentovai, 2011-01-31 17:15:47]

If the symbols in question come from assembly source as the comment indicates,
the correct fix is to decorate the symbols in assembly with .private_extern
directives. For example:

.globl _my_asm_func
	.private_extern _my_asm_func
_my_asm_func:
	push %ebp
	mov %esp, %ebp
	...

If needed and depending on how things are set up, you can wrap .private_extern
in some macro goodness. On Linux, the correct directive would be .hidden instead
of .private_extern.

[Craig, 2011-01-31 17:40:42]

There's one change on the stable branch that looks useful that we may want to
pull into our copy:

$ svn diff -r323:324
https://libjpeg-turbo.svn.sourceforge.net/svnroot/libjpeg-turbo/branches/1.0.x

Index: jdhuff.c
===================================================================
--- jdhuff.c	(revision 323)
+++ jdhuff.c	(revision 324)
@@ -658,7 +658,7 @@
       symbol |= GET_BITS(1); \
       size++; \
     } \
-    symbol = htbl->pub->huffval[ (int) (symbol + htbl->valoffset[size]) ]; \
+    symbol = htbl->pub->huffval[ (int) (symbol + htbl->valoffset[size]) & 0xFF
]; \
   } \
 }
 
Index: ChangeLog.txt
===================================================================
--- ChangeLog.txt	(revision 323)
+++ ChangeLog.txt	(revision 324)
@@ -1,3 +1,9 @@
+Significant changes since 1.0.1
+===============================
+
+[1] Added further protections against invalid Huffman codes.
+
+
 Significant changes since 1.0.0
 ===============================

[Hironori Bono, 2011-02-01 06:47:18]

Greetings Mark,

Thank you for your comments.
This is exactly that I have been struggling with and would like to
hear your opinions. Unfortunately, the current yasm cannot set the
N_PEXT bit of Mach-O object files. Even though I have written a yasm
change that adds the N_PEXT-bit support to yasm, I'm wondering what
is the best option and would like to hear opinions of Mac experts.

(*1) http://codereview.chromium.org/6347060/

Since I have also filed a yasm ticket #236 (*2) and upload this
change, I wish yasm supports the bit soon. :)

(*2) <http://www.tortall.net/projects/yasm/ticket/236>

On 2011/01/31 17:15:47, Mark Mentovai wrote:
> If the symbols in question come from assembly source as the comment
> indicates, the correct fix is to decorate the symbols in assembly with
> .private_extern directives. For example:
> 
> .globl _my_asm_func
> 	.private_extern _my_asm_func
> _my_asm_func:
> 	push %ebp
> 	mov %esp, %ebp
> 	...
> 
> If needed and depending on how things are set up, you can wrap
> .private_extern in some macro goodness. On Linux, the correct
> directive would be .hidden instead of .private_extern.

Sure. When the above yasm issue is fixed, I will change our
libjpeg-turbo to add 'private_extern' attributes for Mac or
'hidden' attributes for Linux.

Regards,

Hironori Bono

[Mark Mentovai, 2011-02-01 15:17:32]

Since we build yasm from source, and our checked-in copy is patched already
anyway, why not just make your change in our patched version so that we can set
the private-extern bit now?

[Hironori Bono, 2011-02-02 07:04:36]

Greetings,

On 2011/02/01 15:17:32, Mark Mentovai wrote:
> Since we build yasm from source, and our checked-in copy is patched
> already anyway, why not just make your change in our patched
> version so that we can set the private-extern bit now?

Thank you for your suggestion. Yes, I should have sent its review request. I'm a
little wondering who is the best reviewer for this change, though. (fbarchard?)

Regards,

Hironori Bono

[Mark Mentovai, 2011-02-02 15:08:14]

hbono@chromium.org wrote:
> Thank you for your suggestion. Yes, I should have sent its review request.
> I'm a little wondering who is the best reviewer for this change, though.
> (fbarchard?)

ajwong or fbarchard seem like good choices. You can also send it to me.

[Craig, 2011-02-03 17:56:59]

http://codereview.chromium.org/6332003/diff/2001/DEPS
File DEPS (right):

http://codereview.chromium.org/6332003/diff/2001/DEPS#newcode182
DEPS:182: "/trunk/deps/third_party/libjpeg_turbo@71642",
Can you change this to 73588 please to pick up the invalid huffman code fix too
please.

[Hironori Bono, 2011-02-04 08:05:41]

Greetings Craig,

On 2011/02/03 17:56:59, Craig wrote:

> Can you change this to 73588 please to pick up the invalid huffman code fix
too
> please.

Thank you for your notice. As discussed earilier in this thread, I need another
change to libjpeg-turbo to avoid a change to the 'framework.order' file. (It is
<http://codereview.chromium.org/6246098/>.) I will change this DEPS file after
landing my change to pull both your huffman-code fix and my libjpeg-turbo
change.

Regards,

Hironori Bono

[Hironori Bono, 2011-02-07 10:40:51]

Greetings,

Thank you all for your help and advices in advance.
I have landed a few changes and we can now build libjpeg-turbo without changing
the framework.order file. Would it be possible to review the updated one?

Regards,

Hironori Bono

[Mark Mentovai, 2011-02-07 14:58:04]

LGTM!

[Craig, 2011-02-07 15:47:44]

LGTM2