Prevent URLRequestRedirectJob from doing async execution

Prevent URLRequestRedirectJob from doing async execution
when request is already canceled and job is killed.

BUG=508900, 503306
Committed: https://crrev.com/cd904b5cd82adca2fb432e37da4302c67d2b4c5d
Cr-Commit-Position: refs/heads/master@{#340298}

[mmenke, 2015-07-13 18:49:51]

mmenke@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2015-07-13 21:14:17]

Thanks for this!

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
net/url_request/url_request_redirect_job.cc:86: return;
Instead of this, I think would be a little more consistent with other
implementations is to add an override method URLRequestRedirectJob::Kill() which
calls  weak_factory_.InvalidateWeakPtrs() (And then URLRequestJob::Kill()).

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
net/url_request/url_request_redirect_job.cc:86: return;
Also, we should have a unit test for this.  I don't think this should be too
bad, but please feel free to ask me for help, our network testing infrastructure
is a bit ugly, and you shouldn't waste your time learning it unless you're going
to be doing more work on the network code (Which, to be clear, is certainly
always welcome, just not sure that's what you'll be spending your time on in the
future).

My suggestion to to take a look at the test InterceptPost302RedirectGet in
url_request_unittest.cc, and base your test on that.

Just remove the stuff we don't need (No need to be a post, etc), cancel the
request just after starting it, and check that the request fails at the end with
whatever error we passed it.

[alexanderk, 2015-07-16 09:48:56]

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
net/url_request/url_request_redirect_job.cc:86: return;
On 2015/07/13 21:14:17, mmenke wrote:
> Also, we should have a unit test for this.  I don't think this should be too
> bad, but please feel free to ask me for help, our network testing
infrastructure
> is a bit ugly, and you shouldn't waste your time learning it unless you're
going
> to be doing more work on the network code (Which, to be clear, is certainly
> always welcome, just not sure that's what you'll be spending your time on in
the
> future).
> 
> My suggestion to to take a look at the test InterceptPost302RedirectGet in
> url_request_unittest.cc, and base your test on that.
> 
> Just remove the stuff we don't need (No need to be a post, etc), cancel the
> request just after starting it, and check that the request fails at the end
with
> whatever error we passed it.

Well, it's kind of complicated to develop this unit test, at least I still
cannot reproduce the same crash. Meanwhile I found another crash with:

