Implement a trivial Mojo EchoService that can be connected to over DBus

Implement a trivial Mojo EchoService that can be connected to over DBus

Some systems might like to use Mojo IPC to talk to services that are
running outside the auspices of the Mojo shell. DBus is one potential
channel to use for bootstrapping such a connection.

In order to allow the externally-running service to accept connections
from a Mojo shell, we need to get it a ShellHandle. This change
defines a DBusServiceLoader that implements the ServiceLoader
interface.  DBusServiceLoader creates a dedicated MessagePipe, passes
a handle to one end to the desired service over DBus, and then passes
the ShellHandle over that pipe.

This class assumes the following:
1) Your service is already running,
2) Your service implements the Mojo ExternalService API
   (from external_service.mojom).
3) Your service exports an object that implements the org.chromium.Mojo
   DBus interface:
   <interface name="org.chromium.Mojo">
     <method name="ConnectChannel">
       <arg type="h" name="file_descriptor" direction="in" />
     </method>
   </interface>

This change also includes a trivial DBusEchoService that can be
"loaded" using the DBusServiceLoader, as well as a mojo example app
that connects to this service.

BUG=364903
TEST=run mojo_dbus_echo_service in one shell, and then load mojo_dbus_echo in the mojo shell. "who" should successfully echo between the two.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=265927

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266508

[Chris Masone, 2014-04-18 18:26:27]

I still need to make all the dbus-using stuff conditionally compile only on
Linux, but I would very much appreciate feedback on my approach here.

There's a big comment in dbus_service_loader.h that explains in a bit more
detail than the commit message, and I'm happy to talk through this on chat, VC,
or in person when possible.

https://codereview.chromium.org/242763003/diff/10001/mojo/mojo.gyp
File mojo/mojo.gyp (right):

https://codereview.chromium.org/242763003/diff/10001/mojo/mojo.gyp#newcode430
mojo/mojo.gyp:430: '../build/linux/system.gyp:dbus',
I need to make all of this linux-specific stuff be conditional before landing
this.

https://codereview.chromium.org/242763003/diff/10001/mojo/services/dbus_echo/...
File mojo/services/dbus_echo/echo.mojom (right):

https://codereview.chromium.org/242763003/diff/10001/mojo/services/dbus_echo/...
mojo/services/dbus_echo/echo.mojom:5: module mojo {
I have no idea how git went and found this file that it assumes I moved and
changed, but I created this file anew.

[viettrungluu, 2014-04-18 19:40:15]

I haven't looked at it super-closely yet (which I should do), but what I've seen
so far looks good....

https://codereview.chromium.org/242763003/diff/10001/mojo/shell/context.h
File mojo/shell/context.h (right):

https://codereview.chromium.org/242763003/diff/10001/mojo/shell/context.h#new...
mojo/shell/context.h:50: scoped_ptr<DBusServiceLoader> dbus_service_loader_;
FYI: I believe Dave is working on making the ServiceManager just own the loaders
its given, so soon this won't be necessary.

(Which is good, since it'll make it easier/nicer to make this conditional on
being on Linux.)

[Chris Masone, 2014-04-22 03:20:54]

On 2014/04/18 19:40:15, viettrungluu wrote:
> I haven't looked at it super-closely yet (which I should do), but what I've
seen
> so far looks good....
> 
> https://codereview.chromium.org/242763003/diff/10001/mojo/shell/context.h
> File mojo/shell/context.h (right):
> 
>
https://codereview.chromium.org/242763003/diff/10001/mojo/shell/context.h#new...
> mojo/shell/context.h:50: scoped_ptr<DBusServiceLoader> dbus_service_loader_;
> FYI: I believe Dave is working on making the ServiceManager just own the
loaders
> its given, so soon this won't be necessary.
> 
> (Which is good, since it'll make it easier/nicer to make this conditional on
> being on Linux.)

I still have to rebase to take Dave's new change re: ServiceLoader ownership
into account, but that closer look at the approach I've taken here would be
incredibly helpful :) i can hold off on doing that rebase if you're looking now,
or post a new CL with the rebase before tomorrow AM.

