Allow MessageFilters to restrict listening to specific message classes

Allow MessageFilters to restrict listening to specific message classes

ChannelProxy currently offers messages to all member MessageFilters.  It turns
out that a good portion of the most common message types will never be filtered,
making the O(N) filter walk an unnecessary affair.  To prevent this, allow
MessageFilters to indicate which (if any) subset of message classes they may
filter, allowing the ChannelProxy to refine the list of filters that are offered
a particular message.  This saves ~35us per message received on the browser IO
thread for a typical Android device.

Relanding with a few more guards for MessageFilter removal, access and message
class subscription.

BUG=340881
TBR=asargent@chromium.org

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

[jdduke (slow), 2014-02-10 21:10:53]

jam@: I'd like to get your thoughts on this WIP change, particularly regarding
IPC::ChannelProxy and BrowserMessageFilter.  

We found that simply offering the message to all filters (in the browser), for
messages that went unfiltered, was taking ~40us on Android.  This is one of the
solutions I had in mind.

I may be able to simplify this further if I find that 90% of filters only listen
to a single message class.  However, it's not a whole lot more complicated to
support multiple message classes, so I've started with that.

My other proposal was to have |OnMessageReceived| indicate whether it could have
handle a particular message type, and build/refine a map of message types to
filters from that.  However, I think the message class refinement gets us 95% of
the way there.  What do you think?

[jdduke (slow), 2014-02-10 22:11:54]

I went ahead and finished updating all BrowserMessageFilter derivatives.

On my Nexus 4, in release mode, this brings the average unhandled message time
down from ~42us to ~9us during a typical touch-scroll sequence.

[epennerAtGoogle, 2014-02-10 23:55:46]

Cool. Haven't reviewed the entire thing but this is very similar to what I was
thinking.

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h
File ipc/ipc_channel_proxy.h (right):

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h#...
ipc/ipc_channel_proxy.h:245: // or simply an array of length (LastIPCMsgStart +
1).
+1 to just a resized-vector, or something equivalent.

[epennerAtGoogle, 2014-02-11 00:17:30]

One question: If a filter specifies that it only handles a couple classes, what
happens if it doesn't actually filter it? Does it still get processed by the
other filters? It would be fastest if those messages can just be dropped at that
point.

[jdduke (slow), 2014-02-11 00:29:28]

On 2014/02/11 00:17:30, epennerAtGoogle wrote:
> One question: If a filter specifies that it only handles a couple classes,
what
> happens if it doesn't actually filter it? Does it still get processed by the
> other filters? It would be fastest if those messages can just be dropped at
that
> point.

Well, if there are multiple filters per class (and there are for a few classes),
they'll be offered to every such filter.  However, I'm not sure that we can just
drop the message; shouldn't all unhandled messages be forwarded to the
ChannelProxy's listener?  I noticed a few BrowserMessageFilter's that listen to
ViewHostMsg's, which we definitely can't drop.

