Convert the utility process image decoder into a Mojo service.

Convert the utility process image decoder into a Mojo service.

BUG=597124
Committed: https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d
Cr-Commit-Position: refs/heads/master@{#385278}

[Anand Mistry (off Chromium), 2016-03-31 03:10:34]

amistry@chromium.org changed reviewers:
+ jam@chromium.org

[Anand Mistry (off Chromium), 2016-03-31 03:10:34]

This is a resurrection of my old image decoder change which never got submitted
due to undebuggable android bot issues.

So, this time around, I've just done the IPC->Mojo conversion and won't bother
moving things to //components.

[jam, 2016-03-31 17:46:33]

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decode...
File chrome/browser/image_decoder.cc (right):

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decode...
chrome/browser/image_decoder.cc:191: utility_process_host_.reset();
(this already existed)
should also delete utility_process_host_.get() to avoid a leak right?

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decoder.h
File chrome/browser/image_decoder.h (right):

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decode...
chrome/browser/image_decoder.h:169:
scoped_ptr<mojo::InterfacePtr<mojom::ImageDecoder>> decoder_;
why is this in a scoped ptr instead of having the raw interfaceptr as we usually
do?

[Anand Mistry (off Chromium), 2016-04-01 05:35:11]

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decode...
File chrome/browser/image_decoder.cc (right):

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decode...
chrome/browser/image_decoder.cc:191: utility_process_host_.reset();
On 2016/03/31 17:46:33, jam wrote:
> (this already existed)
> should also delete utility_process_host_.get() to avoid a leak right?

Oops, yeah, this should be a delete (the reset would be redundant). Note, this
works around crbug.com/431906.

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decoder.h
File chrome/browser/image_decoder.h (right):

https://codereview.chromium.org/1844103004/diff/1/chrome/browser/image_decode...
chrome/browser/image_decoder.h:169:
scoped_ptr<mojo::InterfacePtr<mojom::ImageDecoder>> decoder_;
On 2016/03/31 17:46:33, jam wrote:
> why is this in a scoped ptr instead of having the raw interfaceptr as we
usually
> do? 

Oops. Artifact of the previous (in components) incarnation where I wanted to
avoid exposing mojo. Not necessary any more.

[jam, 2016-04-04 17:30:05]

lgtm

[Anand Mistry (off Chromium), 2016-04-05 01:44:18]

amistry@chromium.org changed reviewers:
+ jorgelo@chromium.org

[Anand Mistry (off Chromium), 2016-04-05 01:44:19]

jorgelo@chromium.org: chrome_utility_messages

[Jorge Lucangeli Obes, 2016-04-05 03:59:09]

https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
File chrome/common/image_decoder.mojom (right):

https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
chrome/common/image_decoder.mojom:12: ROBUST_PNG,
How do you restrict these to being Chrome OS only?

[Anand Mistry (off Chromium), 2016-04-05 04:29:41]

https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
File chrome/common/image_decoder.mojom (right):

https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
chrome/common/image_decoder.mojom:12: ROBUST_PNG,
On 2016/04/05 03:59:09, Jorge Lucangeli Obes wrote:
> How do you restrict these to being Chrome OS only?

They're #if defined restricted in the implementation, on both ends. We don't
have conditional compilation in the Mojo bindings generator.

[Jorge Lucangeli Obes, 2016-04-05 04:33:40]

On 2016/04/05 04:29:41, Anand Mistry wrote:
>
https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
> File chrome/common/image_decoder.mojom (right):
> 
>
https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
> chrome/common/image_decoder.mojom:12: ROBUST_PNG,
> On 2016/04/05 03:59:09, Jorge Lucangeli Obes wrote:
> > How do you restrict these to being Chrome OS only?
> 
> They're #if defined restricted in the implementation, on both ends. We don't
> have conditional compilation in the Mojo bindings generator.

But that means a renderer can send a message with an unsupported codec?

[Jorge Lucangeli Obes, 2016-04-05 04:33:51]

https://codereview.chromium.org/1844103004/diff/20001/chrome/utility/image_de...
File chrome/utility/image_decoder_impl.cc (right):

https://codereview.chromium.org/1844103004/diff/20001/chrome/utility/image_de...
chrome/utility/image_decoder_impl.cc:51: SkBitmap decoded_image;
Will this be NULL when a renderer on non-Chrome OS sends a message using the
codecs that are not defined?

[Anand Mistry (off Chromium), 2016-04-05 05:15:17]

On 2016/04/05 04:33:40, Jorge Lucangeli Obes wrote:
> On 2016/04/05 04:29:41, Anand Mistry wrote:
> >
>
https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
> > File chrome/common/image_decoder.mojom (right):
> > 
> >
>
https://codereview.chromium.org/1844103004/diff/20001/chrome/common/image_dec...
> > chrome/common/image_decoder.mojom:12: ROBUST_PNG,
> > On 2016/04/05 03:59:09, Jorge Lucangeli Obes wrote:
> > > How do you restrict these to being Chrome OS only?
> > 
> > They're #if defined restricted in the implementation, on both ends. We don't
> > have conditional compilation in the Mojo bindings generator.
> 
> But that means a renderer can send a message with an unsupported codec?


If you were using the service directly, sure. But the service implementation
guards against the usage and returns a null image.

[Anand Mistry (off Chromium), 2016-04-05 05:15:27]

https://codereview.chromium.org/1844103004/diff/20001/chrome/utility/image_de...
File chrome/utility/image_decoder_impl.cc (right):

https://codereview.chromium.org/1844103004/diff/20001/chrome/utility/image_de...
chrome/utility/image_decoder_impl.cc:51: SkBitmap decoded_image;
On 2016/04/05 04:33:51, Jorge Lucangeli Obes wrote:
> Will this be NULL when a renderer on non-Chrome OS sends a message using the
> codecs that are not defined?

Firstly, renderers don't send these messages, the browser process does. But, if
somehow this does get a request for an unsupported codec (robust jpeg on
non-chromeos), then yes, this (a default constructed SkBitmap) will be null.

[Jorge Lucangeli Obes, 2016-04-05 16:39:55]

chrome_utility_messages.h LGTM

[Anand Mistry (off Chromium), 2016-04-05 19:57:35]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2016-04-05 19:57:36]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/1844103004/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-04-05 19:57:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1844103004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1844103004/40001

[commit-bot: I haz the power, 2016-04-05 20:42:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-05 20:43:48]

Description was changed from

==========
Convert the utility process image decoder into a Mojo service.

BUG=597124
==========

to

==========
Convert the utility process image decoder into a Mojo service.

BUG=597124

Committed: https://crrev.com/6f72f04ebdeb6d7d3344b3cb985a9d9f4390df2d
Cr-Commit-Position: refs/heads/master@{#385278}
==========

[commit-bot: I haz the power, 2016-04-05 20:43:49]

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