[Suggestions] Introduce a different image encoder/decoder for iOS

[Suggestions] Introduce a different image encoder/decoder for iOS

This introduces the new files and dependencies (not used yet). A further improvement will be to switch to use gfx::Image.

TBR=senorblanco
BUG=409156
Committed: https://crrev.com/a27b85e6387d5731c9a42312fbeea99891915ecd
Cr-Commit-Position: refs/heads/master@{#299158}

[Mathieu, 2014-10-09 20:39:23]

Patchset #1 (id:1) has been deleted

[Mathieu, 2014-10-09 20:48:40]

mathp@chromium.org changed reviewers:
+ blundell@chromium.org, justincohen@chromium.org

[Mathieu, 2014-10-09 20:48:52]

Hi guys, 

Can you take a look?

[justincohen, 2014-10-09 21:11:26]

lgtm

[blundell, 2014-10-10 06:01:05]

LGTM

as an optional nit, I think it would work well from a code cleanliness POV to
have the following:

image_encoder.h with Encode() and Decode() functions (note: not virtual
interfaces)
image_encoder.cc that implements these functions using ::gfx
image_encoder_ios.mm that implements these functions using the system interface

then exclude image_encoder.cc from being built on iOS

I'm not insisting on this since I've caused thrashing here already :\, just
something to consider.

https://codereview.chromium.org/641513003/diff/110001/components/suggestions/...
File components/suggestions/image_manager.cc (right):

https://codereview.chromium.org/641513003/diff/110001/components/suggestions/...
components/suggestions/image_manager.cc:23: return
suggestions::DecodeJPEGToSkBitmap(encoded_data);
nit: this should be in the suggestions namespace, and then this can go.

https://codereview.chromium.org/641513003/diff/110001/components/suggestions/...
components/suggestions/image_manager.cc:228: return
suggestions::EncodeSkBitmapToJPEG(bitmap, dest);
this is in the suggestions namespace.

[Mathieu, 2014-10-10 18:06:10]

Thanks! Implemented your suggestion.

https://codereview.chromium.org/641513003/diff/110001/components/suggestions/...
File components/suggestions/image_manager.cc (right):

https://codereview.chromium.org/641513003/diff/110001/components/suggestions/...
components/suggestions/image_manager.cc:23: return
suggestions::DecodeJPEGToSkBitmap(encoded_data);
On 2014/10/10 06:01:05, blundell wrote:
> nit: this should be in the suggestions namespace, and then this can go.

Done.

https://codereview.chromium.org/641513003/diff/110001/components/suggestions/...
components/suggestions/image_manager.cc:228: return
suggestions::EncodeSkBitmapToJPEG(bitmap, dest);
On 2014/10/10 06:01:05, blundell wrote:
> this is in the suggestions namespace.

Done.

[Mathieu, 2014-10-10 18:06:14]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2014-10-10 18:07:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/641513003/170001

[commit-bot: I haz the power, 2014-10-10 18:15:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-10 18:15:03]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Mathieu, 2014-10-10 18:18:53]

mathp@chromium.org changed reviewers:
+ senorblanco@google.com

[Mathieu, 2014-10-10 18:19:23]

TBR senorblanco because of silly presubmit asking for owners of added DEPS.

[Mathieu, 2014-10-10 18:19:28]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2014-10-10 18:20:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/641513003/170001

[Stephen White, 2014-10-10 18:23:18]

senorblanco@chromium.org changed reviewers:
+ senorblanco@chromium.org

[Stephen White, 2014-10-10 18:23:19]

rsLGTM

[commit-bot: I haz the power, 2014-10-10 19:20:05]

Committed patchset #3 (id:170001)

[commit-bot: I haz the power, 2014-10-10 19:21:45]

Patchset 3 (id:??) landed as
https://crrev.com/a27b85e6387d5731c9a42312fbeea99891915ecd
Cr-Commit-Position: refs/heads/master@{#299158}

[blundell, 2014-10-10 19:28:42]

Awesome! I really appreciate your work here.

https://codereview.chromium.org/641513003/diff/170001/components/suggestions/...
File components/suggestions/image_encoder_ios.h (right):

https://codereview.chromium.org/641513003/diff/170001/components/suggestions/...
components/suggestions/image_encoder_ios.h:14: // From encoded bytes to
SkBitmap. It's the caller's responsibility to delete
You actually don't even need this; just include image_encoder.h in
image_encoder_ios.mm.

[Stephen White, 2014-10-10 19:32:36]

https://codereview.chromium.org/641513003/diff/170001/components/suggestions/...
File components/suggestions/image_encoder_ios.h (right):

https://codereview.chromium.org/641513003/diff/170001/components/suggestions/...
components/suggestions/image_encoder_ios.h:16: SkBitmap*
DecodeJPEGToSkBitmap(const std::vector<unsigned char>& encoded_data);
BTW, you can actually just return an SkBitmap by value; it refcounts its
SkPixelRef data internally, so copying it is relatively cheap.

[Mathieu, 2014-10-10 19:36:52]

On 2014/10/10 19:32:36, Stephen White wrote:
>
https://codereview.chromium.org/641513003/diff/170001/components/suggestions/...
> File components/suggestions/image_encoder_ios.h (right):
> 
>
https://codereview.chromium.org/641513003/diff/170001/components/suggestions/...
> components/suggestions/image_encoder_ios.h:16: SkBitmap*
> DecodeJPEGToSkBitmap(const std::vector<unsigned char>& encoded_data);
> BTW, you can actually just return an SkBitmap by value; it refcounts its
> SkPixelRef data internally, so copying it is relatively cheap.

Thanks! Next CL will modify the interface to use gfx::Image so this may change.

Colin: I'm removing the header in a followup CL.