Remove redundancy in security interstitial UMA logic

Remove redundancy in security interstitial UMA logic

Currently, very similar events are being recorded in different ways between the
Safe Browsing and SSL interstitials. This unifies the event types and shares a
common helper method between the two. It should be extensible to future
interstitials. It also has the pleasant side effect of reducing the overwhelming
complexity of the previous interstitial.ssl histogram.

BUG=448487
Committed: https://crrev.com/4cd6e60d57c83b5f7b3a6de4497329de7822e316
Cr-Commit-Position: refs/heads/master@{#312042}

[felt, 2015-01-09 02:54:00]

felt@chromium.org changed reviewers:
+ mattm@chromium.org, meacer@chromium.org

[felt, 2015-01-09 02:58:20]

mattm, meacer:

I started this CL to remove redundancy in the security interstitial UMA logic.
The idea is that I want to eventually have comparable/parallel UMA stats -- with
events handled in identical parallel ways -- for all of the security
interstitials. This makes sense to me because right now there is a substantial
amount of shared code between the two. I thought I would put the new, shared
methods in the SecurityInterstitialPage.

My eventual goal is to for the ssl_blocking_page and safe_browsing_blocking_page
code to be completely parallel, with as little redundancy as possible.

However, to do this the SecurityInterstitialPage has to know which type of error
is being displayed. meacer@ brings it to my attention that this can be
considered a layering violation.

This CL isn't finished (probably doesn't even compile), but I wanted to ask you
about the layering issue before I got any farther.

WDYT?

Thanks
felt

[mattm, 2015-01-09 04:01:53]

On 2015/01/09 02:58:20, felt wrote:
> mattm, meacer:
> 
> I started this CL to remove redundancy in the security interstitial UMA logic.
> The idea is that I want to eventually have comparable/parallel UMA stats --
with
> events handled in identical parallel ways -- for all of the security
> interstitials. This makes sense to me because right now there is a substantial
> amount of shared code between the two. I thought I would put the new, shared
> methods in the SecurityInterstitialPage.
> 
> My eventual goal is to for the ssl_blocking_page and
safe_browsing_blocking_page
> code to be completely parallel, with as little redundancy as possible.
> 
> However, to do this the SecurityInterstitialPage has to know which type of
error
> is being displayed. meacer@ brings it to my attention that this can be
> considered a layering violation.
> 
> This CL isn't finished (probably doesn't even compile), but I wanted to ask
you
> about the layering issue before I got any farther.
> 
> WDYT?
> 
> Thanks
> felt

Yeah, I don't really like this approach since you're basically moving
implementation details of the children into the parent.

For the specific case of UMA, instead of making the reason enum be part of the
parent, you could have a virtual method like "GetHistogramSuffix" that the child
implements and then the parent could implement the histograms by using suffixed
histograms instead of the histogram macros.
Ex: https://codereview.chromium.org/815863002/ 
Also see the note about inserting in the middle of a histogram:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...
I think you could use that to generate the histogram names like
"interstitial.<foo>.decision".

Might want to ask a histograms expert about doing this with existing histograms
though, just to make sure there are no gotchas.

[felt, 2015-01-10 00:48:09]

On 2015/01/09 04:01:53, mattm wrote:
> On 2015/01/09 02:58:20, felt wrote:
> > mattm, meacer:
> > 
> > I started this CL to remove redundancy in the security interstitial UMA
logic.
> > The idea is that I want to eventually have comparable/parallel UMA stats --
> with
> > events handled in identical parallel ways -- for all of the security
> > interstitials. This makes sense to me because right now there is a
substantial
> > amount of shared code between the two. I thought I would put the new, shared
> > methods in the SecurityInterstitialPage.
> > 
> > My eventual goal is to for the ssl_blocking_page and
> safe_browsing_blocking_page
> > code to be completely parallel, with as little redundancy as possible.
> > 
> > However, to do this the SecurityInterstitialPage has to know which type of
> error
> > is being displayed. meacer@ brings it to my attention that this can be
> > considered a layering violation.
> > 
> > This CL isn't finished (probably doesn't even compile), but I wanted to ask
> you
> > about the layering issue before I got any farther.
> > 
> > WDYT?
> > 
> > Thanks
> > felt
> 
> Yeah, I don't really like this approach since you're basically moving
> implementation details of the children into the parent.
> 
> For the specific case of UMA, instead of making the reason enum be part of the
> parent, you could have a virtual method like "GetHistogramSuffix" that the
child
> implements and then the parent could implement the histograms by using
suffixed
> histograms instead of the histogram macros.
> Ex: https://codereview.chromium.org/815863002/ 
> Also see the note about inserting in the middle of a histogram:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...
> I think you could use that to generate the histogram names like
> "interstitial.<foo>.decision".
> 
> Might want to ask a histograms expert about doing this with existing
histograms
> though, just to make sure there are no gotchas.