I suppose we could have the listener expose which message classes it's
interested in, though that gets a bit trickier when you consider all of the
different paths a message can take after it hits RenderProcessHost (Maybe not?
Maybe it's just a small subset like ViewMsg/InputMsg and a few others?).

[epennerAtGoogle, 2014-02-11 00:48:08]

On 2014/02/11 00:29:28, jdduke wrote:
> On 2014/02/11 00:17:30, epennerAtGoogle wrote:
> > One question: If a filter specifies that it only handles a couple classes,
> what
> > happens if it doesn't actually filter it? Does it still get processed by the
> > other filters? It would be fastest if those messages can just be dropped at
> that
> > point.
> 
> Well, if there are multiple filters per class (and there are for a few
classes),
> they'll be offered to every such filter.  However, I'm not sure that we can
just
> drop the message; shouldn't all unhandled messages be forwarded to the
> ChannelProxy's listener?  I noticed a few BrowserMessageFilter's that listen
to
> ViewHostMsg's, which we definitely can't drop.
> 
> I suppose we could have the listener expose which message classes it's
> interested in, though that gets a bit trickier when you consider all of the
> different paths a message can take after it hits RenderProcessHost (Maybe not?
> Maybe it's just a small subset like ViewMsg/InputMsg and a few others?).

Yeah I guess the semantics aren't correct to just drop it. I bet in practice a
filter can speak on behalf of other filters that the message should be dropped.
But I guess a better/cleaner way to achieve that would be for each filter to
handle all the messages in the class, all the time, and just have it be a no-op.

[jdduke (slow), 2014-02-11 00:53:58]

On 2014/02/11 00:48:08, epennerAtGoogle wrote:
> On 2014/02/11 00:29:28, jdduke wrote:
> > On 2014/02/11 00:17:30, epennerAtGoogle wrote:
> > > One question: If a filter specifies that it only handles a couple classes,
> > what
> > > happens if it doesn't actually filter it? Does it still get processed by
the
> > > other filters? It would be fastest if those messages can just be dropped
at
> > that
> > > point.
> > 
> > Well, if there are multiple filters per class (and there are for a few
> classes),
> > they'll be offered to every such filter.  However, I'm not sure that we can
> just
> > drop the message; shouldn't all unhandled messages be forwarded to the
> > ChannelProxy's listener?  I noticed a few BrowserMessageFilter's that listen
> to
> > ViewHostMsg's, which we definitely can't drop.
> > 
> > I suppose we could have the listener expose which message classes it's
> > interested in, though that gets a bit trickier when you consider all of the
> > different paths a message can take after it hits RenderProcessHost (Maybe
not?
> > Maybe it's just a small subset like ViewMsg/InputMsg and a few others?).
> 
> Yeah I guess the semantics aren't correct to just drop it. I bet in practice a
> filter can speak on behalf of other filters that the message should be
dropped.
> But I guess a better/cleaner way to achieve that would be for each filter to
> handle all the messages in the class, all the time, and just have it be a
no-op.

Agreed, we're so close to having that  "1 message class to 1 filter instance"
relationship...  it could be worth looking at each exception and seeing if the
handled message types can be regrouped accordingly into additional classes to
enforce this.

[jdduke (slow), 2014-02-11 18:46:40]

piman@: Looks like jam@ is out, what do you think of this approach, particularly
the changes in content/public/browser/browser_message_filter.{cc,h} and
ipc/ipc_channel_proxy.{cc,h}?

[piman, 2014-02-11 21:48:45]

Overall it seems reasonable. A couple of comments.

Do you have data that shows how much time do we save on filtering and/or an
histogram of how many filters we go through per message (before/after)?

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.cc...
ipc/ipc_channel_proxy.cc:119: }
So, there is an assumption that filters are run in the order they are added.
I suspect we mostly don't rely on that but we should audit to make sure, since
we're changing the order here.

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h
File ipc/ipc_channel_proxy.h (right):

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h#...
ipc/ipc_channel_proxy.h:234: std::vector<scoped_refptr<MessageFilter> >
all_filters_;
Do we need this? Can't we just iterate over message_global_filters_ and
message_class_filters_ each time?

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h#...
ipc/ipc_channel_proxy.h:245: // or simply an array of length (LastIPCMsgStart +
1).
On 2014/02/10 23:55:47, epennerAtGoogle wrote:
> +1 to just a resized-vector, or something equivalent.

An array sg.
Generally, hash_maps are preferred to maps if the order doesn't matter (usually
cheaper both in run-time and memory), but in this case, because the size is
static and the key space is compact, an array is even better.

[jdduke (slow), 2014-02-11 23:01:47]

On 2014/02/11 21:48:45, piman wrote:
> Do you have data that shows how much time do we save on filtering and/or an
> histogram of how many filters we go through per message (before/after)?

I can grab some numbers for a histogram.  On my N4, this drives the ~42us cost
per unfiltered event down to ~9us.  There are still a few filters that listen to
ViewMsg's, which are quite common, so there's still some small overhead.  I'd
like to also measure this on an N5 and Galaxy Nexus.

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.cc...
ipc/ipc_channel_proxy.cc:119: }
On 2014/02/11 21:48:46, piman wrote:
> So, there is an assumption that filters are run in the order they are added.
> I suspect we mostly don't rely on that but we should audit to make sure, since
> we're changing the order here.

Good call.  I *think* we're OK in our current form, as all BrowserMessageFilters
will only exist in |message_class_filters_|, and message filter order for any
given class is preserved with respect to |AddFilter()|.  That may be a gotcha
for other ChanneProxy instances where some filters are global and others not.

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h
File ipc/ipc_channel_proxy.h (right):

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h#...
ipc/ipc_channel_proxy.h:234: std::vector<scoped_refptr<MessageFilter> >
all_filters_;
On 2014/02/11 21:48:46, piman wrote:
> Do we need this? Can't we just iterate over message_global_filters_ and
> message_class_filters_ each time?