[DaveMoore, 2014-04-22 20:39:51]

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp
File mojo/mojo.gyp (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp#newcode487
mojo/mojo.gyp:487: 'shell/dbus_service_loader.cc',
It's more common in gyp files to list all files in the target and then exclude
them when not required. For OS dependent ones you can do that automatically
calling this dbux_service_loader_linux.{h,cc}. You would leave the extra
dependencies here though.

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi
File mojo/mojo_examples.gypi (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi#...
mojo/mojo_examples.gypi:340: {
Since this doesn't run on android you can remove the package target.

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi
File mojo/mojo_services.gypi (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi#...
mojo/mojo_services.gypi:170: '../gin/gin.gyp:gin',
Is gin needed?

https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
File mojo/shell/external_service.mojom (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
mojo/shell/external_service.mojom:7: [Peer=ExternalServiceHost]
Why can't you just use the Shell / ShellClient interface here? Even if you need
to interpose yourself between the real shell and this client you could simply
proxy both sides. If you don't I think you can just hand the shell_handle off to
the dbus service, which would implement the ShellClient interface.

[Chris Masone, 2014-04-22 21:16:29]

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp
File mojo/mojo.gyp (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp#newcode487
mojo/mojo.gyp:487: 'shell/dbus_service_loader.cc',
On 2014/04/22 20:39:51, DaveMoore wrote:
> It's more common in gyp files to list all files in the target and then exclude
> them when not required. For OS dependent ones you can do that automatically
> calling this dbux_service_loader_linux.{h,cc}. You would leave the extra
> dependencies here though.

Done.

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi
File mojo/mojo_examples.gypi (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi#...
mojo/mojo_examples.gypi:340: {
On 2014/04/22 20:39:51, DaveMoore wrote:
> Since this doesn't run on android you can remove the package target.

Done.

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi
File mojo/mojo_services.gypi (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi#...
mojo/mojo_services.gypi:170: '../gin/gin.gyp:gin',
On 2014/04/22 20:39:51, DaveMoore wrote:
> Is gin needed?

Done.

https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
File mojo/shell/external_service.mojom (right):

https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
mojo/shell/external_service.mojom:7: [Peer=ExternalServiceHost]
On 2014/04/22 20:39:51, DaveMoore wrote:
> Why can't you just use the Shell / ShellClient interface here? Even if you
need
> to interpose yourself between the real shell and this client you could simply
> proxy both sides. If you don't I think you can just hand the shell_handle off
to
> the dbus service, which would implement the ShellClient interface.


This interface exists to allow me to hand the ShellHandle off to the dbus
service, because I couldn't figure out another way to do it :)

DBus provides a primitive that allows me to pass file descriptors across, and by
the time I'm in the DBusServiceLoader code, I have a ShellHandle and (as far as
I could tell, please correct if I'm wrong) no way to recover the FD of the
MessagePipe that underlies it.  So, instead I create a new FD pair and pass one
over DBus, which I use to get (what I believe is) a MessagePipe set up between
the two processes.

It's probably true that I could manually craft and send a message containing the
ShellHandle over the pipe I just created, if that'd be preferred. It seemed like
defining this little API just automatically handled marshalling and such for me,
so I did it this way.

I would prefer to have some way to get the ShellHandle directly to the
externally running process, without have to create a MessagePipe for this sole
purpose, but I couldn't figure out how to do so with the primitives I had.

[viettrungluu, 2014-04-23 16:44:41]

On 2014/04/22 21:16:29, Chris Masone wrote:
> https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp
> File mojo/mojo.gyp (right):
> 
> https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp#newcode487
> mojo/mojo.gyp:487: 'shell/dbus_service_loader.cc',
> On 2014/04/22 20:39:51, DaveMoore wrote:
> > It's more common in gyp files to list all files in the target and then
exclude
> > them when not required. For OS dependent ones you can do that automatically
> > calling this dbux_service_loader_linux.{h,cc}. You would leave the extra
> > dependencies here though.
> 
> Done.
> 
> https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi
> File mojo/mojo_examples.gypi (right):
> 
>
https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi#...
> mojo/mojo_examples.gypi:340: {
> On 2014/04/22 20:39:51, DaveMoore wrote:
> > Since this doesn't run on android you can remove the package target.
> 
> Done.
> 
> https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi
> File mojo/mojo_services.gypi (right):
> 
>
https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi#...
> mojo/mojo_services.gypi:170: '../gin/gin.gyp:gin',
> On 2014/04/22 20:39:51, DaveMoore wrote:
> > Is gin needed?
> 
> Done.
> 
>
https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
> File mojo/shell/external_service.mojom (right):
> 
>
https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
> mojo/shell/external_service.mojom:7: [Peer=ExternalServiceHost]
> On 2014/04/22 20:39:51, DaveMoore wrote:
> > Why can't you just use the Shell / ShellClient interface here? Even if you
> need
> > to interpose yourself between the real shell and this client you could
simply
> > proxy both sides. If you don't I think you can just hand the shell_handle
off
> to
> > the dbus service, which would implement the ShellClient interface.
> 
> 
> This interface exists to allow me to hand the ShellHandle off to the dbus
> service, because I couldn't figure out another way to do it :)
> 
> DBus provides a primitive that allows me to pass file descriptors across, and
by
> the time I'm in the DBusServiceLoader code, I have a ShellHandle and (as far
as
> I could tell, please correct if I'm wrong) no way to recover the FD of the
> MessagePipe that underlies it.  So, instead I create a new FD pair and pass
one
> over DBus, which I use to get (what I believe is) a MessagePipe set up between
> the two processes.
> 
> It's probably true that I could manually craft and send a message containing
the
> ShellHandle over the pipe I just created, if that'd be preferred. It seemed
like
> defining this little API just automatically handled marshalling and such for
me,
> so I did it this way.
> 
> I would prefer to have some way to get the ShellHandle directly to the
> externally running process, without have to create a MessagePipe for this sole
> purpose, but I couldn't figure out how to do so with the primitives I had.

The general problem is that there's no way to "transform" a message pipe handle
into the remote end of the bootstrap message pipe over a Channel. Now, we could
make it possible to make Channel work in this way instead, but independent of
this it's not clear to me that we really want the primordial bootstrap message
pipe to be the one to the shell:
* whenever you're using a Channel, there are likely specific, non-generic
lifecycle management/etc. things that you want to support (that are specific to
that situation, and not generic to the shell)
* you may want to host multiple services/apps over the same Channel, i.e., have
multiple connections to the shell

[Chris Masone, 2014-04-23 19:41:36]

On 2014/04/23 16:44:41, viettrungluu wrote:
> On 2014/04/22 21:16:29, Chris Masone wrote:
> > https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp
> > File mojo/mojo.gyp (right):
> > 
> >
https://codereview.chromium.org/242763003/diff/50001/mojo/mojo.gyp#newcode487
> > mojo/mojo.gyp:487: 'shell/dbus_service_loader.cc',
> > On 2014/04/22 20:39:51, DaveMoore wrote:
> > > It's more common in gyp files to list all files in the target and then
> exclude
> > > them when not required. For OS dependent ones you can do that
automatically
> > > calling this dbux_service_loader_linux.{h,cc}. You would leave the extra
> > > dependencies here though.
> > 
> > Done.
> > 
> > https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi
> > File mojo/mojo_examples.gypi (right):
> > 
> >
>
https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_examples.gypi#...
> > mojo/mojo_examples.gypi:340: {
> > On 2014/04/22 20:39:51, DaveMoore wrote:
> > > Since this doesn't run on android you can remove the package target.
> > 
> > Done.
> > 
> > https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi
> > File mojo/mojo_services.gypi (right):
> > 
> >
>
https://codereview.chromium.org/242763003/diff/50001/mojo/mojo_services.gypi#...
> > mojo/mojo_services.gypi:170: '../gin/gin.gyp:gin',
> > On 2014/04/22 20:39:51, DaveMoore wrote:
> > > Is gin needed?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
> > File mojo/shell/external_service.mojom (right):
> > 
> >
>
https://codereview.chromium.org/242763003/diff/50001/mojo/shell/external_serv...
> > mojo/shell/external_service.mojom:7: [Peer=ExternalServiceHost]
> > On 2014/04/22 20:39:51, DaveMoore wrote:
> > > Why can't you just use the Shell / ShellClient interface here? Even if you
> > need
> > > to interpose yourself between the real shell and this client you could
> simply
> > > proxy both sides. If you don't I think you can just hand the shell_handle
> off
> > to
> > > the dbus service, which would implement the ShellClient interface.
> > 
> > 
> > This interface exists to allow me to hand the ShellHandle off to the dbus
> > service, because I couldn't figure out another way to do it :)
> > 
> > DBus provides a primitive that allows me to pass file descriptors across,
and
> by
> > the time I'm in the DBusServiceLoader code, I have a ShellHandle and (as far
> as
> > I could tell, please correct if I'm wrong) no way to recover the FD of the
> > MessagePipe that underlies it.  So, instead I create a new FD pair and pass
> one
> > over DBus, which I use to get (what I believe is) a MessagePipe set up
between
> > the two processes.
> > 
> > It's probably true that I could manually craft and send a message containing
> the
> > ShellHandle over the pipe I just created, if that'd be preferred. It seemed
> like
> > defining this little API just automatically handled marshalling and such for
> me,
> > so I did it this way.
> > 
> > I would prefer to have some way to get the ShellHandle directly to the
> > externally running process, without have to create a MessagePipe for this
sole
> > purpose, but I couldn't figure out how to do so with the primitives I had.
> 
> The general problem is that there's no way to "transform" a message pipe
handle
> into the remote end of the bootstrap message pipe over a Channel. Now, we
could
> make it possible to make Channel work in this way instead, but independent of
> this it's not clear to me that we really want the primordial bootstrap message
> pipe to be the one to the shell:
> * whenever you're using a Channel, there are likely specific, non-generic
> lifecycle management/etc. things that you want to support (that are specific
to
> that situation, and not generic to the shell)
> * you may want to host multiple services/apps over the same Channel, i.e.,
have
> multiple connections to the shell

Ok, so it sounds like I've done this in a reasonable way, here. What do I need
to do to get this into landable shape?

[viettrungluu, 2014-04-23 20:07:24]

LGTM w/nits.

https://codereview.chromium.org/242763003/diff/70001/mojo/services/dbus_echo/...
File mojo/services/dbus_echo/dbus_echo_service.cc (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/services/dbus_echo/...
mojo/services/dbus_echo/dbus_echo_service.cc:1: // Copyright (c) 2014 The
Chromium Authors. All rights reserved.
Nit: There's no "(c)" in the current copyright notice.

https://codereview.chromium.org/242763003/diff/70001/mojo/services/dbus_echo/...
mojo/services/dbus_echo/dbus_echo_service.cc:166: }
nit: Presumably, this never actually quits, and the C++ standard actually carves
out a special rule for main() (defining the return value of main() to be 0 if
there's no return), but I hate this special rule.

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:20: #if defined(OS_LINUX)
You should probably explicitly include build/build_config.h to pick up OS_...
macros.

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.h
File mojo/shell/context.h (left):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.h#old...
mojo/shell/context.h:23: 
nit: It probably wasn't necessary to delete this blank line. :)

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
File mojo/shell/dbus_service_loader_linux.cc (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
mojo/shell/dbus_service_loader_linux.cc:87: NOTREACHED();
nit: You may as well do a DCHECK_NE here.

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
File mojo/shell/dbus_service_loader_linux.h (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
mojo/shell/dbus_service_loader_linux.h:36: // 1) Your service is already
running,
nit: , -> . (that, or none of these should begin with a capital letter, and 2)
shouldn't end with a period either).

[Chris Masone, 2014-04-23 22:30:59]

https://codereview.chromium.org/242763003/diff/70001/mojo/services/dbus_echo/...
File mojo/services/dbus_echo/dbus_echo_service.cc (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/services/dbus_echo/...
mojo/services/dbus_echo/dbus_echo_service.cc:1: // Copyright (c) 2014 The
Chromium Authors. All rights reserved.
On 2014/04/23 20:07:24, viettrungluu wrote:
> Nit: There's no "(c)" in the current copyright notice.

Done.

https://codereview.chromium.org/242763003/diff/70001/mojo/services/dbus_echo/...
mojo/services/dbus_echo/dbus_echo_service.cc:166: }
On 2014/04/23 20:07:24, viettrungluu wrote:
> nit: Presumably, this never actually quits, and the C++ standard actually
carves
> out a special rule for main() (defining the return value of main() to be 0 if
> there's no return), but I hate this special rule.

Done.

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:20: #if defined(OS_LINUX)
On 2014/04/23 20:07:24, viettrungluu wrote:
> You should probably explicitly include build/build_config.h to pick up OS_...
> macros.

Done.

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.h
File mojo/shell/context.h (left):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/context.h#old...
mojo/shell/context.h:23: 
On 2014/04/23 20:07:24, viettrungluu wrote:
> nit: It probably wasn't necessary to delete this blank line. :)

I blame a shoddy merge!

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
File mojo/shell/dbus_service_loader_linux.cc (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
mojo/shell/dbus_service_loader_linux.cc:87: NOTREACHED();
On 2014/04/23 20:07:24, viettrungluu wrote:
> nit: You may as well do a DCHECK_NE here.

Done.

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
File mojo/shell/dbus_service_loader_linux.h (right):

https://codereview.chromium.org/242763003/diff/70001/mojo/shell/dbus_service_...
mojo/shell/dbus_service_loader_linux.h:36: // 1) Your service is already
running,
On 2014/04/23 20:07:24, viettrungluu wrote:
> nit: , -> . (that, or none of these should begin with a capital letter, and 2)
> shouldn't end with a period either).

Done.

[Chris Masone, 2014-04-24 00:13:54]

The CQ bit was checked by cmasone@chromium.org

[commit-bot: I haz the power, 2014-04-24 00:14:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cmasone@chromium.org/242763003/110001

[commit-bot: I haz the power, 2014-04-24 00:23:20]

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

[commit-bot: I haz the power, 2014-04-24 00:23:21]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[Chris Masone, 2014-04-24 00:39:03]

On 2014/04/24 00:23:21, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on chromium_presubmit

Keybuk@, can you LGTM the addition of dbus to mojo/services/dbus/dbus_echo/DEPS?

[keybuk, 2014-04-24 11:43:48]

lgtm

[Chris Masone, 2014-04-24 13:06:49]

The CQ bit was checked by cmasone@chromium.org

[commit-bot: I haz the power, 2014-04-24 13:30:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cmasone@chromium.org/242763003/110001

[commit-bot: I haz the power, 2014-04-24 16:17:59]

Change committed as 265927

[Chris Masone, 2014-04-24 18:44:52]

The CQ bit was checked by cmasone@chromium.org

[commit-bot: I haz the power, 2014-04-24 21:10:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cmasone@chromium.org/242763003/130001

[commit-bot: I haz the power, 2014-04-24 22:58:56]

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

[commit-bot: I haz the power, 2014-04-24 22:58:56]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Chris Masone, 2014-04-25 16:34:06]

The CQ bit was checked by cmasone@chromium.org

[commit-bot: I haz the power, 2014-04-25 17:54:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cmasone@chromium.org/242763003/130001

[Chris Masone, 2014-04-25 23:34:24]

The CQ bit was unchecked by cmasone@chromium.org

[Chris Masone, 2014-04-27 22:01:06]

The CQ bit was checked by cmasone@chromium.org

[commit-bot: I haz the power, 2014-04-27 22:02:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cmasone@chromium.org/242763003/150001

[commit-bot: I haz the power, 2014-04-28 03:52:36]

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

[commit-bot: I haz the power, 2014-04-28 03:52:37]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[Chris Masone, 2014-04-28 03:57:05]

The CQ bit was checked by cmasone@chromium.org

[commit-bot: I haz the power, 2014-04-28 03:58:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cmasone@chromium.org/242763003/150001

[commit-bot: I haz the power, 2014-04-28 11:53:18]

Change committed as 266508