BlobChannelService is modified to be Mojo Service.

BlobChannelService is modified to be Mojo Service.

BlobChannelService is made to be a Mojo Service, and now keeps a 1:many
relationship between itself and requests.

BUG=621202
Committed: https://crrev.com/7a2db3214e124e79496ff3d7123b7d9813694532
Cr-Commit-Position: refs/heads/master@{#412374}

[CJ, 2016-07-27 22:40:28]

lethalantidote@chromium.org changed reviewers:
+ kmarshall@chromium.org

[Kevin M, 2016-07-28 18:14:47]

Just some minor comments.

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
File blimp/engine/mojo/blob_channel_service.cc (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
blimp/engine/mojo/blob_channel_service.cc:64: // Object lifetime is managed by
BlobChannelService's BindingSet
This isn't the case anymore?

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
File blimp/engine/mojo/blob_channel_service.h (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
blimp/engine/mojo/blob_channel_service.h:29: // Binds BlobChannel to the
connection specified by |request|.
nit: "Binds |this|..." because the term BlobChannel is vague

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/renderer/b...
File blimp/engine/renderer/blob_channel_sender_proxy_unittest.cc (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/renderer/b...
blimp/engine/renderer/blob_channel_sender_proxy_unittest.cc:31: :
mojo_service_impl_(
base::MakeUnique<BlobChannelService>(&mock_sender_)

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
File blimp/engine/session/blimp_engine_session.cc (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.cc:245: blob_channel_service_ =
Switch this to MakeUnique

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.cc:303: EngineGeolocationFeature*
BlimpEngineSession::GetGeolocationFeature() {
??

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
File blimp/engine/session/blimp_engine_session.h (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.h:104: EngineGeolocationFeature*
GetGeolocationFeature();
What's this doing here? :)

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.h:211: // Receives image data sends it
to the client via
Doesn't parse?

[CJ, 2016-07-28 19:54:15]

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
File blimp/engine/mojo/blob_channel_service.cc (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
blimp/engine/mojo/blob_channel_service.cc:64: // Object lifetime is managed by
BlobChannelService's BindingSet
On 2016/07/28 18:14:47, Kevin M wrote:
> This isn't the case anymore?

Done.

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
File blimp/engine/mojo/blob_channel_service.h (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/mojo/blob_...
blimp/engine/mojo/blob_channel_service.h:29: // Binds BlobChannel to the
connection specified by |request|.
On 2016/07/28 18:14:47, Kevin M wrote:
> nit: "Binds |this|..." because the term BlobChannel is vague

Done.

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/renderer/b...
File blimp/engine/renderer/blob_channel_sender_proxy_unittest.cc (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/renderer/b...
blimp/engine/renderer/blob_channel_sender_proxy_unittest.cc:31: :
mojo_service_impl_(
On 2016/07/28 18:14:47, Kevin M wrote:
> base::MakeUnique<BlobChannelService>(&mock_sender_)

Cool!

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
File blimp/engine/session/blimp_engine_session.cc (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.cc:245: blob_channel_service_ =
On 2016/07/28 18:14:47, Kevin M wrote:
> Switch this to MakeUnique

Done.

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.cc:303: EngineGeolocationFeature*
BlimpEngineSession::GetGeolocationFeature() {
On 2016/07/28 18:14:47, Kevin M wrote:
> ??

I think I screwed up merging. Fixing.

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
File blimp/engine/session/blimp_engine_session.h (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.h:104: EngineGeolocationFeature*
GetGeolocationFeature();
On 2016/07/28 18:14:47, Kevin M wrote:
> What's this doing here? :)

I screwed up merging I think. Fixing.

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.h:211: // Receives image data sends it
to the client via
On 2016/07/28 18:14:47, Kevin M wrote:
> Doesn't parse?

Not sure what you mean. Are you saying that I somehow indicated that it does
parse, or that I left out parsing?

[Kevin M, 2016-07-28 20:59:17]

lgtm

Make sure this runs correctly on the device etc. before submitting!

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
File blimp/engine/session/blimp_engine_session.h (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.h:211: // Receives image data sends it
to the client via
On 2016/07/28 19:54:15, CJ wrote:
> On 2016/07/28 18:14:47, Kevin M wrote:
> > Doesn't parse?
> 
> Not sure what you mean. Are you saying that I somehow indicated that it does
> parse, or that I left out parsing?

As in the sentence has two disjoint phrases that don't form a sentence. :)

https://codereview.chromium.org/2189503004/diff/40001/blimp/engine/mojo/blob_...
File blimp/engine/mojo/blob_channel_service.h (right):

https://codereview.chromium.org/2189503004/diff/40001/blimp/engine/mojo/blob_...
blimp/engine/mojo/blob_channel_service.h:41: // Binds |this| and its object
lifetime to a set of Mojo connections.
The lifetime is not bound to BindingSet at all?

[CJ, 2016-07-28 22:47:05]

Would running blimp on my device and searching for a picture work to test this?

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
File blimp/engine/session/blimp_engine_session.h (right):

https://codereview.chromium.org/2189503004/diff/20001/blimp/engine/session/bl...
blimp/engine/session/blimp_engine_session.h:211: // Receives image data sends it
to the client via
On 2016/07/28 20:59:17, Kevin M wrote:
> On 2016/07/28 19:54:15, CJ wrote:
> > On 2016/07/28 18:14:47, Kevin M wrote:
> > > Doesn't parse?
> > 
> > Not sure what you mean. Are you saying that I somehow indicated that it does
> > parse, or that I left out parsing?
> 
> As in the sentence has two disjoint phrases that don't form a sentence. :)

Done.

https://codereview.chromium.org/2189503004/diff/40001/blimp/engine/mojo/blob_...
File blimp/engine/mojo/blob_channel_service.h (right):

https://codereview.chromium.org/2189503004/diff/40001/blimp/engine/mojo/blob_...
blimp/engine/mojo/blob_channel_service.h:41: // Binds |this| and its object
lifetime to a set of Mojo connections.
On 2016/07/28 20:59:17, Kevin M wrote:
> The lifetime is not bound to BindingSet at all?

Done.

[Kevin M, 2016-07-28 22:58:21]

You'll need to run the engine locally, as this is an engine-specific change.

[CJ, 2016-08-01 20:58:18]

The CQ bit was checked by lethalantidote@chromium.org

[CJ, 2016-08-01 20:58:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
kmarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/2189503004/#ps60001
(title: "Addresses kmarshall's #5 comments")

[commit-bot: I haz the power, 2016-08-01 20:58:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[CJ, 2016-08-01 21:00:56]

The CQ bit was unchecked by lethalantidote@chromium.org

[CJ, 2016-08-02 21:14:23]

The CQ bit was checked by lethalantidote@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 21:14:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 21:18:34]

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

[commit-bot: I haz the power, 2016-08-02 21:18:35]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[CJ, 2016-08-02 22:36:53]

The CQ bit was checked by lethalantidote@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 22:37:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 22:43:16]

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

[commit-bot: I haz the power, 2016-08-02 22:43:17]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[CJ, 2016-08-02 23:09:31]

The CQ bit was checked by lethalantidote@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 23:10:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 00:51:35]

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

[commit-bot: I haz the power, 2016-08-03 00:51:36]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[CJ, 2016-08-16 22:19:00]

The CQ bit was checked by lethalantidote@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 22:19:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 23:24:03]

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

[commit-bot: I haz the power, 2016-08-16 23:24:04]

Dry run: This issue passed the CQ dry run.

[CJ, 2016-08-16 23:26:57]

The CQ bit was checked by lethalantidote@chromium.org

[CJ, 2016-08-16 23:26:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
kmarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/2189503004/#ps120001
(title: "Merge branch 'master' into bcs")

[commit-bot: I haz the power, 2016-08-16 23:27:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 23:34:21]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-08-16 23:37:29]

Description was changed from

==========
BlobChannelService is modified to be Mojo Service.

BlobChannelService is made to be a Mojo Service, and now keeps a 1:many
relationship between itself and requests.

BUG=621202
==========

to

==========
BlobChannelService is modified to be Mojo Service.

BlobChannelService is made to be a Mojo Service, and now keeps a 1:many
relationship between itself and requests.

BUG=621202

Committed: https://crrev.com/7a2db3214e124e79496ff3d7123b7d9813694532
Cr-Commit-Position: refs/heads/master@{#412374}
==========

[commit-bot: I haz the power, 2016-08-16 23:37:30]

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