Well, there are a few operations where we iterate over each filter once and call
a particular function, e.g., |OnChannelError()|, |OnChannelClosed()|.  If we
don't store the list of all filters, we'd need to dynamically generate a unique
list of filters from those in  |message_class_filters_|.

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h#...
ipc/ipc_channel_proxy.h:245: // or simply an array of length (LastIPCMsgStart +
1).
On 2014/02/11 21:48:46, piman wrote:
> On 2014/02/10 23:55:47, epennerAtGoogle wrote:
> > +1 to just a resized-vector, or something equivalent.
> 
> An array sg.
> Generally, hash_maps are preferred to maps if the order doesn't matter
(usually
> cheaper both in run-time and memory), but in this case, because the size is
> static and the key space is compact, an array is even better.

Done.

[piman, 2014-02-11 23:10:55]

On Tue, Feb 11, 2014 at 3:01 PM, <jdduke@chromium.org> wrote:

> On 2014/02/11 21:48:45, piman wrote:
>
>> Do you have data that shows how much time do we save on filtering and/or
>> an
>> histogram of how many filters we go through per message (before/after)?
>>
>
> I can grab some numbers for a histogram.  On my N4, this drives the ~42us
> cost
> per unfiltered event down to ~9us.  There are still a few filters that
> listen to
> ViewMsg's, which are quite common, so there's still some small overhead.
>  I'd
> like to also measure this on an N5 and Galaxy Nexus.
>
>
>
> https://codereview.chromium.org/142923005/diff/60001/ipc/
> ipc_channel_proxy.cc
> File ipc/ipc_channel_proxy.cc (right):
>
> https://codereview.chromium.org/142923005/diff/60001/ipc/
> ipc_channel_proxy.cc#newcode119
> ipc/ipc_channel_proxy.cc:119: }
> On 2014/02/11 21:48:46, piman wrote:
>
>> So, there is an assumption that filters are run in the order they are
>>
> added.
>
>> I suspect we mostly don't rely on that but we should audit to make
>>
> sure, since
>
>> we're changing the order here.
>>
>
> Good call.  I *think* we're OK in our current form, as all
> BrowserMessageFilters will only exist in |message_class_filters_|, and
> message filter order for any given class is preserved with respect to
> |AddFilter()|.  That may be a gotcha for other ChanneProxy instances
> where some filters are global and others not.
>
>
> https://codereview.chromium.org/142923005/diff/60001/ipc/
> ipc_channel_proxy.h
> File ipc/ipc_channel_proxy.h (right):
>
> https://codereview.chromium.org/142923005/diff/60001/ipc/
> ipc_channel_proxy.h#newcode234
> ipc/ipc_channel_proxy.h:234: std::vector<scoped_refptr<MessageFilter> >
> all_filters_;
> On 2014/02/11 21:48:46, piman wrote:
>
>> Do we need this? Can't we just iterate over message_global_filters_
>>
> and
>
>> message_class_filters_ each time?
>>
>
> Well, there are a few operations where we iterate over each filter once
> and call a particular function, e.g., |OnChannelError()|,
> |OnChannelClosed()|.  If we don't store the list of all filters, we'd
> need to dynamically generate a unique list of filters from those in
> |message_class_filters_|.


I'm mostly trying to see if we can avoid storing and maintaining duplicate
lists. If it's only OnChannelError and OnChannelClosed, those are rare
enough that we can simply use a hash_set there to uniquify the lists.

>
>
> https://codereview.chromium.org/142923005/diff/60001/ipc/
> ipc_channel_proxy.h#newcode245
> ipc/ipc_channel_proxy.h:245: // or simply an array of length
> (LastIPCMsgStart + 1).
> On 2014/02/11 21:48:46, piman wrote:
>
>> On 2014/02/10 23:55:47, epennerAtGoogle wrote:
>> > +1 to just a resized-vector, or something equivalent.
>>
>
>  An array sg.
>> Generally, hash_maps are preferred to maps if the order doesn't matter
>>
> (usually
>
>> cheaper both in run-time and memory), but in this case, because the
>>
> size is
>
>> static and the key space is compact, an array is even better.
>>
>
> Done.
>
> https://codereview.chromium.org/142923005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jdduke (slow), 2014-02-11 23:44:59]