OK, what do you think of this approach now? (Note: still not finished for a real
review, but you should be able to see the tack I'm taking.)

[mattm, 2015-01-12 23:35:40]

On 2015/01/10 00:48:09, felt wrote:
> On 2015/01/09 04:01:53, mattm wrote:
> > On 2015/01/09 02:58:20, felt wrote:
> > > mattm, meacer:
> > > 
> > > I started this CL to remove redundancy in the security interstitial UMA
> logic.
> > > The idea is that I want to eventually have comparable/parallel UMA stats
--
> > with
> > > events handled in identical parallel ways -- for all of the security
> > > interstitials. This makes sense to me because right now there is a
> substantial
> > > amount of shared code between the two. I thought I would put the new,
shared
> > > methods in the SecurityInterstitialPage.
> > > 
> > > My eventual goal is to for the ssl_blocking_page and
> > safe_browsing_blocking_page
> > > code to be completely parallel, with as little redundancy as possible.
> > > 
> > > However, to do this the SecurityInterstitialPage has to know which type of
> > error
> > > is being displayed. meacer@ brings it to my attention that this can be
> > > considered a layering violation.
> > > 
> > > This CL isn't finished (probably doesn't even compile), but I wanted to
ask
> > you
> > > about the layering issue before I got any farther.
> > > 
> > > WDYT?
> > > 
> > > Thanks
> > > felt
> > 
> > Yeah, I don't really like this approach since you're basically moving
> > implementation details of the children into the parent.
> > 
> > For the specific case of UMA, instead of making the reason enum be part of
the
> > parent, you could have a virtual method like "GetHistogramSuffix" that the
> child
> > implements and then the parent could implement the histograms by using
> suffixed
> > histograms instead of the histogram macros.
> > Ex: https://codereview.chromium.org/815863002/ 
> > Also see the note about inserting in the middle of a histogram:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...
> > I think you could use that to generate the histogram names like
> > "interstitial.<foo>.decision".
> > 
> > Might want to ask a histograms expert about doing this with existing
> histograms
> > though, just to make sure there are no gotchas.
> 
> OK, what do you think of this approach now? (Note: still not finished for a
real
> review, but you should be able to see the tack I'm taking.)

Yeah, looks good.

[felt, 2015-01-13 20:51:04]

mattm, please review now.

I decided to move this into a SecurityInterstitialUmaHelper class instead of
putting it in the parent class. I hope that will further ease the concerns about
layering. Let me know what you think.

Cheers
Adrienne

[mattm, 2015-01-14 00:33:04]

Like the approach, just a few questions.

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
File chrome/browser/interstitials/security_interstitial_uma_helper.h (right):

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
chrome/browser/interstitials/security_interstitial_uma_helper.h:9: #include
"chrome/browser/history/history_service.h"
This could be in the .cc?

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
chrome/browser/interstitials/security_interstitial_uma_helper.h:13: #include
"chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h"
I think you could forward declare extensions::ExperienceSamplingEvent

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
chrome/browser/interstitials/security_interstitial_uma_helper.h:52: private:
indentation

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (left):

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:171:
UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_type",
does this histogram need to be marked obsolete?

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:552: return "ssl_nonoverridable";
Is there a reason for the difference here vs how the SB ones use
PROCEED_DISABLED?

[felt, 2015-01-14 00:55:18]

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
File chrome/browser/interstitials/security_interstitial_uma_helper.h (right):

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
chrome/browser/interstitials/security_interstitial_uma_helper.h:9: #include
"chrome/browser/history/history_service.h"
On 2015/01/14 00:33:03, mattm wrote:
> This could be in the .cc?

You're right, moved.

However I had to add:
#include "base/task/cancelable_task_tracker.h"
to the .h for base::CancelableTaskTracker. Should have been there to begin with.

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
chrome/browser/interstitials/security_interstitial_uma_helper.h:13: #include
"chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h"
On 2015/01/14 00:33:03, mattm wrote:
> I think you could forward declare extensions::ExperienceSamplingEvent

Done.

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/interstit...
chrome/browser/interstitials/security_interstitial_uma_helper.h:52: private:
On 2015/01/14 00:33:04, mattm wrote:
> indentation

Done.

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (left):

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:171:
UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_type",
On 2015/01/14 00:33:04, mattm wrote:
> does this histogram need to be marked obsolete?

I'm moving it into SSLErrorClassification::RecordUMAStatistics. I thought maybe
I'd do that in a follow-up CL, but I've added it in here to avoid confusion.

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/80001/chrome/browser/ssl/ssl_b...
chrome/browser/ssl/ssl_blocking_page.cc:552: return "ssl_nonoverridable";
On 2015/01/14 00:33:04, mattm wrote:
> Is there a reason for the difference here vs how the SB ones use
> PROCEED_DISABLED?

ssl_overridable and ssl_nonoverridable are different warnings, visually -- they
have different strings, different buttons, etc.

For Safe Browsing, the overridable and nonoverridable ones are basically the
same.

[mattm, 2015-01-14 01:25:57]

lgtm!

[felt, 2015-01-14 02:28:42]

felt@chromium.org changed reviewers:
+ asvitkine@chromium.org, palmer@chromium.org

[felt, 2015-01-14 02:38:43]

alexei, palmer, PTAL?

alexei: for histograms.xml and SecurityInterstitialUmaHelper; note that I'm not
using the macros to record the histograms

palmer: for chrome/browser/ssl/

[Alexei Svitkine (slow), 2015-01-14 22:12:33]

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
File chrome/browser/interstitials/security_interstitial_uma_helper.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.cc:137: bool
success, int num_visits, base::Time first_visit) {
Nit: When params wrap in a decl, put one param on a separate line.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
File chrome/browser/interstitials/security_interstitial_uma_helper.h (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.h:20: class
SecurityInterstitialUmaHelper {
Should have a comment explaining the purpose of this class.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.h:56:
content::WebContents* web_contents() const;
If it's private, why not use web_contents_ directly? Same below.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:453:
DCHECK(interstitial_reason_ == SB_REASON_MALWARE);
Nit: Can this be a DCHECK_EQ?

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
File chrome/browser/ssl/ssl_blocking_page.cc (left):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_blocking_page.cc:715: for (i = 0; i <
static_cast<int>(extra_info.size()); i++) {
Why not use size_t for i?

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_blocking_page.cc:718: for (; i < 5; i++) {
Use arraysize() macro.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_blocking_page.cc:238: uma_helper_.reset(new
SecurityInterstitialUmaHelper(
Why is it a scoped_ptr and not a member? Same for the other one?

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:11595: +<histogram
name="interstitial.ssl_overridable.interaction"
Consider using histogram_suffixes XML construct (see examples in this file) to
specify the decision and interaction suffixes for all of these histograms.

[palmer, 2015-01-15 01:09:32]

LGTM mod noodles.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
File chrome/browser/interstitials/security_interstitial_uma_helper.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.cc:137: bool
success, int num_visits, base::Time first_visit) {
Or whatever "git cl format" does :)

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:559: NOTREACHED();
Do you have to return string() here, for non-Debug builds to build? I'm not
sure.

[felt, 2015-01-15 06:47:22]

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
File chrome/browser/interstitials/security_interstitial_uma_helper.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.cc:137: bool
success, int num_visits, base::Time first_visit) {
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Nit: When params wrap in a decl, put one param on a separate line.

Done.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
File chrome/browser/interstitials/security_interstitial_uma_helper.h (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.h:20: class
SecurityInterstitialUmaHelper {
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Should have a comment explaining the purpose of this class.

Done.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/intersti...
chrome/browser/interstitials/security_interstitial_uma_helper.h:56:
content::WebContents* web_contents() const;
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> If it's private, why not use web_contents_ directly? Same below.

Done.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:453:
DCHECK(interstitial_reason_ == SB_REASON_MALWARE);
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Nit: Can this be a DCHECK_EQ?

Done.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:559: NOTREACHED();
On 2015/01/15 01:09:32, Chromium Palmer wrote:
> Do you have to return string() here, for non-Debug builds to build? I'm not
> sure.

Good question.

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
File chrome/browser/ssl/ssl_blocking_page.cc (left):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_blocking_page.cc:715: for (i = 0; i <
static_cast<int>(extra_info.size()); i++) {
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Why not use size_t for i?

This code is being deleted

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_blocking_page.cc:718: for (; i < 5; i++) {
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Use arraysize() macro.

This code is being deleted

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
File chrome/browser/ssl/ssl_blocking_page.cc (right):

https://codereview.chromium.org/839183002/diff/100001/chrome/browser/ssl/ssl_...
chrome/browser/ssl/ssl_blocking_page.cc:238: uma_helper_.reset(new
SecurityInterstitialUmaHelper(
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Why is it a scoped_ptr and not a member? Same for the other one?

It needs to be done after calculating |interstitial_reason_|. I've added a
comment to that effect.

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:11595: +<histogram
name="interstitial.ssl_overridable.interaction"
On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> Consider using histogram_suffixes XML construct (see examples in this file) to
> specify the decision and interaction suffixes for all of these histograms.

If I do something like:

<histogram_suffixes name="SecurityInterstitialType" ordering="prefix">
  <suffix name="malware"/>
  <suffix name="phishing"/>
  <suffix name="ssl_nonoverridable"/>
  <suffix name="ssl_overridable"/>
  <affected-histogram name="interstitial..decision"/>
  <affected-histogram name="interstitial..interaction"/>
</histogram_suffixes>

Will that generate interstitial.malware.decision? Or will it yield
interstitial.malware_.decision? I'm asking because the malware and phishing ones
already exist in the format interstitial.malware.decision (without the "_").

[Alexei Svitkine (slow), 2015-01-16 22:14:20]

Sorry for the tardy reply, I was out of the office yesterday and today.

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:11595: +<histogram
name="interstitial.ssl_overridable.interaction"
On 2015/01/15 06:47:22, felt wrote:
> On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> > Consider using histogram_suffixes XML construct (see examples in this file)
to
> > specify the decision and interaction suffixes for all of these histograms.
> 
> If I do something like:
> 
> <histogram_suffixes name="SecurityInterstitialType" ordering="prefix">
>   <suffix name="malware"/>
>   <suffix name="phishing"/>
>   <suffix name="ssl_nonoverridable"/>
>   <suffix name="ssl_overridable"/>
>   <affected-histogram name="interstitial..decision"/>
>   <affected-histogram name="interstitial..interaction"/>
> </histogram_suffixes>
> 
> Will that generate interstitial.malware.decision? Or will it yield
> interstitial.malware_.decision? I'm asking because the malware and phishing
ones
> already exist in the format interstitial.malware.decision (without the "_").

I believe you can specify separator="." to use a dot instead of a _.

So you can specify interstitial.interaction in your above example.

[felt, 2015-01-17 00:32:49]

Thanks, updated to use the prefixes.

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/839183002/diff/100001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:11595: +<histogram
name="interstitial.ssl_overridable.interaction"
On 2015/01/16 22:14:20, Alexei Svitkine wrote:
> On 2015/01/15 06:47:22, felt wrote:
> > On 2015/01/14 22:12:33, Alexei Svitkine wrote:
> > > Consider using histogram_suffixes XML construct (see examples in this
file)
> to
> > > specify the decision and interaction suffixes for all of these histograms.
> > 
> > If I do something like:
> > 
> > <histogram_suffixes name="SecurityInterstitialType" ordering="prefix">
> >   <suffix name="malware"/>
> >   <suffix name="phishing"/>
> >   <suffix name="ssl_nonoverridable"/>
> >   <suffix name="ssl_overridable"/>
> >   <affected-histogram name="interstitial..decision"/>
> >   <affected-histogram name="interstitial..interaction"/>
> > </histogram_suffixes>
> > 
> > Will that generate interstitial.malware.decision? Or will it yield
> > interstitial.malware_.decision? I'm asking because the malware and phishing
> ones
> > already exist in the format interstitial.malware.decision (without the "_").
> 
> I believe you can specify separator="." to use a dot instead of a _.
> 
> So you can specify interstitial.interaction in your above example.

Done.

[Alexei Svitkine (slow), 2015-01-17 00:40:54]

lgtm

[felt, 2015-01-17 00:45:27]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2015-01-17 00:45:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/839183002/140001

[commit-bot: I haz the power, 2015-01-17 00:59:51]

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

[commit-bot: I haz the power, 2015-01-17 00:59:52]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[felt, 2015-01-17 16:41:39]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2015-01-17 16:41:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/839183002/220001

[commit-bot: I haz the power, 2015-01-17 17:04:32]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-01-17 17:05:35]

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