TEST_F(URLRequestTestHTTP, RedirectCancelTests) {
  TestDelegate d;
  {
    //GURL url = GURL("http://www.paypal.com/");
    GURL url = GURL("http://www.facebook.com/");
    scoped_ptr<URLRequest> r(
        default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
    r->set_method("GET");
    r->Start();
    EXPECT_TRUE(r->is_pending());
    r->Cancel();
    base::RunLoop().Run();
    EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
    EXPECT_EQ(ERR_ABORTED, r->status().error());
  }
}

[mmenke, 2015-07-16 12:23:02]

On 2015/07/16 09:48:56, alexanderk wrote:
>
https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
> File net/url_request/url_request_redirect_job.cc (right):
> 
>
https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
> net/url_request/url_request_redirect_job.cc:86: return;
> On 2015/07/13 21:14:17, mmenke wrote:
> > Also, we should have a unit test for this.  I don't think this should be too
> > bad, but please feel free to ask me for help, our network testing
> infrastructure
> > is a bit ugly, and you shouldn't waste your time learning it unless you're
> going
> > to be doing more work on the network code (Which, to be clear, is certainly
> > always welcome, just not sure that's what you'll be spending your time on in
> the
> > future).
> > 
> > My suggestion to to take a look at the test InterceptPost302RedirectGet in
> > url_request_unittest.cc, and base your test on that.
> > 
> > Just remove the stuff we don't need (No need to be a post, etc), cancel the
> > request just after starting it, and check that the request fails at the end
> with
> > whatever error we passed it.
> 
> Well, it's kind of complicated to develop this unit test, at least I still
> cannot reproduce the same crash.

You wouldn't get a crash, you'd get the contradictory state that is causing the
crash (Request cancelled but the request ends up being redirected anyways, and
status indicates success, or at least IO_PENDING).

> Meanwhile I found another crash with:
> 
> TEST_F(URLRequestTestHTTP, RedirectCancelTests) {
>   TestDelegate d;
>   {
>     //GURL url = GURL("http://www.paypal.com/");
>     GURL url = GURL("http://www.facebook.com/");
>     scoped_ptr<URLRequest> r(
>         default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
>     r->set_method("GET");
>     r->Start();
>     EXPECT_TRUE(r->is_pending());
>     r->Cancel();
>     base::RunLoop().Run();
>     EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
>     EXPECT_EQ(ERR_ABORTED, r->status().error());
>   }
> }

Curious.  I'll look into it.

[mmenke, 2015-07-16 15:26:14]

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
net/url_request/url_request_redirect_job.cc:86: return;
On 2015/07/16 09:48:56, alexanderk wrote:
> On 2015/07/13 21:14:17, mmenke wrote:
> > Also, we should have a unit test for this.  I don't think this should be too
> > bad, but please feel free to ask me for help, our network testing
> infrastructure
> > is a bit ugly, and you shouldn't waste your time learning it unless you're
> going
> > to be doing more work on the network code (Which, to be clear, is certainly
> > always welcome, just not sure that's what you'll be spending your time on in
> the
> > future).
> > 
> > My suggestion to to take a look at the test InterceptPost302RedirectGet in
> > url_request_unittest.cc, and base your test on that.
> > 
> > Just remove the stuff we don't need (No need to be a post, etc), cancel the
> > request just after starting it, and check that the request fails at the end
> with
> > whatever error we passed it.
> 
> Well, it's kind of complicated to develop this unit test, at least I still
> cannot reproduce the same crash. Meanwhile I found another crash with:
> 
> TEST_F(URLRequestTestHTTP, RedirectCancelTests) {
>   TestDelegate d;
>   {
>     //GURL url = GURL("http://www.paypal.com/");
>     GURL url = GURL("http://www.facebook.com/");
>     scoped_ptr<URLRequest> r(
>         default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
>     r->set_method("GET");
>     r->Start();
>     EXPECT_TRUE(r->is_pending());
>     r->Cancel();
>     base::RunLoop().Run();
>     EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
>     EXPECT_EQ(ERR_ABORTED, r->status().error());
>   }
> }

Thanks for catching this!  This is a bug in the DCHECK that's triggering.  It's
just the logging that's wrong, so not a "real" crasher in release build, but
definitely should be fixed.  I try and get a fix out today.

May need to get that fix in before you can reasonably test your fix, not sure.

[mmenke, 2015-07-16 17:58:32]

Here's my suggested test:

TEST_F(URLRequestTest, URLRequestRedirectJobCancelTest) {
  TestDelegate d;
  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
      GURL("http://not-a-real-domain/"), DEFAULT_PRIORITY, &d));

  URLRequestRedirectJob* job = new URLRequestRedirectJob(
      req.get(), &default_network_delegate_,
      GURL("http://this-should-never-be-navigated-to/"),
      URLRequestRedirectJob::REDIRECT_307_TEMPORARY_REDIRECT,
      "Jumbo shrimp");
  AddTestInterceptor()->set_main_intercept_job(job);

  req->Start();
  req->Cancel();
  base::RunLoop().Run();
  EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
  EXPECT_EQ(0, d.received_redirect_count());
}

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
net/url_request/url_request_redirect_job.cc:86: return;
On 2015/07/16 15:26:14, mmenke wrote:
> On 2015/07/16 09:48:56, alexanderk wrote:
> > On 2015/07/13 21:14:17, mmenke wrote:
> > > Also, we should have a unit test for this.  I don't think this should be
too
> > > bad, but please feel free to ask me for help, our network testing
> > infrastructure
> > > is a bit ugly, and you shouldn't waste your time learning it unless you're
> > going
> > > to be doing more work on the network code (Which, to be clear, is
certainly
> > > always welcome, just not sure that's what you'll be spending your time on
in
> > the
> > > future).
> > > 
> > > My suggestion to to take a look at the test InterceptPost302RedirectGet in
> > > url_request_unittest.cc, and base your test on that.
> > > 
> > > Just remove the stuff we don't need (No need to be a post, etc), cancel
the
> > > request just after starting it, and check that the request fails at the
end
> > with
> > > whatever error we passed it.
> > 
> > Well, it's kind of complicated to develop this unit test, at least I still
> > cannot reproduce the same crash. Meanwhile I found another crash with:
> > 
> > TEST_F(URLRequestTestHTTP, RedirectCancelTests) {
> >   TestDelegate d;
> >   {
> >     //GURL url = GURL("http://www.paypal.com/");
> >     GURL url = GURL("http://www.facebook.com/");
> >     scoped_ptr<URLRequest> r(
> >         default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
> >     r->set_method("GET");
> >     r->Start();
> >     EXPECT_TRUE(r->is_pending());
> >     r->Cancel();
> >     base::RunLoop().Run();
> >     EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
> >     EXPECT_EQ(ERR_ABORTED, r->status().error());
> >   }
> > }
> 
> Thanks for catching this!  This is a bug in the DCHECK that's triggering. 
It's
> just the logging that's wrong, so not a "real" crasher in release build, but
> definitely should be fixed.  I try and get a fix out today.
> 
> May need to get that fix in before you can reasonably test your fix, not sure.

Turns out this is actually a symptom of the bug you're fixing. 
URLRequestJob::NotifyHeadersComplete is being called when the request status is
already cancelled, which only happens because of the bug.

[alexanderk, 2015-07-20 11:55:18]

On 2015/07/16 17:58:32, mmenke wrote:
> Here's my suggested test:
> 
> TEST_F(URLRequestTest, URLRequestRedirectJobCancelTest) {
>   TestDelegate d;
>   scoped_ptr<URLRequest> req(default_context_.CreateRequest(
>       GURL("http://not-a-real-domain/"), DEFAULT_PRIORITY, &d));
> 
>   URLRequestRedirectJob* job = new URLRequestRedirectJob(
>       req.get(), &default_network_delegate_,
>       GURL("http://this-should-never-be-navigated-to/"),
>       URLRequestRedirectJob::REDIRECT_307_TEMPORARY_REDIRECT,
>       "Jumbo shrimp");
>   AddTestInterceptor()->set_main_intercept_job(job);
> 
>   req->Start();
>   req->Cancel();
>   base::RunLoop().Run();
>   EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
>   EXPECT_EQ(0, d.received_redirect_count());
> }
> 
>
https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
> File net/url_request/url_request_redirect_job.cc (right):
> 
>
https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
> net/url_request/url_request_redirect_job.cc:86: return;
> On 2015/07/16 15:26:14, mmenke wrote:
> > On 2015/07/16 09:48:56, alexanderk wrote:
> > > On 2015/07/13 21:14:17, mmenke wrote:
> > > > Also, we should have a unit test for this.  I don't think this should be
> too
> > > > bad, but please feel free to ask me for help, our network testing
> > > infrastructure
> > > > is a bit ugly, and you shouldn't waste your time learning it unless
you're
> > > going
> > > > to be doing more work on the network code (Which, to be clear, is
> certainly
> > > > always welcome, just not sure that's what you'll be spending your time
on
> in
> > > the
> > > > future).
> > > > 
> > > > My suggestion to to take a look at the test InterceptPost302RedirectGet
in
> > > > url_request_unittest.cc, and base your test on that.
> > > > 
> > > > Just remove the stuff we don't need (No need to be a post, etc), cancel
> the
> > > > request just after starting it, and check that the request fails at the
> end
> > > with
> > > > whatever error we passed it.
> > > 
> > > Well, it's kind of complicated to develop this unit test, at least I still
> > > cannot reproduce the same crash. Meanwhile I found another crash with:
> > > 
> > > TEST_F(URLRequestTestHTTP, RedirectCancelTests) {
> > >   TestDelegate d;
> > >   {
> > >     //GURL url = GURL("http://www.paypal.com/");
> > >     GURL url = GURL("http://www.facebook.com/");
> > >     scoped_ptr<URLRequest> r(
> > >         default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
> > >     r->set_method("GET");
> > >     r->Start();
> > >     EXPECT_TRUE(r->is_pending());
> > >     r->Cancel();
> > >     base::RunLoop().Run();
> > >     EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
> > >     EXPECT_EQ(ERR_ABORTED, r->status().error());
> > >   }
> > > }
> > 
> > Thanks for catching this!  This is a bug in the DCHECK that's triggering. 
> It's
> > just the logging that's wrong, so not a "real" crasher in release build, but
> > definitely should be fixed.  I try and get a fix out today.
> > 
> > May need to get that fix in before you can reasonably test your fix, not
sure.
> 
> Turns out this is actually a symptom of the bug you're fixing. 
> URLRequestJob::NotifyHeadersComplete is being called when the request status
is
> already cancelled, which only happens because of the bug.

What about the following?:

TEST_F(URLRequestTest, URLRequestRedirectJobDetachRequestTest) {
  TestDelegate d;
  scoped_ptr<URLRequest> req(default_context_.CreateRequest(
      GURL("http://not-a-real-domain/"), DEFAULT_PRIORITY, &d));

  URLRequestRedirectJob* job = new URLRequestRedirectJob(
      req.get(), &default_network_delegate_,
      GURL("http://this-should-never-be-navigated-to/"),
      URLRequestRedirectJob::REDIRECT_307_TEMPORARY_REDIRECT,
      "Jumbo shrimp");
  AddTestInterceptor()->set_main_intercept_job(job);

  req->Start();
  req->Cancel();
  job->DetachRequest();
  base::RunLoop().Run();
  EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
  EXPECT_EQ(0, d.received_redirect_count());
}

[mmenke, 2015-07-20 15:31:08]

On 2015/07/20 11:55:18, alexanderk wrote:
> On 2015/07/16 17:58:32, mmenke wrote:
> > Here's my suggested test:
> > 
> > TEST_F(URLRequestTest, URLRequestRedirectJobCancelTest) {
> >   TestDelegate d;
> >   scoped_ptr<URLRequest> req(default_context_.CreateRequest(
> >       GURL("http://not-a-real-domain/"), DEFAULT_PRIORITY, &d));
> > 
> >   URLRequestRedirectJob* job = new URLRequestRedirectJob(
> >       req.get(), &default_network_delegate_,
> >       GURL("http://this-should-never-be-navigated-to/"),
> >       URLRequestRedirectJob::REDIRECT_307_TEMPORARY_REDIRECT,
> >       "Jumbo shrimp");
> >   AddTestInterceptor()->set_main_intercept_job(job);
> > 
> >   req->Start();
> >   req->Cancel();
> >   base::RunLoop().Run();
> >   EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
> >   EXPECT_EQ(0, d.received_redirect_count());
> > }
> > 
> >
>
https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
> > File net/url_request/url_request_redirect_job.cc (right):
> > 
> >
>
https://codereview.chromium.org/1232113002/diff/1/net/url_request/url_request...
> > net/url_request/url_request_redirect_job.cc:86: return;
> > On 2015/07/16 15:26:14, mmenke wrote:
> > > On 2015/07/16 09:48:56, alexanderk wrote:
> > > > On 2015/07/13 21:14:17, mmenke wrote:
> > > > > Also, we should have a unit test for this.  I don't think this should
be
> > too
> > > > > bad, but please feel free to ask me for help, our network testing
> > > > infrastructure
> > > > > is a bit ugly, and you shouldn't waste your time learning it unless
> you're
> > > > going
> > > > > to be doing more work on the network code (Which, to be clear, is
> > certainly
> > > > > always welcome, just not sure that's what you'll be spending your time
> on
> > in
> > > > the
> > > > > future).
> > > > > 
> > > > > My suggestion to to take a look at the test
InterceptPost302RedirectGet
> in
> > > > > url_request_unittest.cc, and base your test on that.
> > > > > 
> > > > > Just remove the stuff we don't need (No need to be a post, etc),
cancel
> > the
> > > > > request just after starting it, and check that the request fails at
the
> > end
> > > > with
> > > > > whatever error we passed it.
> > > > 
> > > > Well, it's kind of complicated to develop this unit test, at least I
still
> > > > cannot reproduce the same crash. Meanwhile I found another crash with:
> > > > 
> > > > TEST_F(URLRequestTestHTTP, RedirectCancelTests) {
> > > >   TestDelegate d;
> > > >   {
> > > >     //GURL url = GURL("http://www.paypal.com/");
> > > >     GURL url = GURL("http://www.facebook.com/");
> > > >     scoped_ptr<URLRequest> r(
> > > >         default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
> > > >     r->set_method("GET");
> > > >     r->Start();
> > > >     EXPECT_TRUE(r->is_pending());
> > > >     r->Cancel();
> > > >     base::RunLoop().Run();
> > > >     EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
> > > >     EXPECT_EQ(ERR_ABORTED, r->status().error());
> > > >   }
> > > > }
> > > 
> > > Thanks for catching this!  This is a bug in the DCHECK that's triggering. 
> > It's
> > > just the logging that's wrong, so not a "real" crasher in release build,
but
> > > definitely should be fixed.  I try and get a fix out today.
> > > 
> > > May need to get that fix in before you can reasonably test your fix, not
> sure.
> > 
> > Turns out this is actually a symptom of the bug you're fixing. 
> > URLRequestJob::NotifyHeadersComplete is being called when the request status
> is
> > already cancelled, which only happens because of the bug.
> 
> What about the following?:
> 
> TEST_F(URLRequestTest, URLRequestRedirectJobDetachRequestTest) {
>   TestDelegate d;
>   scoped_ptr<URLRequest> req(default_context_.CreateRequest(
>       GURL("http://not-a-real-domain/"), DEFAULT_PRIORITY, &d));
> 
>   URLRequestRedirectJob* job = new URLRequestRedirectJob(
>       req.get(), &default_network_delegate_,
>       GURL("http://this-should-never-be-navigated-to/"),
>       URLRequestRedirectJob::REDIRECT_307_TEMPORARY_REDIRECT,
>       "Jumbo shrimp");
>   AddTestInterceptor()->set_main_intercept_job(job);
> 
>   req->Start();
>   req->Cancel();
>   job->DetachRequest();
>   base::RunLoop().Run();
>   EXPECT_EQ(URLRequestStatus::CANCELED, req->status().status());
>   EXPECT_EQ(0, d.received_redirect_count());
> }

Works for me

[mmenke, 2015-07-22 19:03:56]

Sorry, didn't notice you'd uploaded a new CL.  You should post a message when
you do, so reviewers will know they need to re-review the CL.  I thought I was
still waiting on you to upload a CL.

https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
File net/url_request/url_request_redirect_job.cc (right):

https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
net/url_request/url_request_redirect_job.cc:86: return;
Per earlier comment, I'd much rather we override the Kill() method, and clear
the weak pointers there.  I think it's cleaner to cancel what we're doing when
we're cancelled, rather than to check at a later point in time if we were
cancelled in the past.

[mmenke, 2015-07-22 19:05:05]

https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
File net/url_request/url_request_unittest.cc (right):

https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
net/url_request/url_request_unittest.cc:9216: // See http://crbug.com/508900
nit:  Method descriptions should go before the method.

[alexanderk, 2015-07-23 08:07:43]

On 2015/07/22 19:03:56, mmenke wrote:
> Sorry, didn't notice you'd uploaded a new CL.  You should post a message when
> you do, so reviewers will know they need to re-review the CL.  I thought I was
> still waiting on you to upload a CL.

Sorry, I was sure that message was sent.
 
>
https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
> File net/url_request/url_request_redirect_job.cc (right):
> 
>
https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
> net/url_request/url_request_redirect_job.cc:86: return;
> Per earlier comment, I'd much rather we override the Kill() method, and clear
> the weak pointers there.  I think it's cleaner to cancel what we're doing when
> we're cancelled, rather than to check at a later point in time if we were
> cancelled in the past.

OK, I'll try, but anyway it will not save us from DetachRequest case.

[alexanderk, 2015-07-23 10:47:42]

On 2015/07/23 08:07:43, alexanderk wrote:
> On 2015/07/22 19:03:56, mmenke wrote:
> > Sorry, didn't notice you'd uploaded a new CL.  You should post a message
when
> > you do, so reviewers will know they need to re-review the CL.  I thought I
was
> > still waiting on you to upload a CL.
> 
> Sorry, I was sure that message was sent.
>  
> >
>
https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
> > File net/url_request/url_request_redirect_job.cc (right):
> > 
> >
>
https://codereview.chromium.org/1232113002/diff/20001/net/url_request/url_req...
> > net/url_request/url_request_redirect_job.cc:86: return;
> > Per earlier comment, I'd much rather we override the Kill() method, and
clear
> > the weak pointers there.  I think it's cleaner to cancel what we're doing
when
> > we're cancelled, rather than to check at a later point in time if we were
> > cancelled in the past.
> 
> OK, I'll try, but anyway it will not save us from DetachRequest case.

Yes, you were absolutely right about overriding Kill().
Please see updated CL.

[mmenke, 2015-07-23 15:09:38]

LGTM!  Great job on catching this.

[mmenke, 2015-07-23 15:41:16]

Also, could you replace the bug line with:

BUG=508900,503306

Bug 503306 is just the same problem.  Since it's a crasher, it's restricted to
access by Googlers, unfortunately - not my policy.

[alexanderk, 2015-07-24 11:36:57]

On 2015/07/23 15:41:16, mmenke wrote:
> Also, could you replace the bug line with:
> 
> BUG=508900,503306
> 
> Bug 503306 is just the same problem.  Since it's a crasher, it's restricted to
> access by Googlers, unfortunately - not my policy.

np

[mmenke, 2015-07-24 15:42:43]

I'm going to go ahead an CQ this.  Hope you don't mind - I'm considering a merge
in for M45, though it isn't a very common crash.

[mmenke, 2015-07-24 15:42:47]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2015-07-24 15:43:12]

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

[commit-bot: I haz the power, 2015-07-24 15:49:31]

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

[commit-bot: I haz the power, 2015-07-24 15:49:32]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2015-07-24 16:25:20]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2015-07-24 16:25:25]

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

[commit-bot: I haz the power, 2015-07-24 16:31:25]

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

[commit-bot: I haz the power, 2015-07-24 16:31:26]

Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2015-07-24 18:15:56]

Not sure what's going on....  I'll try once more.

[mmenke, 2015-07-24 18:15:59]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2015-07-24 18:16:25]

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

[commit-bot: I haz the power, 2015-07-24 18:57:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-24 18:58:15]

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