On 2014/02/11 23:10:55, piman wrote:
> I'm mostly trying to see if we can avoid storing and maintaining duplicate
> lists. If it's only OnChannelError and OnChannelClosed, those are rare
> enough that we can simply use a hash_set there to uniquify the lists.

I see.  Yeah, we also use it in OnChannelOpened and OnChannelConnected.  I
initially hoped not to duplicate the lists, then I found that it was actually
less bookkeeping to store the original full filter list.  I'll take another look
to see if anything clever comes to mind other than just dynamically generating a
hash_set when uniqueness is required.

[piman, 2014-02-12 01:04:02]

https://codereview.chromium.org/142923005/diff/200001/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

https://codereview.chromium.org/142923005/diff/200001/ipc/ipc_channel_proxy.c...
ipc/ipc_channel_proxy.cc:23: return message_class > 0 && message_class <
LastIPCMsgStart;
0 is actually valid.

https://codereview.chromium.org/142923005/diff/200001/ipc/ipc_channel_proxy.c...
ipc/ipc_channel_proxy.cc:121: NOTREACHED() << "Invalid message class: " <<
message_class;
Not NOTREACHED. This could be reached in 2 cases:
- by a malicious renderer / nacl app.
- for message replies (class is 0xffff iirc).

Returning false is correct in either case.

[jdduke (slow), 2014-02-12 02:12:37]

I'd like to add a few basic unit tests for this change.  I didn't see any
existing ones specifically for ChannelProxy, so this may be a good time to add
them.

https://codereview.chromium.org/142923005/diff/200001/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

https://codereview.chromium.org/142923005/diff/200001/ipc/ipc_channel_proxy.c...
ipc/ipc_channel_proxy.cc:23: return message_class > 0 && message_class <
LastIPCMsgStart;
On 2014/02/12 01:04:03, piman wrote:
> 0 is actually valid.

Ugg, good catch.

https://codereview.chromium.org/142923005/diff/200001/ipc/ipc_channel_proxy.c...
ipc/ipc_channel_proxy.cc:121: NOTREACHED() << "Invalid message class: " <<
message_class;
On 2014/02/12 01:04:03, piman wrote:
> Not NOTREACHED. This could be reached in 2 cases:
> - by a malicious renderer / nacl app.
> - for message replies (class is 0xffff iirc).
> 
> Returning false is correct in either case.

Ahh, I hadn't considered replies, I'll make a slight assert distinction.

[epennerAtGoogle, 2014-02-12 02:34:08]

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h
File ipc/ipc_channel_proxy.h (right):

