Mojo C++ bindings: introduce MultiplexRouter and related classes.

Mojo C++ bindings: introduce MultiplexRouter and related classes.

MultiplexRouter supports routing messages for multiple interfaces over a single message pipe. It supports multi-thread access as well.
This is part of the work to support associated interfaces.

BUG=546067
TEST=Newly added tests

Committed: https://crrev.com/5176a0cd84b342d3d9abcb6ec9060f05a1eaf57c
Cr-Commit-Position: refs/heads/master@{#360903}

[yzshen1, 2015-11-18 23:17:14]

yzshen@chromium.org changed reviewers:
+ sky@chromium.org

[yzshen1, 2015-11-18 23:17:14]

Hi, Scott.

Would you please take a look?

Sorry it is a fairly big CL. I would be more than happy to chat about the
implementation if you would like to.

More tests for the MultiplexRouter will be in a follow-up CL where I add
AssociatedInterfacePtr and AssociatedBinding.

Thanks!

[sky, 2015-11-19 17:16:27]

https://codereview.chromium.org/1455063004/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1455063004/diff/40001/chrome/BUILD.gn#newcode6
chrome/BUILD.gn:6: import("//build/config/compiler/compiler.gni")
The fix landed, so you should be able to revert your change here.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/connector.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:29: //   - Sending messages can be
configured to be thread safe (please see comments
Can you use threading asserts to verify this(ThreadChecker)?

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:34: enum SendingThreadSaftyType {
SINGLE_THREADED_SEND, MULTI_THREADED_SEND };
nit: enum class

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:37: // If |sending_thread_safty_type|
is set to MULTI_THREADED_SEND, Accept() is
safty->safety

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/interface_endpoint_client.cc (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:19: class
ResponderThunk : public MessageReceiverWithStatus {
Please add a description as to what classes do, even implementation ones.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:65: };
DISALLOW...

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:189: bool ok =
incoming_receiver_->AcceptWithResponder(message, responder);
It would be nice if AcceptWithResponder took a scoped_ptr

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/interface_endpoint_client.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.h:76: typedef
std::map<uint64_t, MessageReceiver*> ResponderMap;
using

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.h:88: };
DISALLOW...

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.cc (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:17: // with
nit: fix wrapping.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:18: // the router. Always
accessed under the router's lock.
To help debugging could this object take the lock and assert that it is
acquired?

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:33: AddRef();
I suspect a scoped_refptr to this member would be less error prone, and it gets
rid of the double AddRefs() and is better documenting what is going on.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:143: auto iter =
endpoints_.begin();
Use a for loop (you can still increment inside the for loop), but that way iter
is scoped to the loop.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:146: ++iter;
This is subtle and worth a comment.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:180: // If the endpoint already
exist, it is because we have received a
It seems weird that you have to detect this case in CreateLocalEndpointHandle,
can't it be detected before calling this?

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:408: client->NotifyError();
Can bad things happen between the time you unlock and call NotifyError? For
example, could DetachEndpointClient() be called?

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:465: result =
client->HandleIncomingMessage(owned_message.get());
Similar question to above about unlocking.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:33: // single message pipe.
Please add a more general description of threading. You clearly define which
methods are intended for which thread, but an overview would be helpful.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:51: // supposed to be used
locally; while |remote_endpoint| is supposed to be sent
You can remove 'supposed' to everywhere you have it in this file. Without it the
text reads more natural.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:186: std::deque<Task*> tasks_;
Can you make this a std::deque<scoped_ptr>?

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:189: };
DISALLOW..

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc:14: :
id_(kInvalidInterfaceId), is_local_(true) {}
Use delegating constructors.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc:37: reset();
Do you need to handle the case of other == this?

[yzshen1, 2015-11-19 22:00:41]

Thanks, Scott!

Please take another look.

https://codereview.chromium.org/1455063004/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1455063004/diff/40001/chrome/BUILD.gn#newcode6
chrome/BUILD.gn:6: import("//build/config/compiler/compiler.gni")
On 2015/11/19 17:16:26, sky wrote:
> The fix landed, so you should be able to revert your change here.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/connector.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:29: //   - Sending messages can be
configured to be thread safe (please see comments
On 2015/11/19 17:16:26, sky wrote:
> Can you use threading asserts to verify this(ThreadChecker)?

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:34: enum SendingThreadSaftyType {
SINGLE_THREADED_SEND, MULTI_THREADED_SEND };
On 2015/11/19 17:16:27, sky wrote:
> nit: enum class

Currently I refer to the values at the callsites using
Connector::SINGLE_THREADED_SEND

If I change it to enum class, I will have to refer to the values at the
callsites using
Connector::SendingThreadSafetyType::SINGLE_THREADED_SEND

I think that is a little too verbose.
Do you think it is okay if I keep it the current way? Thanks!

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:37: // If |sending_thread_safty_type|
is set to MULTI_THREADED_SEND, Accept() is
On 2015/11/19 17:16:27, sky wrote:
> safty->safety

Ah, sorry. :)

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/interface_endpoint_client.cc (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:19: class
ResponderThunk : public MessageReceiverWithStatus {
On 2015/11/19 17:16:27, sky wrote:
> Please add a description as to what classes do, even implementation ones.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:65: };
On 2015/11/19 17:16:27, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.cc:189: bool ok =
incoming_receiver_->AcceptWithResponder(message, responder);
On 2015/11/19 17:16:27, sky wrote:
> It would be nice if AcceptWithResponder took a scoped_ptr

Right. I agree.

That would require quite some changes because this interface is used in quite
some places across the bindings. I think it is out of the scope of this CL. But
it seems to be a nice improvement now that we allow base/.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/interface_endpoint_client.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.h:76: typedef
std::map<uint64_t, MessageReceiver*> ResponderMap;
On 2015/11/19 17:16:27, sky wrote:
> using

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/interface_endpoint_client.h:88: };
On 2015/11/19 17:16:27, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.cc (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:17: // with
On 2015/11/19 17:16:27, sky wrote:
> nit: fix wrapping.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:18: // the router. Always
accessed under the router's lock.
On 2015/11/19 17:16:27, sky wrote:
> To help debugging could this object take the lock and assert that it is
> acquired?

Done. I added the asserts.

It adds a little cost though, because otherwise this object doesn't need to
store a pointer to the router's lock.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:33: AddRef();
On 2015/11/19 17:16:27, sky wrote:
> I suspect a scoped_refptr to this member would be less error prone, and it
gets
> rid of the double AddRefs() and is better documenting what is going on.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:143: auto iter =
endpoints_.begin();
On 2015/11/19 17:16:27, sky wrote:
> Use a for loop (you can still increment inside the for loop), but that way
iter
> is scoped to the loop.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:146: ++iter;
On 2015/11/19 17:16:27, sky wrote:
> This is subtle and worth a comment.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:180: // If the endpoint already
exist, it is because we have received a
On 2015/11/19 17:16:27, sky wrote:
> It seems weird that you have to detect this case in CreateLocalEndpointHandle,
> can't it be detected before calling this?

This is legitimate:
this method is a public method, typically called by the generated bindings code
when it deserializes an incoming message containing an interface endpoint. The
callsite shouldn't care (nor be able to know without races) whether a control
message PeerAssociatedEndpointClosed for that interface has been received by
this router or not.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:408: client->NotifyError();
On 2015/11/19 17:16:27, sky wrote:
> Can bad things happen between the time you unlock and call NotifyError? For
> example, could DetachEndpointClient() be called?

|client| lives on a single thread. NotifyError() and DetachEndpointClient() are
both called on that thread. So it is not possible for DetachEndpointClient() to
be called in the middle.

I added a comment.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:465: result =
client->HandleIncomingMessage(owned_message.get());
On 2015/11/19 17:16:27, sky wrote:
> Similar question to above about unlocking.

(Please see my comment for the previous question.)

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:33: // single message pipe.
On 2015/11/19 17:16:27, sky wrote:
> Please add a more general description of threading. You clearly define which
> methods are intended for which thread, but an overview would be helpful.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:51: // supposed to be used
locally; while |remote_endpoint| is supposed to be sent
On 2015/11/19 17:16:27, sky wrote:
> You can remove 'supposed' to everywhere you have it in this file. Without it
the
> text reads more natural.

I see. Thanks!

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:186: std::deque<Task*> tasks_;
On 2015/11/19 17:16:27, sky wrote:
> Can you make this a std::deque<scoped_ptr>?

I don't think we can do that. IIUC, scoped_ptr moves using Pass() which is not
understood by std::deque. We may want to use the counterpart of scoped_ptr in
the stl library. But I would like to wait util the discussion around what C++
library features can be used settles. :)

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:189: };
On 2015/11/19 17:16:27, sky wrote:
> DISALLOW..

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc:14: :
id_(kInvalidInterfaceId), is_local_(true) {}
On 2015/11/19 17:16:27, sky wrote:
> Use delegating constructors.

Done.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/scoped_interface_endpoint_handle.cc:37: reset();
On 2015/11/19 17:16:27, sky wrote:
> Do you need to handle the case of other == this?

Because this is a move assignment, I think it is unnecessary.

https://codereview.chromium.org/1455063004/diff/60001/tools/perf/chrome_telem...
File tools/perf/chrome_telemetry_build/BUILD.gn (right):

https://codereview.chromium.org/1455063004/diff/60001/tools/perf/chrome_telem...
tools/perf/chrome_telemetry_build/BUILD.gn:5:
import("//build/config/compiler/compiler.gni")
I will make sure I remove this file when the CL is ready to land.

[sky, 2015-11-20 01:00:19]

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/connector.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:34: enum SendingThreadSaftyType {
SINGLE_THREADED_SEND, MULTI_THREADED_SEND };
On 2015/11/19 22:00:40, yzshen1 wrote:
> On 2015/11/19 17:16:27, sky wrote:
> > nit: enum class
> 
> Currently I refer to the values at the callsites using
> Connector::SINGLE_THREADED_SEND
> 
> If I change it to enum class, I will have to refer to the values at the
> callsites using
> Connector::SendingThreadSafetyType::SINGLE_THREADED_SEND
> 
> I think that is a little too verbose.
> Do you think it is okay if I keep it the current way? Thanks!

Would using address that?
Part of this is because you went with such a verbose name (that has send twice).
If you go with:
enum class ThreadType {
  SINGLE_THREADED_SEND,
  MULTI_THREADED_SEND
}
It's shorter overall.

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:186: std::deque<Task*> tasks_;
On 2015/11/19 22:00:41, yzshen1 wrote:
> On 2015/11/19 17:16:27, sky wrote:
> > Can you make this a std::deque<scoped_ptr>?
> 
> I don't think we can do that. IIUC, scoped_ptr moves using Pass() which is not
> understood by std::deque. We may want to use the counterpart of scoped_ptr in
> the stl library. But I would like to wait util the discussion around what C++
> library features can be used settles. :)

