Add UMA histogram to count hard revokes of user certificate error decisions.

Add UMA histogram to count hard revokes of user certificate error decisions.

In https://codereview.chromium.org/473643002/ we added a button for users in the
"Remember Certificate Error Decisions" experiment to allow them to revoke their
decision to bypass an SSL error for a given host. This commit adds an UMA
histogram to count how often a hard revoke actually occurs.

BUG=262615
R=felt@chromium.org

Committed: https://crrev.com/1ed8ea710ae7616a84432b2a24828edec2080fe4
Cr-Commit-Position: refs/heads/master@{#292993}

[felt, 2014-08-13 23:55:20]

On 2014/08/13 23:49:03, jww wrote:

this looks like a count instead of a histogram -- I've never added a count
before that I can remember. i defer to an owner for review, but i'll follow the
thread so that i can send it through the CQ / fix any issues if needed

[jww, 2014-08-14 00:02:18]

jar@, can you take a look at this histogram addition, both for review and your
opinion?

I feel like this should just be a simple histogram count since it's measuring
the number of times a function is called, but Adrienne pointed out since it's
tied to a button in UX, maybe it should be an action instead?

[jar (doing other things), 2014-08-14 22:10:37]

micro histogram tutorial attached... hope it helps.

Asvitkine can probably provide reviews as this goes forward.  You can stop by to
chat if my commentary is confusing.

Thanks!

https://codereview.chromium.org/476513002/diff/1/chrome/browser/ssl/chrome_ss...
File chrome/browser/ssl/chrome_ssl_host_state_delegate.cc (right):

https://codereview.chromium.org/476513002/diff/1/chrome/browser/ssl/chrome_ss...
chrome/browser/ssl/chrome_ssl_host_state_delegate.cc:343:
UMA_HISTOGRAM_COUNTS("interstitial.ssl.revoke_user_decisions_hard", 1);
Several issues/suggestions:

Yeah, this would in a strange way generate a count... but it would be
meaningless, as there would be no way to normalize it.  Suppose you find that
you get 200,000 times that a user walked through this code... what would that
tell you?  How would that show you a different answer from it saying 400,000?? 
Ans: Nothing really.

You would much rather (for example) look at how many times a user saw an
interstitial, and count clicked through, vs back away (cancel).  Hence you would
(in that case) sample *either* a counter for "CLICK_THROUGH" or for
"CANCEL_REQUEST".  The histogram to generally use for that would be
UMA_HISTOGRAM_ENUMERATION("some name", sample_enum,
LARGER_THAN_ANY_SAMPLED_ENUMS).

You might also want to normalize relative to the number of times folks did an
SSL validation (and hence didn't even get the need to click through), as well as
how many times they hit a problem where they couldn't click through (i.e.,
perhaps there would be four valid enum values??).

IF you wanted to only do a count, you wouldn't use this macro
(UMA_HISTOGRAM_COUNTS), as it expects samples in the range of about 1 to 10,000,
and puts it into 1 of 50 distinct buckets.  That macro is more useful in trying
to (for example) record how many frames were on a page (that might be the
sample), or how many KB of data was loaded during a page load, etc.  It wouldn't
be used for a singleton count.

...and if you really just wanted a random counter, showing (without
normalization!) how many times a feature was used, you *might* use an Event
counter rather than a histogram... but that is a different story... and less
commonly useful.

[felt, 2014-08-14 22:35:54]

Joel is OOO, so I'm going to try to take over this CL from him. Two quick notes:

* I suspect that this event will be extremely rare. An absolute count would be
valuable because if it's *not* extremely rare, we might need to rethink things.

* In terms of normalization, we *could* put it in interstitial.ssl because that
records the number of times people proceeded. However, right now the
interstitial.ssl only records things that happened on the SSL blocking page
(whereas this code is triggered from a different UI surface). Should I move it
into interstitial.ssl anyway?

On 2014/08/14 22:10:37, jar wrote:
> micro histogram tutorial attached... hope it helps.
> 
> Asvitkine can probably provide reviews as this goes forward.  You can stop by
to
> chat if my commentary is confusing.
> 
> Thanks!
> 
>
https://codereview.chromium.org/476513002/diff/1/chrome/browser/ssl/chrome_ss...
> File chrome/browser/ssl/chrome_ssl_host_state_delegate.cc (right):
> 
>
https://codereview.chromium.org/476513002/diff/1/chrome/browser/ssl/chrome_ss...
> chrome/browser/ssl/chrome_ssl_host_state_delegate.cc:343:
> UMA_HISTOGRAM_COUNTS("interstitial.ssl.revoke_user_decisions_hard", 1);
> Several issues/suggestions:
> 
> Yeah, this would in a strange way generate a count... but it would be
> meaningless, as there would be no way to normalize it.  Suppose you find that
> you get 200,000 times that a user walked through this code... what would that
> tell you?  How would that show you a different answer from it saying 400,000??

> Ans: Nothing really.
> 
> You would much rather (for example) look at how many times a user saw an
> interstitial, and count clicked through, vs back away (cancel).  Hence you
would
> (in that case) sample *either* a counter for "CLICK_THROUGH" or for
> "CANCEL_REQUEST".  The histogram to generally use for that would be
> UMA_HISTOGRAM_ENUMERATION("some name", sample_enum,
> LARGER_THAN_ANY_SAMPLED_ENUMS).
> 
> You might also want to normalize relative to the number of times folks did an
> SSL validation (and hence didn't even get the need to click through), as well
as
> how many times they hit a problem where they couldn't click through (i.e.,
> perhaps there would be four valid enum values??).
> 
> IF you wanted to only do a count, you wouldn't use this macro
> (UMA_HISTOGRAM_COUNTS), as it expects samples in the range of about 1 to
10,000,
> and puts it into 1 of 50 distinct buckets.  That macro is more useful in
trying
> to (for example) record how many frames were on a page (that might be the
> sample), or how many KB of data was loaded during a page load, etc.  It
wouldn't
> be used for a singleton count.
> 
> ...and if you really just wanted a random counter, showing (without
> normalization!) how many times a feature was used, you *might* use an Event
> counter rather than a histogram... but that is a different story... and less
> commonly useful.

[jar (doing other things), 2014-08-14 23:00:36]

Experience suggests that when you have a bazillion users... and a bazillion
events.... that a "small fraction" is a confusing element without
normalization.

YMMV.


On Thu, Aug 14, 2014 at 3:35 PM, <felt@chromium.org> wrote:

> Joel is OOO, so I'm going to try to take over this CL from him. Two quick
> notes:
>
> * I suspect that this event will be extremely rare. An absolute count
> would be
> valuable because if it's *not* extremely rare, we might need to rethink
> things.
>
> * In terms of normalization, we *could* put it in interstitial.ssl because
> that
> records the number of times people proceeded. However, right now the
> interstitial.ssl only records things that happened on the SSL blocking page
> (whereas this code is triggered from a different UI surface). Should I
> move it
> into interstitial.ssl anyway?
>
>
> On 2014/08/14 22:10:37, jar wrote:
>
>> micro histogram tutorial attached... hope it helps.
>>
>
>  Asvitkine can probably provide reviews as this goes forward.  You can
>> stop by
>>
> to
>
>> chat if my commentary is confusing.
>>
>
>  Thanks!
>>
>
>
> https://codereview.chromium.org/476513002/diff/1/chrome/
> browser/ssl/chrome_ssl_host_state_delegate.cc
>
>> File chrome/browser/ssl/chrome_ssl_host_state_delegate.cc (right):
>>
>
>
> https://codereview.chromium.org/476513002/diff/1/chrome/
> browser/ssl/chrome_ssl_host_state_delegate.cc#newcode343
>
>> chrome/browser/ssl/chrome_ssl_host_state_delegate.cc:343:
>> UMA_HISTOGRAM_COUNTS("interstitial.ssl.revoke_user_decisions_hard", 1);
>> Several issues/suggestions:
>>
>
>  Yeah, this would in a strange way generate a count... but it would be
>> meaningless, as there would be no way to normalize it.  Suppose you find
>> that
>> you get 200,000 times that a user walked through this code... what would
>> that
>> tell you?  How would that show you a different answer from it saying
>> 400,000??
>>
>
>  Ans: Nothing really.
>>
>
>  You would much rather (for example) look at how many times a user saw an
>> interstitial, and count clicked through, vs back away (cancel).  Hence you
>>
> would
>
>> (in that case) sample *either* a counter for "CLICK_THROUGH" or for
>> "CANCEL_REQUEST".  The histogram to generally use for that would be
>> UMA_HISTOGRAM_ENUMERATION("some name", sample_enum,
>> LARGER_THAN_ANY_SAMPLED_ENUMS).
>>
>
>  You might also want to normalize relative to the number of times folks
>> did an
>> SSL validation (and hence didn't even get the need to click through), as
>> well
>>
> as
>
>> how many times they hit a problem where they couldn't click through (i.e.,
>> perhaps there would be four valid enum values??).
>>
>
>  IF you wanted to only do a count, you wouldn't use this macro
>> (UMA_HISTOGRAM_COUNTS), as it expects samples in the range of about 1 to
>>
> 10,000,
>
>> and puts it into 1 of 50 distinct buckets.  That macro is more useful in
>>
> trying
>
>> to (for example) record how many frames were on a page (that might be the
>> sample), or how many KB of data was loaded during a page load, etc.  It
>>
> wouldn't
>
>> be used for a singleton count.
>>
>
>  ...and if you really just wanted a random counter, showing (without
>> normalization!) how many times a feature was used, you *might* use an
>> Event
>> counter rather than a histogram... but that is a different story... and
>> less
>> commonly useful.
>>
>
>
>
> https://codereview.chromium.org/476513002/
>

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

[jww, 2014-08-18 22:29:26]

On 2014/08/14 23:00:36, jar wrote:
> Experience suggests that when you have a bazillion users... and a bazillion
> events.... that a "small fraction" is a confusing element without
> normalization.
> 
> YMMV.
> 
> 
> On Thu, Aug 14, 2014 at 3:35 PM, <mailto:felt@chromium.org> wrote:
> 
> > Joel is OOO, so I'm going to try to take over this CL from him. Two quick
> > notes:
> >
> > * I suspect that this event will be extremely rare. An absolute count
> > would be
> > valuable because if it's *not* extremely rare, we might need to rethink
> > things.
> >
> > * In terms of normalization, we *could* put it in interstitial.ssl because
> > that
> > records the number of times people proceeded. However, right now the
> > interstitial.ssl only records things that happened on the SSL blocking page
> > (whereas this code is triggered from a different UI surface). Should I
> > move it
> > into interstitial.ssl anyway?
> >
> >
> > On 2014/08/14 22:10:37, jar wrote:
> >
> >> micro histogram tutorial attached... hope it helps.
> >>
> >
> >  Asvitkine can probably provide reviews as this goes forward.  You can
> >> stop by
> >>
> > to
> >
> >> chat if my commentary is confusing.
> >>
> >
> >  Thanks!
> >>
> >
> >
> > https://codereview.chromium.org/476513002/diff/1/chrome/
> > browser/ssl/chrome_ssl_host_state_delegate.cc
> >
> >> File chrome/browser/ssl/chrome_ssl_host_state_delegate.cc (right):
> >>
> >
> >
> > https://codereview.chromium.org/476513002/diff/1/chrome/
> > browser/ssl/chrome_ssl_host_state_delegate.cc#newcode343
> >
> >> chrome/browser/ssl/chrome_ssl_host_state_delegate.cc:343:
> >> UMA_HISTOGRAM_COUNTS("interstitial.ssl.revoke_user_decisions_hard", 1);
> >> Several issues/suggestions:
> >>
> >
> >  Yeah, this would in a strange way generate a count... but it would be
> >> meaningless, as there would be no way to normalize it.  Suppose you find
> >> that
> >> you get 200,000 times that a user walked through this code... what would
> >> that
> >> tell you?  How would that show you a different answer from it saying
> >> 400,000??
> >>
> >
> >  Ans: Nothing really.
> >>
> >
> >  You would much rather (for example) look at how many times a user saw an
> >> interstitial, and count clicked through, vs back away (cancel).  Hence you
> >>
> > would
> >
> >> (in that case) sample *either* a counter for "CLICK_THROUGH" or for
> >> "CANCEL_REQUEST".  The histogram to generally use for that would be
> >> UMA_HISTOGRAM_ENUMERATION("some name", sample_enum,
> >> LARGER_THAN_ANY_SAMPLED_ENUMS).
> >>
> >
> >  You might also want to normalize relative to the number of times folks
> >> did an
> >> SSL validation (and hence didn't even get the need to click through), as
> >> well
> >>
> > as
> >
> >> how many times they hit a problem where they couldn't click through (i.e.,
> >> perhaps there would be four valid enum values??).
> >>
> >
> >  IF you wanted to only do a count, you wouldn't use this macro
> >> (UMA_HISTOGRAM_COUNTS), as it expects samples in the range of about 1 to
> >>
> > 10,000,
> >
> >> and puts it into 1 of 50 distinct buckets.  That macro is more useful in
> >>
> > trying
> >
> >> to (for example) record how many frames were on a page (that might be the
> >> sample), or how many KB of data was loaded during a page load, etc.  It
> >>
> > wouldn't
> >
> >> be used for a singleton count.
> >>
> >
> >  ...and if you really just wanted a random counter, showing (without
> >> normalization!) how many times a feature was used, you *might* use an
> >> Event
> >> counter rather than a histogram... but that is a different story... and
> >> less
> >> commonly useful.
> >>
> >
> >
> >
> > https://codereview.chromium.org/476513002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Okay, I'

[jww, 2014-08-18 22:30:28]

On 2014/08/18 22:29:26, jww wrote:
> On 2014/08/14 23:00:36, jar wrote:
> > Experience suggests that when you have a bazillion users... and a bazillion
> > events.... that a "small fraction" is a confusing element without
> > normalization.
> > 
> > YMMV.
> > 
> > 
> > On Thu, Aug 14, 2014 at 3:35 PM, <mailto:felt@chromium.org> wrote:
> > 
> > > Joel is OOO, so I'm going to try to take over this CL from him. Two quick
> > > notes:
> > >
> > > * I suspect that this event will be extremely rare. An absolute count
> > > would be
> > > valuable because if it's *not* extremely rare, we might need to rethink
> > > things.
> > >
> > > * In terms of normalization, we *could* put it in interstitial.ssl because
> > > that
> > > records the number of times people proceeded. However, right now the
> > > interstitial.ssl only records things that happened on the SSL blocking
page
> > > (whereas this code is triggered from a different UI surface). Should I
> > > move it
> > > into interstitial.ssl anyway?
> > >
> > >
> > > On 2014/08/14 22:10:37, jar wrote:
> > >
> > >> micro histogram tutorial attached... hope it helps.
> > >>
> > >
> > >  Asvitkine can probably provide reviews as this goes forward.  You can
> > >> stop by
> > >>
> > > to
> > >
> > >> chat if my commentary is confusing.
> > >>
> > >
> > >  Thanks!
> > >>
> > >
> > >
> > > https://codereview.chromium.org/476513002/diff/1/chrome/
> > > browser/ssl/chrome_ssl_host_state_delegate.cc
> > >
> > >> File chrome/browser/ssl/chrome_ssl_host_state_delegate.cc (right):
> > >>
> > >
> > >
> > > https://codereview.chromium.org/476513002/diff/1/chrome/
> > > browser/ssl/chrome_ssl_host_state_delegate.cc#newcode343
> > >
> > >> chrome/browser/ssl/chrome_ssl_host_state_delegate.cc:343:
> > >> UMA_HISTOGRAM_COUNTS("interstitial.ssl.revoke_user_decisions_hard", 1);
> > >> Several issues/suggestions:
> > >>
> > >
> > >  Yeah, this would in a strange way generate a count... but it would be
> > >> meaningless, as there would be no way to normalize it.  Suppose you find
> > >> that
> > >> you get 200,000 times that a user walked through this code... what would
> > >> that
> > >> tell you?  How would that show you a different answer from it saying
> > >> 400,000??
> > >>
> > >
> > >  Ans: Nothing really.
> > >>
> > >
> > >  You would much rather (for example) look at how many times a user saw an
> > >> interstitial, and count clicked through, vs back away (cancel).  Hence
you
> > >>
> > > would
> > >
> > >> (in that case) sample *either* a counter for "CLICK_THROUGH" or for
> > >> "CANCEL_REQUEST".  The histogram to generally use for that would be
> > >> UMA_HISTOGRAM_ENUMERATION("some name", sample_enum,
> > >> LARGER_THAN_ANY_SAMPLED_ENUMS).
> > >>
> > >
> > >  You might also want to normalize relative to the number of times folks
> > >> did an
> > >> SSL validation (and hence didn't even get the need to click through), as
> > >> well
> > >>
> > > as
> > >
> > >> how many times they hit a problem where they couldn't click through
(i.e.,
> > >> perhaps there would be four valid enum values??).
> > >>
> > >
> > >  IF you wanted to only do a count, you wouldn't use this macro
> > >> (UMA_HISTOGRAM_COUNTS), as it expects samples in the range of about 1 to
> > >>
> > > 10,000,
> > >
> > >> and puts it into 1 of 50 distinct buckets.  That macro is more useful in
> > >>
> > > trying
> > >
> > >> to (for example) record how many frames were on a page (that might be the
> > >> sample), or how many KB of data was loaded during a page load, etc.  It
> > >>
> > > wouldn't
> > >
> > >> be used for a singleton count.
> > >>
> > >
> > >  ...and if you really just wanted a random counter, showing (without
> > >> normalization!) how many times a feature was used, you *might* use an
> > >> Event
> > >> counter rather than a histogram... but that is a different story... and
> > >> less
> > >> commonly useful.
> > >>
> > >
> > >
> > >
> > > https://codereview.chromium.org/476513002/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> Okay, I'

Whoops, aborted message!

Okay, I'm convinced. I'll try and figure out how to normalize this best.

[jww, 2014-08-19 00:36:18]

I've moved the UMA stats to website_settings so we how can get a baseline based
on the number of times the page info box is closed. felt@ and asvitkine@, can
you take another look? Thanks!

[felt, 2014-08-19 00:46:28]

On 2014/08/19 00:36:18, jww wrote:
> I've moved the UMA stats to website_settings so we how can get a baseline
based
> on the number of times the page info box is closed. felt@ and asvitkine@, can
> you take another look? Thanks!

Why'd you choose that as the baseline, vs the number of participants in the
experiment
or the number of saved certs?

[jww, 2014-08-19 01:34:39]

On 2014/08/19 00:46:28, felt wrote:
> On 2014/08/19 00:36:18, jww wrote:
> > I've moved the UMA stats to website_settings so we how can get a baseline
> based
> > on the number of times the page info box is closed. felt@ and asvitkine@,
can
> > you take another look? Thanks!
> 
> Why'd you choose that as the baseline, vs the number of participants in the
> experiment
> or the number of saved certs?

We get the experiment info for free in UMA (that is, we'll be able to trivially
partition the data based on experiment since the experiment config specifically
lists interstitials.ssl.* as the histograms of interest).

There's no particular reason I didn't measure over the number of saved certs,
other than this is technically much simpler, and gathers other information
(usage of page info per experimental group) that is also interesting.

[Alexei Svitkine (slow), 2014-08-21 19:36:12]

https://codereview.chromium.org/476513002/diff/60001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.cc (right):

https://codereview.chromium.org/476513002/diff/60001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.cc:334:
UMA_HISTOGRAM_ENUMERATION(
Nit: Can you avoid having two separate macro blocks for the same histogram? Each
one expands to a bunch of code and bloats the binary.

https://codereview.chromium.org/476513002/diff/60001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/476513002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:10317: +    pressed the SSL decisions
revoke button. This can only by done if the user
Please clarify when this is logged. Is it when the page info menu is closed? If
so, say so.

https://codereview.chromium.org/476513002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:37839: +<enum
name="BooleanRevokeUserCertificateDecisions" type="int">
Nit: Make this just BooleanRevoked, since the enum values are not specific to
this histogram (and could be used by other histograms).

[jww, 2014-08-21 20:26:36]

https://codereview.chromium.org/476513002/diff/60001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.cc (right):

https://codereview.chromium.org/476513002/diff/60001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.cc:334:
UMA_HISTOGRAM_ENUMERATION(
On 2014/08/21 19:36:11, Alexei Svitkine wrote:
> Nit: Can you avoid having two separate macro blocks for the same histogram?
Each
> one expands to a bunch of code and bloats the binary.

Done.

https://codereview.chromium.org/476513002/diff/60001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/476513002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:10317: +    pressed the SSL decisions
revoke button. This can only by done if the user
On 2014/08/21 19:36:11, Alexei Svitkine wrote:
> Please clarify when this is logged. Is it when the page info menu is closed?
If
> so, say so.

Done.

https://codereview.chromium.org/476513002/diff/60001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:37839: +<enum
name="BooleanRevokeUserCertificateDecisions" type="int">
On 2014/08/21 19:36:11, Alexei Svitkine wrote:
> Nit: Make this just BooleanRevoked, since the enum values are not specific to
> this histogram (and could be used by other histograms).

Done.

[Alexei Svitkine (slow), 2014-08-22 22:05:32]

lgtm

[jww, 2014-08-22 22:40:14]

pkasting: can you look at chrome/browser/website_settings/*?

tedchoc: can you look at
chrome/browser/ui/android/website_settings_popup_android.cc?

rsesek: can you look at 
chrome/browser/ui/cocoa/website_settings/website_settings_bubble_controller.mm?

Thanks!

[Peter Kasting, 2014-08-22 22:44:00]

LGTM

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.cc (right):

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.cc:335: :
USER_CERT_DECISIONS_NOT_REVOKED;
Nit: clang-format error; wrap like:

      did_revoke_user_ssl_decisions_ ?
          USER_CERT_DECISIONS_REVOKED : USER_CERT_DECISIONS_NOT_REVOKED;

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.h (right):

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.h:96: // This method is
called when the SSL decisions revoke button is pressed.
Nit: "SSL decisions revoke button" feels a little confusing and awkward. 
"Revoke SSL error bypass button"?  I dunno.  (Applies to function name and to
other files too.)

[Ted C, 2014-08-22 22:46:27]

On 2014/08/22 22:44:00, Peter Kasting wrote:
> LGTM
> 
>
https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
> File chrome/browser/ui/website_settings/website_settings.cc (right):
> 
>
https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
> chrome/browser/ui/website_settings/website_settings.cc:335: :
> USER_CERT_DECISIONS_NOT_REVOKED;
> Nit: clang-format error; wrap like:
> 
>       did_revoke_user_ssl_decisions_ ?
>           USER_CERT_DECISIONS_REVOKED : USER_CERT_DECISIONS_NOT_REVOKED;
> 
>
https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
> File chrome/browser/ui/website_settings/website_settings.h (right):
> 
>
https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
> chrome/browser/ui/website_settings/website_settings.h:96: // This method is
> called when the SSL decisions revoke button is pressed.
> Nit: "SSL decisions revoke button" feels a little confusing and awkward. 
> "Revoke SSL error bypass button"?  I dunno.  (Applies to function name and to
> other files too.)

chrome/browser/ui/android/website_settings_popup_android.cc - lgtm

[jww, 2014-08-22 22:56:39]

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.cc (right):

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.cc:335: :
USER_CERT_DECISIONS_NOT_REVOKED;
On 2014/08/22 22:44:00, Peter Kasting wrote:
> Nit: clang-format error; wrap like:
> 
>       did_revoke_user_ssl_decisions_ ?
>           USER_CERT_DECISIONS_REVOKED : USER_CERT_DECISIONS_NOT_REVOKED;

I'm going to push back on this. This instance was not a clang-format decision;
this matches what is done throughout the rest of this file (see lines 108 - 119,
for example).

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.h (right):

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.h:96: // This method is
called when the SSL decisions revoke button is pressed.
On 2014/08/22 22:44:00, Peter Kasting wrote:
> Nit: "SSL decisions revoke button" feels a little confusing and awkward. 
> "Revoke SSL error bypass button"?  I dunno.  (Applies to function name and to
> other files too.)

Done.

[Peter Kasting, 2014-08-22 23:02:22]

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
File chrome/browser/ui/website_settings/website_settings.cc (right):

https://codereview.chromium.org/476513002/diff/80001/chrome/browser/ui/websit...
chrome/browser/ui/website_settings/website_settings.cc:335: :
USER_CERT_DECISIONS_NOT_REVOKED;
On 2014/08/22 22:56:39, jww wrote:
> On 2014/08/22 22:44:00, Peter Kasting wrote:
> > Nit: clang-format error; wrap like:
> > 
> >       did_revoke_user_ssl_decisions_ ?
> >           USER_CERT_DECISIONS_REVOKED : USER_CERT_DECISIONS_NOT_REVOKED;
> 
> I'm going to push back on this. This instance was not a clang-format decision;
> this matches what is done throughout the rest of this file (see lines 108 -
119,
> for example).

It's a nit, so you can override me.  And being consistent with the file is a
fine reason too.  Historically, this style never existed in Chromium until
clang-format introduced it, and despite some discussions that I thought ended
with someone agreeing to fix clang-format not to do it, it still seems to crop
up frequently, so I do my best to prevent it spreading.

[Robert Sesek, 2014-08-25 17:04:56]

cocoa/ LGTM

[jww, 2014-09-02 18:51:49]

The CQ bit was checked by jww@chromium.org

[commit-bot: I haz the power, 2014-09-02 18:53:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jww@chromium.org/476513002/120001

[commit-bot: I haz the power, 2014-09-02 20:44:42]

Committed patchset #7 (id:120001) as e09d50151ccbf3029fa54788dd48e9793d339d37

[commit-bot: I haz the power, 2014-09-10 03:20:38]

Patchset 7 (id:??) landed as
https://crrev.com/1ed8ea710ae7616a84432b2a24828edec2080fe4
Cr-Commit-Position: refs/heads/master@{#292993}