https://codereview.chromium.org/142923005/diff/60001/ipc/ipc_channel_proxy.h#...
ipc/ipc_channel_proxy.h:245: // or simply an array of length (LastIPCMsgStart +
1).
Just FYI, I mentioned vector since it typically has bounds checking in debug
builds (at least IIRC - I haven't actually checked). If our other tools are good
enough to catch out-of-bounds these days, no worries (it's also pretty simple
code).

[piman, 2014-02-12 02:37:28]

https://codereview.chromium.org/142923005/diff/270002/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

https://codereview.chromium.org/142923005/diff/270002/ipc/ipc_channel_proxy.c...
ipc/ipc_channel_proxy.cc:122: << "Invalid message class: " << message_class;
You can DLOG_IF(ERROR), but don't DCHECK.
The path can occur in real life, and we can't assume it doesn't - we need to
handle it correctly. The DCHECK gives the wrong idea here.

(the rationale is that we should have a unit test to verify that we behave
correctly when an untrusted peer sends bogus data, and the DCHECK prevents one
from writing it).

In particular, you wouldn't want someone changing the code into

if (message.type() == IPC_REPLY_ID || message.type() == IPC_LOGGING_ID)
  return false;
DCHECK(ValidMessageClass(message_class));

Which would be equivalent semantically, but would be wrong in prod.

[jdduke (slow), 2014-02-13 19:55:25]

piman@: I think this is nearly ready to go. I can get some additional reviewers
for the various filter implementations in content/ and chrome/ if you'd like;
for the most part they're pretty straightforward message class additions to the
constructor. 

Once you're happy with the general shape of things, I'll get additional
reviewers for android_webview/, extensions/ and components/.

https://codereview.chromium.org/142923005/diff/270002/ipc/ipc_channel_proxy.cc
File ipc/ipc_channel_proxy.cc (right):

https://codereview.chromium.org/142923005/diff/270002/ipc/ipc_channel_proxy.c...
ipc/ipc_channel_proxy.cc:122: << "Invalid message class: " << message_class;
On 2014/02/12 02:37:29, piman wrote:
> You can DLOG_IF(ERROR), but don't DCHECK.
> The path can occur in real life, and we can't assume it doesn't - we need to
> handle it correctly. The DCHECK gives the wrong idea here.
> 
> (the rationale is that we should have a unit test to verify that we behave
> correctly when an untrusted peer sends bogus data, and the DCHECK prevents one
> from writing it).
> 
> In particular, you wouldn't want someone changing the code into
> 
> if (message.type() == IPC_REPLY_ID || message.type() == IPC_LOGGING_ID)
>   return false;
> DCHECK(ValidMessageClass(message_class));
> 
> Which would be equivalent semantically, but would be wrong in prod.

Right, makes perfect sense.

[jdduke (slow), 2014-02-14 17:03:08]

Owner review? Thanks.

jvoung@: components/nacl
boliu@: android_webview/browser/
asargent@: extensions/browser

[boliu, 2014-02-14 17:15:23]

aw lgtm

nice!

[jvoung (off chromium), 2014-02-14 19:07:59]

nacl_host_message_filter.cc LGTM

[piman, 2014-02-14 22:05:04]

https://codereview.chromium.org/142923005/diff/860002/content/browser/rendere...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/142923005/diff/860002/content/browser/rendere...
content/browser/renderer_host/render_message_filter.cc:413:
IPC_MESSAGE_HANDLER(FrameHostMsg_GetPluginInfo, OnGetPluginInfo)
Needs FrameMsgStart in the category list

https://codereview.chromium.org/142923005/diff/860002/ipc/ipc_channel_proxy_u...
File ipc/ipc_channel_proxy_unittest.cc (right):

https://codereview.chromium.org/142923005/diff/860002/ipc/ipc_channel_proxy_u...
ipc/ipc_channel_proxy_unittest.cc:169: thread_->message_loop()->RunUntilIdle();
I don't think this does what you want it to do.
If you look at the implementation, it will create a RunLoop, which will run the
*current* message loop - it makes sense, you can't run another thread's ML on
this thread.
It's already running anyway.

https://codereview.chromium.org/142923005/diff/860002/ipc/ipc_channel_proxy_u...
ipc/ipc_channel_proxy_unittest.cc:192: Send(sender(), -1, SEND);
nit: -1 is special (for replies, etc.). You may want to try something like
LastIPCMsgStart+1 too.

[jdduke (slow), 2014-02-15 02:57:08]

https://codereview.chromium.org/142923005/diff/860002/content/browser/rendere...
File content/browser/renderer_host/render_message_filter.cc (right):

https://codereview.chromium.org/142923005/diff/860002/content/browser/rendere...
content/browser/renderer_host/render_message_filter.cc:413:
IPC_MESSAGE_HANDLER(FrameHostMsg_GetPluginInfo, OnGetPluginInfo)
On 2014/02/14 22:05:05, piman wrote:
> Needs FrameMsgStart in the category list

Ugg, good catch.

https://codereview.chromium.org/142923005/diff/860002/ipc/ipc_channel_proxy_u...
File ipc/ipc_channel_proxy_unittest.cc (right):

https://codereview.chromium.org/142923005/diff/860002/ipc/ipc_channel_proxy_u...
ipc/ipc_channel_proxy_unittest.cc:169: thread_->message_loop()->RunUntilIdle();
On 2014/02/14 22:05:05, piman wrote:
> I don't think this does what you want it to do.
> If you look at the implementation, it will create a RunLoop, which will run
the
> *current* message loop - it makes sense, you can't run another thread's ML on
> this thread.
> It's already running anyway.

Done.

https://codereview.chromium.org/142923005/diff/860002/ipc/ipc_channel_proxy_u...
ipc/ipc_channel_proxy_unittest.cc:192: Send(sender(), -1, SEND);
On 2014/02/14 22:05:05, piman wrote:
> nit: -1 is special (for replies, etc.). You may want to try something like
> LastIPCMsgStart+1 too.

Done.

[piman, 2014-02-15 23:43:33]

lgtm

[jdduke (slow), 2014-02-16 01:01:58]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-16 01:02:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/1360001

[commit-bot: I haz the power, 2014-02-16 01:18:43]

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

[commit-bot: I haz the power, 2014-02-16 01:18:44]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[jdduke (slow), 2014-02-16 01:21:35]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-16 01:21:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/1360001

[commit-bot: I haz the power, 2014-02-16 03:10:17]

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

[commit-bot: I haz the power, 2014-02-16 03:10:19]

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

[jdduke (slow), 2014-02-16 03:21:32]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-16 03:21:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/1690001

[commit-bot: I haz the power, 2014-02-16 03:48:13]

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

[commit-bot: I haz the power, 2014-02-16 03:48:14]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[jdduke (slow), 2014-02-16 03:49:09]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-16 03:49:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/1690001

[commit-bot: I haz the power, 2014-02-16 04:37:26]

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

[commit-bot: I haz the power, 2014-02-16 04:37:28]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[jdduke (slow), 2014-02-16 04:38:04]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-16 04:38:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/1690001

[commit-bot: I haz the power, 2014-02-16 22:11:37]

Change committed as 251622

[jdduke (slow), 2014-02-24 23:21:27]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-24 23:25:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2080001

[commit-bot: I haz the power, 2014-02-25 00:20:31]

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

[commit-bot: I haz the power, 2014-02-25 00:20:32]

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

[jdduke (slow), 2014-02-25 00:36:25]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 00:42:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2100001

[commit-bot: I haz the power, 2014-02-25 01:49:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2100001

[commit-bot: I haz the power, 2014-02-25 04:56:24]

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

[commit-bot: I haz the power, 2014-02-25 04:56:26]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 04:58:43]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 05:01:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2100001

[commit-bot: I haz the power, 2014-02-25 05:02:08]

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

[commit-bot: I haz the power, 2014-02-25 05:02:09]

Failed to apply patch for
content/browser/screen_orientation/screen_orientation_dispatcher_host.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index:
content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
  |diff --git
a/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
b/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
  |index
c868beba44f03262c4b3b94bd9a627009aa1f8d2..261bd89811833665c0b6bb10c4899ef7651ef606
100644
  |---
a/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
  |+++
b/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:      
content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
Index: content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
diff --git
a/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
b/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
index
c868beba44f03262c4b3b94bd9a627009aa1f8d2..261bd89811833665c0b6bb10c4899ef7651ef606
100644
--- a/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
+++ b/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc
@@ -8,7 +8,8 @@
 
 namespace content {
 
-ScreenOrientationDispatcherHost::ScreenOrientationDispatcherHost() {
+ScreenOrientationDispatcherHost::ScreenOrientationDispatcherHost()
+    : BrowserMessageFilter(ScreenOrientationMsgStart) {
 }
 
 bool ScreenOrientationDispatcherHost::OnMessageReceived(

[jdduke (slow), 2014-02-25 05:09:17]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 05:12:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 07:44:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 09:14:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 09:32:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 09:43:18]

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

[commit-bot: I haz the power, 2014-02-25 09:43:19]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 10:12:34]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 10:14:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 10:16:04]

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

[commit-bot: I haz the power, 2014-02-25 10:16:06]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 10:16:58]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 10:19:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 10:22:52]

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

[commit-bot: I haz the power, 2014-02-25 10:22:54]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 12:47:16]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 12:47:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 12:49:09]

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

[commit-bot: I haz the power, 2014-02-25 12:49:12]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 13:45:35]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 13:46:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 13:49:14]

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

[commit-bot: I haz the power, 2014-02-25 13:49:16]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 14:01:13]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 14:01:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2160001

[commit-bot: I haz the power, 2014-02-25 14:03:29]

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

[commit-bot: I haz the power, 2014-02-25 14:03:31]

Try jobs failed on following builders: mac_chromium_rel

[jdduke (slow), 2014-02-25 15:40:38]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 15:40:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2180001

[commit-bot: I haz the power, 2014-02-25 21:45:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2180001

[commit-bot: I haz the power, 2014-02-25 22:09:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2180001

[commit-bot: I haz the power, 2014-02-25 23:07:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2180001

[commit-bot: I haz the power, 2014-02-25 23:07:30]

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

[commit-bot: I haz the power, 2014-02-25 23:07:31]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

The read operation timed out

[jdduke (slow), 2014-02-25 23:21:24]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-02-25 23:58:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/142923005/2180001

[commit-bot: I haz the power, 2014-02-26 05:18:10]

Change committed as 253361