I believe scoped_ptr now supports the move operator.

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.cc (right):

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:406: // We must unlock before
calling into |client| because it may calls this
calls->call

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:468: // We must unlock before
calling into |client| because it may calls this
calls->call

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.h (right):

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:42: // comments for more
detials.
details.

[yzshen1, 2015-11-20 17:01:00]

Thank you Scott! :)

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/connector.h (right):

https://codereview.chromium.org/1455063004/diff/40001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/connector.h:34: enum SendingThreadSaftyType {
SINGLE_THREADED_SEND, MULTI_THREADED_SEND };
On 2015/11/20 01:00:19, sky wrote:
> On 2015/11/19 22:00:40, yzshen1 wrote:
> > On 2015/11/19 17:16:27, sky wrote:
> > > nit: enum class
> > 
> > Currently I refer to the values at the callsites using
> > Connector::SINGLE_THREADED_SEND
> > 
> > If I change it to enum class, I will have to refer to the values at the
> > callsites using
> > Connector::SendingThreadSafetyType::SINGLE_THREADED_SEND
> > 
> > I think that is a little too verbose.
> > Do you think it is okay if I keep it the current way? Thanks!
> 
> Would using address that?
Using doesn't help in this case unfortunately.

> Part of this is because you went with such a verbose name (that has send
twice).
> If you go with:
> enum class ThreadType {
>   SINGLE_THREADED_SEND,
>   MULTI_THREADED_SEND
> }
> It's shorter overall.

