Dispatch DRP IPC call from renderer main thread

Dispatch DRP IPC call from renderer main thread

This ensures that the DRP performance timing histograms are
recorded even when user navigates to a different page. Please
see the bug for more details.

BUG=525329
Committed: https://crrev.com/0c2036dbfa8177857e0df81de7a4bb34023300b5
Cr-Commit-Position: refs/heads/master@{#347972}

[tbansal1, 2015-08-27 15:56:01]

tbansal@chromium.org changed reviewers:
+ bengr@chromium.org

[tbansal1, 2015-08-27 15:56:02]

PTAL. Thanks.

[bengr, 2015-08-27 21:17:44]

lgtm

[tbansal1, 2015-08-27 21:18:48]

tbansal@chromium.org changed reviewers:
+ ppi@chromium.org

[tbansal1, 2015-08-27 21:18:49]

ppi: PTAL. thanks.

[tbansal1, 2015-08-27 21:19:51]

tbansal@chromium.org changed reviewers:
+ sky@chromium.org
- ppi@chromium.org

[tbansal1, 2015-08-27 21:19:51]

sky: PTAL. thanks.

[sky, 2015-08-27 23:19:53]

sky@chromium.org changed reviewers:
+ jam@chromium.org
- sky@chromium.org

[sky, 2015-08-27 23:19:54]

I'm not a good reviewer for this. Maybe John.

[jam, 2015-08-28 20:22:08]

is this not being sent because the Send() reaches  RenderWidget::Send and
is_swapped_out_ is true?

[tbansal1, 2015-08-28 21:30:52]

On 2015/08/28 20:22:08, jam wrote:
> is this not being sent because the Send() reaches  RenderWidget::Send and
> is_swapped_out_ is true?

Yes, in the version without the fix, I see that is_swapped_out_ is true. Also,
enabling IPC logging prints another message:
[24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
MessageAttachmentSet destroyed with unconsumed descriptors: 0/1

This message is printed after the caller is done with IPC Send method call
(without actually getting any results back).

[tbansal1, 2015-08-31 20:06:55]

ping. I would also like to merge this in M-46.

[jam, 2015-08-31 23:12:47]

jam@chromium.org changed reviewers:
+ creis@chromium.org

[jam, 2015-08-31 23:13:08]

On 2015/08/28 21:30:52, tbansal1 wrote:
> On 2015/08/28 20:22:08, jam wrote:
> > is this not being sent because the Send() reaches  RenderWidget::Send and
> > is_swapped_out_ is true?
> 
> Yes, in the version without the fix, I see that is_swapped_out_ is true. Also,
> enabling IPC logging prints another message:
> [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> 
> This message is printed after the caller is done with IPC Send method call
> (without actually getting any results back).

in that case, i believe the standard thing that's done here is to add the
message id to  SwappedOutMessages::CanSendWhileSwappedOut
+creis to confirm

[Charlie Reis, 2015-08-31 23:18:50]

creis@chromium.org changed reviewers:
+ nasko@chromium.org

[Charlie Reis, 2015-08-31 23:21:34]

On 2015/08/31 23:13:08, jam wrote:
> On 2015/08/28 21:30:52, tbansal1 wrote:
> > On 2015/08/28 20:22:08, jam wrote:
> > > is this not being sent because the Send() reaches  RenderWidget::Send and
> > > is_swapped_out_ is true?
> > 
> > Yes, in the version without the fix, I see that is_swapped_out_ is true.
Also,
> > enabling IPC logging prints another message:
> > [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > 
> > This message is printed after the caller is done with IPC Send method call
> > (without actually getting any results back).
> 
> in that case, i believe the standard thing that's done here is to add the
> message id to  SwappedOutMessages::CanSendWhileSwappedOut
> +creis to confirm

[+nasko]

Generally you shouldn't be sending IPCs once a RenderView is swapped out.  It
looks like this is called in response to ClosePage(), though, which I suppose
happens after we're in that state.  I don't have an immediately better answer
(other than waiting for swappedout:// logic to be removed in the coming weeks),
so John's suggestion of adding it to CanSendWhileSwappedOut seems like the right
one.

[tbansal1, 2015-08-31 23:22:16]

On 2015/08/31 23:13:08, jam wrote:
> On 2015/08/28 21:30:52, tbansal1 wrote:
> > On 2015/08/28 20:22:08, jam wrote:
> > > is this not being sent because the Send() reaches  RenderWidget::Send and
> > > is_swapped_out_ is true?
> > 
> > Yes, in the version without the fix, I see that is_swapped_out_ is true.
Also,
> > enabling IPC logging prints another message:
> > [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > 
> > This message is printed after the caller is done with IPC Send method call
> > (without actually getting any results back).
> 
> in that case, i believe the standard thing that's done here is to add the
> message id to  SwappedOutMessages::CanSendWhileSwappedOut
> +creis to confirm

Interesting, because that file has similar code that sends IPC calls on 
renderer thread:
 content::RenderThread::Get()->UpdateHistograms(...);

[nasko, 2015-08-31 23:36:05]

On 2015/08/31 23:21:34, Charlie Reis wrote:
> On 2015/08/31 23:13:08, jam wrote:
> > On 2015/08/28 21:30:52, tbansal1 wrote:
> > > On 2015/08/28 20:22:08, jam wrote:
> > > > is this not being sent because the Send() reaches  RenderWidget::Send
and
> > > > is_swapped_out_ is true?
> > > 
> > > Yes, in the version without the fix, I see that is_swapped_out_ is true.
> Also,
> > > enabling IPC logging prints another message:
> > > [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > > 
> > > This message is printed after the caller is done with IPC Send method call
> > > (without actually getting any results back).
> > 
> > in that case, i believe the standard thing that's done here is to add the
> > message id to  SwappedOutMessages::CanSendWhileSwappedOut
> > +creis to confirm
> 
> [+nasko]
> 
> Generally you shouldn't be sending IPCs once a RenderView is swapped out.  It
> looks like this is called in response to ClosePage(), though, which I suppose
> happens after we're in that state.  I don't have an immediately better answer
> (other than waiting for swappedout:// logic to be removed in the coming
weeks),
> so John's suggestion of adding it to CanSendWhileSwappedOut seems like the
right
> one.

Punching a hole for this in CanSendWhileSwappedOut seems like the wrong approach
to me. PageLoadHistograms should be converted to RenderFrameObserver as
described in https://crbug.com/472140.

[tbansal1, 2015-08-31 23:53:24]

On 2015/08/31 23:36:05, nasko wrote:
> On 2015/08/31 23:21:34, Charlie Reis wrote:
> > On 2015/08/31 23:13:08, jam wrote:
> > > On 2015/08/28 21:30:52, tbansal1 wrote:
> > > > On 2015/08/28 20:22:08, jam wrote:
> > > > > is this not being sent because the Send() reaches  RenderWidget::Send
> and
> > > > > is_swapped_out_ is true?
> > > > 
> > > > Yes, in the version without the fix, I see that is_swapped_out_ is true.
> > Also,
> > > > enabling IPC logging prints another message:
> > > > [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > > > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > > > 
> > > > This message is printed after the caller is done with IPC Send method
call
> > > > (without actually getting any results back).
> > > 
> > > in that case, i believe the standard thing that's done here is to add the
> > > message id to  SwappedOutMessages::CanSendWhileSwappedOut
> > > +creis to confirm
> > 
> > [+nasko]
> > 
> > Generally you shouldn't be sending IPCs once a RenderView is swapped out. 
It
> > looks like this is called in response to ClosePage(), though, which I
suppose
> > happens after we're in that state.  I don't have an immediately better
answer
> > (other than waiting for swappedout:// logic to be removed in the coming
> weeks),
> > so John's suggestion of adding it to CanSendWhileSwappedOut seems like the
> right
> > one.
> 
> Punching a hole for this in CanSendWhileSwappedOut seems like the wrong
approach
> to me. PageLoadHistograms should be converted to RenderFrameObserver as
> described in https://crbug.com/472140.

Is there a simple fix that can be merged into M-46 to fix this regression?

[nasko, 2015-09-01 00:04:32]

On 2015/08/31 23:53:24, tbansal1 wrote:
> On 2015/08/31 23:36:05, nasko wrote:
> > On 2015/08/31 23:21:34, Charlie Reis wrote:
> > > On 2015/08/31 23:13:08, jam wrote:
> > > > On 2015/08/28 21:30:52, tbansal1 wrote:
> > > > > On 2015/08/28 20:22:08, jam wrote:
> > > > > > is this not being sent because the Send() reaches 
RenderWidget::Send
> > and
> > > > > > is_swapped_out_ is true?
> > > > > 
> > > > > Yes, in the version without the fix, I see that is_swapped_out_ is
true.
> > > Also,
> > > > > enabling IPC logging prints another message:
> > > > > [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > > > > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > > > > 
> > > > > This message is printed after the caller is done with IPC Send method
> call
> > > > > (without actually getting any results back).
> > > > 
> > > > in that case, i believe the standard thing that's done here is to add
the
> > > > message id to  SwappedOutMessages::CanSendWhileSwappedOut
> > > > +creis to confirm
> > > 
> > > [+nasko]
> > > 
> > > Generally you shouldn't be sending IPCs once a RenderView is swapped out. 
> It
> > > looks like this is called in response to ClosePage(), though, which I
> suppose
> > > happens after we're in that state.  I don't have an immediately better
> answer
> > > (other than waiting for swappedout:// logic to be removed in the coming
> > weeks),
> > > so John's suggestion of adding it to CanSendWhileSwappedOut seems like the
> > right
> > > one.
> > 
> > Punching a hole for this in CanSendWhileSwappedOut seems like the wrong
> approach
> > to me. PageLoadHistograms should be converted to RenderFrameObserver as
> > described in https://crbug.com/472140.
> 
> Is there a simple fix that can be merged into M-46 to fix this regression?

How is this a regression? The logic in swapped out hasn't changed for a long
time and this has been a RenderViewObserver for a while.

[tbansal1, 2015-09-01 00:10:25]

On 2015/09/01 00:04:32, nasko wrote:
> On 2015/08/31 23:53:24, tbansal1 wrote:
> > On 2015/08/31 23:36:05, nasko wrote:
> > > On 2015/08/31 23:21:34, Charlie Reis wrote:
> > > > On 2015/08/31 23:13:08, jam wrote:
> > > > > On 2015/08/28 21:30:52, tbansal1 wrote:
> > > > > > On 2015/08/28 20:22:08, jam wrote:
> > > > > > > is this not being sent because the Send() reaches 
> RenderWidget::Send
> > > and
> > > > > > > is_swapped_out_ is true?
> > > > > > 
> > > > > > Yes, in the version without the fix, I see that is_swapped_out_ is
> true.
> > > > Also,
> > > > > > enabling IPC logging prints another message:
> > > > > > [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > > > > > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > > > > > 
> > > > > > This message is printed after the caller is done with IPC Send
method
> > call
> > > > > > (without actually getting any results back).
> > > > > 
> > > > > in that case, i believe the standard thing that's done here is to add
> the
> > > > > message id to  SwappedOutMessages::CanSendWhileSwappedOut
> > > > > +creis to confirm
> > > > 
> > > > [+nasko]
> > > > 
> > > > Generally you shouldn't be sending IPCs once a RenderView is swapped
out. 
> > It
> > > > looks like this is called in response to ClosePage(), though, which I
> > suppose
> > > > happens after we're in that state.  I don't have an immediately better
> > answer
> > > > (other than waiting for swappedout:// logic to be removed in the coming
> > > weeks),
> > > > so John's suggestion of adding it to CanSendWhileSwappedOut seems like
the
> > > right
> > > > one.
> > > 
> > > Punching a hole for this in CanSendWhileSwappedOut seems like the wrong
> > approach
> > > to me. PageLoadHistograms should be converted to RenderFrameObserver as
> > > described in https://crbug.com/472140.
> > 
> > Is there a simple fix that can be merged into M-46 to fix this regression?
> 
> How is this a regression? The logic in swapped out hasn't changed for a long
> time and this has been a RenderViewObserver for a while.

We used to have non-IPC code here. At that point, it recorded
the data reduction proxy (DRP) metrics accurately.
Somebody from  DRP team changed it to IPC code and since then, it stopped
recording the metrics properly.

It is a regression in the way we record the DRP histograms
(It is not a regression in IPC code itself or the renderer).
Sorry, if that was not clear.

[nasko, 2015-09-01 00:23:23]

On 2015/09/01 00:10:25, tbansal1 wrote:
> On 2015/09/01 00:04:32, nasko wrote:
> > On 2015/08/31 23:53:24, tbansal1 wrote:
> > > On 2015/08/31 23:36:05, nasko wrote:
> > > > On 2015/08/31 23:21:34, Charlie Reis wrote:
> > > > > On 2015/08/31 23:13:08, jam wrote:
> > > > > > On 2015/08/28 21:30:52, tbansal1 wrote:
> > > > > > > On 2015/08/28 20:22:08, jam wrote:
> > > > > > > > is this not being sent because the Send() reaches 
> > RenderWidget::Send
> > > > and
> > > > > > > > is_swapped_out_ is true?
> > > > > > > 
> > > > > > > Yes, in the version without the fix, I see that is_swapped_out_ is
> > true.
> > > > > Also,
> > > > > > > enabling IPC logging prints another message:
> > > > > > >
[24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > > > > > > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > > > > > > 
> > > > > > > This message is printed after the caller is done with IPC Send
> method
> > > call
> > > > > > > (without actually getting any results back).
> > > > > > 
> > > > > > in that case, i believe the standard thing that's done here is to
add
> > the
> > > > > > message id to  SwappedOutMessages::CanSendWhileSwappedOut
> > > > > > +creis to confirm
> > > > > 
> > > > > [+nasko]
> > > > > 
> > > > > Generally you shouldn't be sending IPCs once a RenderView is swapped
> out. 
> > > It
> > > > > looks like this is called in response to ClosePage(), though, which I
> > > suppose
> > > > > happens after we're in that state.  I don't have an immediately better
> > > answer
> > > > > (other than waiting for swappedout:// logic to be removed in the
coming
> > > > weeks),
> > > > > so John's suggestion of adding it to CanSendWhileSwappedOut seems like
> the
> > > > right
> > > > > one.
> > > > 
> > > > Punching a hole for this in CanSendWhileSwappedOut seems like the wrong
> > > approach
> > > > to me. PageLoadHistograms should be converted to RenderFrameObserver as
> > > > described in https://crbug.com/472140.
> > > 
> > > Is there a simple fix that can be merged into M-46 to fix this regression?
> > 
> > How is this a regression? The logic in swapped out hasn't changed for a long
> > time and this has been a RenderViewObserver for a while.
> 
> We used to have non-IPC code here. At that point, it recorded
> the data reduction proxy (DRP) metrics accurately.
> Somebody from  DRP team changed it to IPC code and since then, it stopped
> recording the metrics properly.
> 
> It is a regression in the way we record the DRP histograms
> (It is not a regression in IPC code itself or the renderer).
> Sorry, if that was not clear.

Let's go with the hole in CanSendWhileSwappedOut. Having poked around a bit,
there might be other issues, but at least it should not block the IPC due to
swapped out state. It will be good to verify it actually works with a test, as
how swapped out works is a bit of magic and hard to reason about.

[tbansal1, 2015-09-01 18:43:58]

On 2015/09/01 00:23:23, nasko wrote:
> On 2015/09/01 00:10:25, tbansal1 wrote:
> > On 2015/09/01 00:04:32, nasko wrote:
> > > On 2015/08/31 23:53:24, tbansal1 wrote:
> > > > On 2015/08/31 23:36:05, nasko wrote:
> > > > > On 2015/08/31 23:21:34, Charlie Reis wrote:
> > > > > > On 2015/08/31 23:13:08, jam wrote:
> > > > > > > On 2015/08/28 21:30:52, tbansal1 wrote:
> > > > > > > > On 2015/08/28 20:22:08, jam wrote:
> > > > > > > > > is this not being sent because the Send() reaches 
> > > RenderWidget::Send
> > > > > and
> > > > > > > > > is_swapped_out_ is true?
> > > > > > > > 
> > > > > > > > Yes, in the version without the fix, I see that is_swapped_out_
is
> > > true.
> > > > > > Also,
> > > > > > > > enabling IPC logging prints another message:
> > > > > > > >
> [24065:24094:0828/142638:WARNING:ipc_message_attachment_set.cc(53)]
> > > > > > > > MessageAttachmentSet destroyed with unconsumed descriptors: 0/1
> > > > > > > > 
> > > > > > > > This message is printed after the caller is done with IPC Send
> > method
> > > > call
> > > > > > > > (without actually getting any results back).
> > > > > > > 
> > > > > > > in that case, i believe the standard thing that's done here is to
> add
> > > the
> > > > > > > message id to  SwappedOutMessages::CanSendWhileSwappedOut
> > > > > > > +creis to confirm
> > > > > > 
> > > > > > [+nasko]
> > > > > > 
> > > > > > Generally you shouldn't be sending IPCs once a RenderView is swapped
> > out. 
> > > > It
> > > > > > looks like this is called in response to ClosePage(), though, which
I
> > > > suppose
> > > > > > happens after we're in that state.  I don't have an immediately
better
> > > > answer
> > > > > > (other than waiting for swappedout:// logic to be removed in the
> coming
> > > > > weeks),
> > > > > > so John's suggestion of adding it to CanSendWhileSwappedOut seems
like
> > the
> > > > > right
> > > > > > one.
> > > > > 
> > > > > Punching a hole for this in CanSendWhileSwappedOut seems like the
wrong
> > > > approach
> > > > > to me. PageLoadHistograms should be converted to RenderFrameObserver
as
> > > > > described in https://crbug.com/472140.
> > > > 
> > > > Is there a simple fix that can be merged into M-46 to fix this
regression?
> > > 
> > > How is this a regression? The logic in swapped out hasn't changed for a
long
> > > time and this has been a RenderViewObserver for a while.
> > 
> > We used to have non-IPC code here. At that point, it recorded
> > the data reduction proxy (DRP) metrics accurately.
> > Somebody from  DRP team changed it to IPC code and since then, it stopped
> > recording the metrics properly.
> > 
> > It is a regression in the way we record the DRP histograms
> > (It is not a regression in IPC code itself or the renderer).
> > Sorry, if that was not clear.
> 
> Let's go with the hole in CanSendWhileSwappedOut. Having poked around a bit,
> there might be other issues, but at least it should not block the IPC due to
> swapped out state. It will be good to verify it actually works with a test, as
> how swapped out works is a bit of magic and hard to reason about.

If we go with the hole in CanSendWhileSwappedOut, then //content/common
will depend on //components/data_reduction_proxy. This is because
the message is defined in
"components/data_reduction_proxy/content/common/data_reduction_proxy_messages.h".

Right now, //components/ depends on //content (not the other way round).
Is it okay to introduce this cyclic dependency?

[Charlie Reis, 2015-09-01 21:46:41]

On 2015/09/01 18:43:58, tbansal1 wrote:
> On 2015/09/01 00:23:23, nasko wrote:
> > Let's go with the hole in CanSendWhileSwappedOut. Having poked around a bit,
> > there might be other issues, but at least it should not block the IPC due to
> > swapped out state. It will be good to verify it actually works with a test,
as
> > how swapped out works is a bit of magic and hard to reason about.
> 
> If we go with the hole in CanSendWhileSwappedOut, then //content/common
> will depend on //components/data_reduction_proxy. This is because
> the message is defined in
>
"components/data_reduction_proxy/content/common/data_reduction_proxy_messages.h".
> 
> Right now, //components/ depends on //content (not the other way round).
> Is it okay to introduce this cyclic dependency?

No, that dependency is not ok.  It looks like you can override
ContentClient::CanSendWhileSwappedOut to do it from chrome/, though.

[tbansal1, 2015-09-02 01:43:15]

PTAL. Now, I am overriding CanSendWhileSwappedOut() method.
Thanks!

[Charlie Reis, 2015-09-02 17:16:30]

This approach seems fine with a few nits.

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
chrome/common/chrome_content_client.cc:616: return
content::ContentClient::CanSendWhileSwappedOut(message) ||
Remove this first clause.  The default implementation is meant to be overridden
and not called here.  (It also just returns false, so it's a no-op.)

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.h (right):

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
chrome/common/chrome_content_client.h:83: bool CanSendWhileSwappedOut(const
IPC::Message* message) override;
nit: List this in the same order as in content_client.h, above GetProduct(). 
(Same in .cc file.)

[tbansal1, 2015-09-02 17:30:26]

Charlie: PTAL. thanks.

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
chrome/common/chrome_content_client.cc:616: return
content::ContentClient::CanSendWhileSwappedOut(message) ||
On 2015/09/02 17:16:30, Charlie Reis (OOO till 9-8) wrote:
> Remove this first clause.  The default implementation is meant to be
overridden
> and not called here.  (It also just returns false, so it's a no-op.)

Added DCHECK. Please let me know if that's not correct.

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.h (right):

https://codereview.chromium.org/1315303002/diff/20001/chrome/common/chrome_co...
chrome/common/chrome_content_client.h:83: bool CanSendWhileSwappedOut(const
IPC::Message* message) override;
On 2015/09/02 17:16:30, Charlie Reis (OOO till 9-8) wrote:
> nit: List this in the same order as in content_client.h, above GetProduct(). 
> (Same in .cc file.)

Done.

[Charlie Reis, 2015-09-02 17:35:05]

LGTM if you remove the DCHECK.

https://codereview.chromium.org/1315303002/diff/40001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/1315303002/diff/40001/chrome/common/chrome_co...
chrome/common/chrome_content_client.cc:544:
DCHECK(!content::ContentClient::CanSendWhileSwappedOut(message));
You don't need this.  ContentClient::CanSendWhileSwappedOut is just a default
implementation, and this implementation replaces it.  We shouldn't call the
default one at all from here.

[tbansal1, 2015-09-02 17:38:48]

nasko@: PTAL. Thanks.

https://codereview.chromium.org/1315303002/diff/40001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/1315303002/diff/40001/chrome/common/chrome_co...
chrome/common/chrome_content_client.cc:544:
DCHECK(!content::ContentClient::CanSendWhileSwappedOut(message));
On 2015/09/02 17:35:05, Charlie Reis (OOO till 9-8) wrote:
> You don't need this.  ContentClient::CanSendWhileSwappedOut is just a default
> implementation, and this implementation replaces it.  We shouldn't call the
> default one at all from here.

Done.

[tbansal1, 2015-09-02 17:45:26]

jam@: PTAL * for OWNERS approval.

[nasko, 2015-09-02 22:02:49]

You already have Charlie's l-g-t-m, on need for one from me. You need OWNERS
though.

[tbansal1, 2015-09-02 22:35:14]

On 2015/09/02 22:02:49, nasko (slow to review) wrote:
> You already have Charlie's l-g-t-m, on need for one from me. You need OWNERS
> though.

ok. John: OWNERS approval please.

[tbansal1, 2015-09-08 18:00:40]

jam: Friendly ping :). thanks.

[jam, 2015-09-09 17:44:30]

lgtm

[tbansal1, 2015-09-09 17:49:22]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2015-09-09 17:49:22]

The patchset sent to the CQ was uploaded after l-g-t-m from bengr@chromium.org,
creis@chromium.org
Link to the patchset: https://codereview.chromium.org/1315303002/#ps60001
(title: "Removed DHCECK")

[commit-bot: I haz the power, 2015-09-09 17:50:13]

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

[commit-bot: I haz the power, 2015-09-09 18:42:59]

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

[commit-bot: I haz the power, 2015-09-09 18:43:00]

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

[tbansal1, 2015-09-09 18:43:43]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2015-09-09 18:44:05]

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

[commit-bot: I haz the power, 2015-09-09 19:13:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-09 19:14:01]

Patchset 4 (id:??) landed as
https://crrev.com/0c2036dbfa8177857e0df81de7a4bb34023300b5
Cr-Commit-Position: refs/heads/master@{#347972}

[commit-bot: I haz the power, 2015-09-23 12:01:50]

Patchset 4 (id:??) landed as
https://crrev.com/0c2036dbfa8177857e0df81de7a4bb34023300b5
Cr-Commit-Position: refs/heads/master@{#347972}