[GCM] Add connection factory for creating MCS connections

[GCM] Add connection factory for creating MCS connections

The connection factory abstracts all net interactions and enforces backoff
policy. Once it creates a connection handler, Connect calls can be made
whenever a reconnection is necessary/ready.

The connection handler is also abstracted into an interface/impl for easier
use in testing (fake connection factory/handlers will be introduced when
testing the main logic of the client).

BUG=284553
R=akalin@chromium.org

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

[Nicolas Zea, 2013-11-01 19:49:47]

Fred, PTAL. In particular, it looks like the case of making an SSL connection to
a non-https address is a new one, so I'm making use of the http socket pool
helper method (and had to export it as a result). Let me know if you think this
isn't the right way.

[akalin, 2013-11-02 00:15:43]

initial comments

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
File google_apis/gcm/engine/connection_factory.h (right):

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory.h:17: // Factory for creating a
ConnectionHandler and maintaing its connection.
maintaing -> maintaining

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:83:
DCHECK(!connection_handler_.get());
i feel like you should do:

if (!connection_handler) {
  NOTREACHED();
  return;
}

so that we don't add ourselves more than once as an observer (unless NCN already
handles that -- check?)

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:98:
DCHECK(connection_handler_.get());
no .get()

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:106: bool
ConnectionFactoryImpl::IsEndpointReachable() const {
no .get() (and everywhere else in a boolean context)

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:130: if
(backoff_entry_.ShouldRejectRequest()) {
i feel like this logic should be moved into ConnectImpl, just so the logic is
clearer, i.e.:

if (should reject) {
  PostDelayedTask(ConnectImpl);
} else {
  ConnectImpl();
}

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:149: // TODO(zea): This should
probably be a raw request, but there's no good
yeah, I think this should be a raw request. having the mcs_endpoint_ URL start
with https doesn't suffice?

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
File google_apis/gcm/engine/connection_factory_impl.h (right):

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.h:26: class GCM_EXPORT
ConnectionFactoryImpl :
tests for ConnectionFactoryImpl?

[Nicolas Zea, 2013-11-15 23:37:34]

Picking this up again, PTAL

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
File google_apis/gcm/engine/connection_factory.h (right):

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory.h:17: // Factory for creating a
ConnectionHandler and maintaing its connection.
On 2013/11/02 00:15:43, akalin wrote:
> maintaing -> maintaining

Done.

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:83:
DCHECK(!connection_handler_.get());
On 2013/11/02 00:15:43, akalin wrote:
> i feel like you should do:
> 
> if (!connection_handler) {
>   NOTREACHED();
>   return;
> }
> 
> so that we don't add ourselves more than once as an observer (unless NCN
already
> handles that -- check?)

Assuming you mean if (connection_handler_), done.

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:98:
DCHECK(connection_handler_.get());
On 2013/11/02 00:15:43, akalin wrote:
> no .get()

Done.

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:106: bool
ConnectionFactoryImpl::IsEndpointReachable() const {
On 2013/11/02 00:15:43, akalin wrote:
> no .get() (and everywhere else in a boolean context)

Done.

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:130: if
(backoff_entry_.ShouldRejectRequest()) {
On 2013/11/02 00:15:43, akalin wrote:
> i feel like this logic should be moved into ConnectImpl, just so the logic is
> clearer, i.e.:
> 
> if (should reject) {
>   PostDelayedTask(ConnectImpl);
> } else {
>   ConnectImpl();
> }

Assuming you mean move to Connect, done.

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.cc:149: // TODO(zea): This should
probably be a raw request, but there's no good
On 2013/11/02 00:15:43, akalin wrote:
> yeah, I think this should be a raw request. having the mcs_endpoint_ URL start
> with https doesn't suffice?

Raw connect takes a host/port pair, and prepends a "http://" onto them to create
a GURL, so ssl is never enabled. That said, it only appears to be used in the
jingle code, so that could maybe be changed?

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
File google_apis/gcm/engine/connection_factory_impl.h (right):

https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
google_apis/gcm/engine/connection_factory_impl.h:26: class GCM_EXPORT
ConnectionFactoryImpl :
On 2013/11/02 00:15:43, akalin wrote:
> tests for ConnectionFactoryImpl?

Wasn't sure how best to test the backoff logic, or if I should just rely on the
backoff entry unittests themselves. That said, I think I have a decent approach
now,  that at least enforces the backoff entries are being used properly. Done.

[akalin, 2013-11-19 05:22:04]

On 2013/11/15 23:37:34, Nicolas Zea wrote:
> Picking this up again, PTAL
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> File google_apis/gcm/engine/connection_factory.h (right):
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory.h:17: // Factory for creating a
> ConnectionHandler and maintaing its connection.
> On 2013/11/02 00:15:43, akalin wrote:
> > maintaing -> maintaining
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> File google_apis/gcm/engine/connection_factory_impl.cc (right):
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory_impl.cc:83:
> DCHECK(!connection_handler_.get());
> On 2013/11/02 00:15:43, akalin wrote:
> > i feel like you should do:
> > 
> > if (!connection_handler) {
> >   NOTREACHED();
> >   return;
> > }
> > 
> > so that we don't add ourselves more than once as an observer (unless NCN
> already
> > handles that -- check?)
> 
> Assuming you mean if (connection_handler_), done.
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory_impl.cc:98:
> DCHECK(connection_handler_.get());
> On 2013/11/02 00:15:43, akalin wrote:
> > no .get()
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory_impl.cc:106: bool
> ConnectionFactoryImpl::IsEndpointReachable() const {
> On 2013/11/02 00:15:43, akalin wrote:
> > no .get() (and everywhere else in a boolean context)
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory_impl.cc:130: if
> (backoff_entry_.ShouldRejectRequest()) {
> On 2013/11/02 00:15:43, akalin wrote:
> > i feel like this logic should be moved into ConnectImpl, just so the logic
is
> > clearer, i.e.:
> > 
> > if (should reject) {
> >   PostDelayedTask(ConnectImpl);
> > } else {
> >   ConnectImpl();
> > }
> 
> Assuming you mean move to Connect, done.
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory_impl.cc:149: // TODO(zea): This
should
> probably be a raw request, but there's no good
> On 2013/11/02 00:15:43, akalin wrote:
> > yeah, I think this should be a raw request. having the mcs_endpoint_ URL
start
> > with https doesn't suffice?
> 
> Raw connect takes a host/port pair, and prepends a "http://" onto them to
create
> a GURL, so ssl is never enabled. That said, it only appears to be used in the
> jingle code, so that could maybe be changed?
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> File google_apis/gcm/engine/connection_factory_impl.h (right):
> 
>
https://codereview.chromium.org/54743007/diff/20012/google_apis/gcm/engine/co...
> google_apis/gcm/engine/connection_factory_impl.h:26: class GCM_EXPORT
> ConnectionFactoryImpl :
> On 2013/11/02 00:15:43, akalin wrote:
> > tests for ConnectionFactoryImpl?
> 
> Wasn't sure how best to test the backoff logic, or if I should just rely on
the
> backoff entry unittests themselves. That said, I think I have a decent
approach
> now,  that at least enforces the backoff entries are being used properly.
Done.

WFTokyo, but I'll try to get to this soon.

[akalin, 2013-11-21 03:21:11]

still looking, but one comment

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:167: // TODO(zea): This should
probably be a raw request, but there's no good
yeah, I think adding a InitSocketHandleForTlsConnect() function would be best
(which would be essentially initSocketHandleForRawConnect() but with https.

[akalin, 2013-11-21 04:13:08]

more comments

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory.h (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory.h:34: const
ConnectionHandler::ProtoReceivedCallback& read_callback,
having two callbacks passed in together indicates that perhaps a delegate class
would be better suited. Have you considered that? (I'm not sure if it will
actually be better, since there are pros/cons for both approaches, but wanna
make sure you thought about it).

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:84: return NULL;
hmm on second thought, it's likely that the caller expects the returned handler
to be non-null, and so this will crash anyway. Convert to CHECK? alternatively,
always return connection_handler_.get()?

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:161: // TODO(zea): resolve
proxies.
this is probably for a future CL, but you can copy what jingle does (i.e., use a
ProxyResolvingClientSocket)

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:195:
scoped_ptr<net::BackoffEntry> ConnectionFactoryImpl::CreateBackoffEntry(
this is used once -- do you need it to be a separate function? Perhaps just
replace the scoped_ptr member variable with a BackoffEntry member variable and
initialize in constructor?

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.h (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:78:
scoped_refptr<net::HttpNetworkSession> network_session_;
const?

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:80: net::NetLog* net_log_;
* const?

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:84:
scoped_ptr<net::BackoffEntry> backoff_entry_;
const?

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl_unittest.cc (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl_unittest.cc:66: // Do nothing.
if they aren't called, perhaps ADD_FAILURE() to assert that?

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl_unittest.cc:68: void
WriteContinuation() {
newline before

[Nicolas Zea, 2013-11-22 05:39:18]

PTAL

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory.h (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory.h:34: const
ConnectionHandler::ProtoReceivedCallback& read_callback,
On 2013/11/21 04:13:09, akalin wrote:
> having two callbacks passed in together indicates that perhaps a delegate
class
> would be better suited. Have you considered that? (I'm not sure if it will
> actually be better, since there are pros/cons for both approaches, but wanna
> make sure you thought about it).

Yeah, given that I don't expect this to be reused outside of the MCS client
implementation, and that the callbacks are the two fundamental actions
associated with the handler, I felt leaving it like this was simpler.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.cc (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:84: return NULL;
On 2013/11/21 04:13:09, akalin wrote:
> hmm on second thought, it's likely that the caller expects the returned
handler
> to be non-null, and so this will crash anyway. Convert to CHECK?
alternatively,
> always return connection_handler_.get()?

True. Opted for DCHECK + return always returning connection_handler_.get().

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:161: // TODO(zea): resolve
proxies.
On 2013/11/21 04:13:09, akalin wrote:
> this is probably for a future CL, but you can copy what jingle does (i.e., use
a
> ProxyResolvingClientSocket)

Will do. I plan to add some fallback logic in general here (including switching
to a 443 port as well).

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:167: // TODO(zea): This should
probably be a raw request, but there's no good
On 2013/11/21 03:21:12, akalin wrote:
> yeah, I think adding a InitSocketHandleForTlsConnect() function would be best
> (which would be essentially initSocketHandleForRawConnect() but with https.

Done.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.cc:195:
scoped_ptr<net::BackoffEntry> ConnectionFactoryImpl::CreateBackoffEntry(
On 2013/11/21 04:13:09, akalin wrote:
> this is used once -- do you need it to be a separate function? Perhaps just
> replace the scoped_ptr member variable with a BackoffEntry member variable and
> initialize in constructor?

Doing it this way allows me to override this method in the test so I can better
control backoff.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl.h (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:78:
scoped_refptr<net::HttpNetworkSession> network_session_;
On 2013/11/21 04:13:09, akalin wrote:
> const?

Done.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:80: net::NetLog* net_log_;
On 2013/11/21 04:13:09, akalin wrote:
> * const?

Done.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl.h:84:
scoped_ptr<net::BackoffEntry> backoff_entry_;
On 2013/11/21 04:13:09, akalin wrote:
> const?

Set via virtual method for testing overrides, so don't think it should be const.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
File google_apis/gcm/engine/connection_factory_impl_unittest.cc (right):

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl_unittest.cc:66: // Do nothing.
On 2013/11/21 04:13:09, akalin wrote:
> if they aren't called, perhaps ADD_FAILURE() to assert that?

Done.

https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
google_apis/gcm/engine/connection_factory_impl_unittest.cc:68: void
WriteContinuation() {
On 2013/11/21 04:13:09, akalin wrote:
> newline before

Done.

[Nicolas Zea, 2013-12-02 17:45:06]

On 2013/11/22 05:39:18, Nicolas Zea wrote:
> PTAL
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> File google_apis/gcm/engine/connection_factory.h (right):
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory.h:34: const
> ConnectionHandler::ProtoReceivedCallback& read_callback,
> On 2013/11/21 04:13:09, akalin wrote:
> > having two callbacks passed in together indicates that perhaps a delegate
> class
> > would be better suited. Have you considered that? (I'm not sure if it will
> > actually be better, since there are pros/cons for both approaches, but wanna
> > make sure you thought about it).
> 
> Yeah, given that I don't expect this to be reused outside of the MCS client
> implementation, and that the callbacks are the two fundamental actions
> associated with the handler, I felt leaving it like this was simpler.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> File google_apis/gcm/engine/connection_factory_impl.cc (right):
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.cc:84: return NULL;
> On 2013/11/21 04:13:09, akalin wrote:
> > hmm on second thought, it's likely that the caller expects the returned
> handler
> > to be non-null, and so this will crash anyway. Convert to CHECK?
> alternatively,
> > always return connection_handler_.get()?
> 
> True. Opted for DCHECK + return always returning connection_handler_.get().
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.cc:161: // TODO(zea): resolve
> proxies.
> On 2013/11/21 04:13:09, akalin wrote:
> > this is probably for a future CL, but you can copy what jingle does (i.e.,
use
> a
> > ProxyResolvingClientSocket)
> 
> Will do. I plan to add some fallback logic in general here (including
switching
> to a 443 port as well).
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.cc:167: // TODO(zea): This
should
> probably be a raw request, but there's no good
> On 2013/11/21 03:21:12, akalin wrote:
> > yeah, I think adding a InitSocketHandleForTlsConnect() function would be
best
> > (which would be essentially initSocketHandleForRawConnect() but with https.
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.cc:195:
> scoped_ptr<net::BackoffEntry> ConnectionFactoryImpl::CreateBackoffEntry(
> On 2013/11/21 04:13:09, akalin wrote:
> > this is used once -- do you need it to be a separate function? Perhaps just
> > replace the scoped_ptr member variable with a BackoffEntry member variable
and
> > initialize in constructor?
> 
> Doing it this way allows me to override this method in the test so I can
better
> control backoff.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> File google_apis/gcm/engine/connection_factory_impl.h (right):
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.h:78:
> scoped_refptr<net::HttpNetworkSession> network_session_;
> On 2013/11/21 04:13:09, akalin wrote:
> > const?
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.h:80: net::NetLog* net_log_;
> On 2013/11/21 04:13:09, akalin wrote:
> > * const?
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl.h:84:
> scoped_ptr<net::BackoffEntry> backoff_entry_;
> On 2013/11/21 04:13:09, akalin wrote:
> > const?
> 
> Set via virtual method for testing overrides, so don't think it should be
const.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> File google_apis/gcm/engine/connection_factory_impl_unittest.cc (right):
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl_unittest.cc:66: // Do nothing.
> On 2013/11/21 04:13:09, akalin wrote:
> > if they aren't called, perhaps ADD_FAILURE() to assert that?
> 
> Done.
> 
>
https://codereview.chromium.org/54743007/diff/240001/google_apis/gcm/engine/c...
> google_apis/gcm/engine/connection_factory_impl_unittest.cc:68: void
> WriteContinuation() {
> On 2013/11/21 04:13:09, akalin wrote:
> > newline before
> 
> Done.

ping?

[akalin, 2013-12-05 07:15:27]

sorry for the delay! lgtm

[Nicolas Zea, 2013-12-05 18:34:48]

On 2013/12/05 07:15:27, akalin (on leave) wrote:
> sorry for the delay! lgtm

Woohoo! Attempting commitbot, wish me luck.

[commit-bot: I haz the power, 2013-12-05 18:35:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/54743007/470001

[commit-bot: I haz the power, 2013-12-05 22:22:27]

Retried try job too often on win for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-12-05 23:21:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/54743007/490001

[commit-bot: I haz the power, 2013-12-06 05:31:26]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[commit-bot: I haz the power, 2013-12-06 18:53:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/54743007/490001

[commit-bot: I haz the power, 2013-12-06 21:27:59]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[Nicolas Zea, 2013-12-06 23:17:28]

Committed patchset #7 manually as r239275 (presubmit successful).