JPEG(JFIF only) directly embedded into PDF

JPEG(JFIF only) directly embedded into PDF

Committed: https://skia.googlesource.com/skia/+/daefa5b340d7aa36fe31865a3646f6fce321bb38

[hal.canary, 2014-08-27 17:39:30]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-08-27 17:39:52]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/halcanary@google.com/515493003/20001

[commit-bot: I haz the power, 2014-08-27 17:39:53]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from a valid reviewer till 2014-08-27 23:39 UTC

[commit-bot: I haz the power, 2014-08-27 17:41:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Build-Mac10.8-Clang-x86_64-Release-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Mac10.8-Clang-x86_64-Release-Trybo...)
  Build-Ubuntu13.10-Clang-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-Clang-x86_64-Debug-Try...)
  Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Android-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-GCC4.8-Arm7-Debug-Andr...)
  Build-Ubuntu13.10-GCC4.8-x86_64-Release-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Ubuntu13.10-GCC4.8-x86_64-Release-...)
  Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug...)
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-Shared-Trybot on tryserver.skia
(http://108.170.220.120:10117/builders/Test-Ubuntu13.10-GCE-NoGPU-x86_64-Relea...)

[hal.canary, 2014-08-27 17:42:03]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2014-08-27 17:46:39]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-08-27 17:47:42]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/halcanary@google.com/515493003/40001

[commit-bot: I haz the power, 2014-08-27 17:47:43]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from a valid reviewer till 2014-08-27 23:47 UTC

[hal.canary, 2014-08-27 18:14:14]

The CQ bit was unchecked by halcanary@google.com

[reed1, 2014-08-27 18:50:40]

reed@google.com changed reviewers:
+ reed@google.com

[reed1, 2014-08-27 18:50:40]

Might add simple comments over some of the help functions.

e.g. // returns true iff the encoded data already in the Bitmap is JPEG format.,
else we have to re-encode it from its raw (decoded) pixels.

Do we have tests/benches that exercise this?

[hal.canary, 2014-08-27 18:51:56]

On 2014/08/27 18:50:40, reed1 wrote:
> Might add simple comments over some of the help functions.

I'm cleaning up the code.

> e.g. // returns true iff the encoded data already in the Bitmap is JPEG
format.,
> else we have to re-encode it from its raw (decoded) pixels.
> 
> Do we have tests/benches that exercise this?

There's a unit test in the CL.

[hal.canary, 2014-08-27 19:44:12]

Patchset #4 (id:60001) has been deleted

[hal.canary, 2014-08-27 19:44:48]

halcanary@google.com changed reviewers:
+ djsollen@google.com, mtklein@google.com

[hal.canary, 2014-08-27 19:44:48]

PTAL

[reed1, 2014-08-27 19:46:49]

lgtm w/ nits in the test file.

https://codereview.chromium.org/515493003/diff/80001/tests/PDFJpegEmbedTest.cpp
File tests/PDFJpegEmbedTest.cpp (right):

https://codereview.chromium.org/515493003/diff/80001/tests/PDFJpegEmbedTest.c...
tests/PDFJpegEmbedTest.cpp:19: static inline SkBitmap bitmap_from_data(SkData*
data) {
nit: don't neet "inline", esp. in a test file.

https://codereview.chromium.org/515493003/diff/80001/tests/PDFJpegEmbedTest.c...
tests/PDFJpegEmbedTest.cpp:44: SkData* load_resource(
static ?

[hal.canary, 2014-08-27 19:51:17]

https://codereview.chromium.org/515493003/diff/80001/tests/PDFJpegEmbedTest.cpp
File tests/PDFJpegEmbedTest.cpp (right):

https://codereview.chromium.org/515493003/diff/80001/tests/PDFJpegEmbedTest.c...
tests/PDFJpegEmbedTest.cpp:19: static inline SkBitmap bitmap_from_data(SkData*
data) {
On 2014/08/27 19:46:48, reed1 wrote:
> nit: don't neet "inline", esp. in a test file.

Done.

https://codereview.chromium.org/515493003/diff/80001/tests/PDFJpegEmbedTest.c...
tests/PDFJpegEmbedTest.cpp:44: SkData* load_resource(
On 2014/08/27 19:46:48, reed1 wrote:
> static ?

Done.

[hal.canary, 2014-08-27 19:51:20]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-08-27 19:52:19]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/halcanary@google.com/515493003/100001

[commit-bot: I haz the power, 2014-08-27 20:00:59]

Committed patchset #5 (id:100001) as daefa5b340d7aa36fe31865a3646f6fce321bb38