Yeah, it could be made shorter. But it would be nice if I could avoid always
typing two scopes. :)

I think our style guide doesn't ban plain old "enum". The two features of "enum
class": (1) type safety: it doesn't seem to be a concern here; (2) scope: that
is what I would like to avoid.

I wish I could still use plain old "enum". But if you still prefer "enum class",
I think one option is to move this enum out of the Connector class. (And I am
thinking to rename it as "ConnectorConfig".)

enum class ConnectorConfig {
...
};

class Connector {
  Connector(...ConnectorConfig config...);
};

One minor annoyance is the definition is not adjacent to where it is used
anymore. But I can live with that.

Please let me know what you think? Thanks!

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.cc (right):

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:406: // We must unlock before
calling into |client| because it may calls this
On 2015/11/20 01:00:19, sky wrote:
> calls->call

Done.

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.cc:468: // We must unlock before
calling into |client| because it may calls this
On 2015/11/20 01:00:19, sky wrote:
> calls->call

Done.

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
File mojo/public/cpp/bindings/lib/multiplex_router.h (right):

https://codereview.chromium.org/1455063004/diff/60001/mojo/public/cpp/binding...
mojo/public/cpp/bindings/lib/multiplex_router.h:42: // comments for more
detials.
On 2015/11/20 01:00:19, sky wrote:
> details.

Done. I need an editor with spell checking! ;)

[sky, 2015-11-20 18:40:07]

Go with what you have, LGTM

[yzshen1, 2015-11-20 18:43:41]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2015-11-20 18:44:26]

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

[yzshen1, 2015-11-20 18:45:51]

The CQ bit was unchecked by yzshen@chromium.org

[yzshen1, 2015-11-20 18:55:22]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2015-11-20 18:55:23]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1455063004/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2015-11-20 18:57:30]

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

[commit-bot: I haz the power, 2015-11-20 20:21:42]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-20 20:22:38]

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