Remove default case from autofill_metrics.cc::GetFieldTypeGroupMetric

Remove default case from autofill_metrics.cc::GetFieldTypeGroupMetric

BUG=267983
TEST=compiles
R=estade@chromium.org

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

[Evan Stade, 2013-09-16 15:46:17]

lgtm
On Sep 13, 2013 8:53 PM, <isherman@chromium.org> wrote:

> Reviewers: Evan Stade,
>
> Description:
> Remove default case from autofill_metrics.cc::**GetFieldTypeGroupMetric
>
> BUG=267983
> TEST=compiles
> R=estade@chromium.org
>
> Please review this at
https://codereview.chromium.**org/23629046/<https://codereview.chromium.org/2...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files (+22, -5 lines):
>   M components/autofill/core/**browser/autofill_metrics.cc
>   M tools/metrics/histograms/**histograms.xml
>
>
> Index: components/autofill/core/**browser/autofill_metrics.cc
> diff --git a/components/autofill/core/**browser/autofill_metrics.cc
> b/components/autofill/core/**browser/autofill_metrics.cc
> index 14d8a646e7d2dff3054c2894e24257**919f3e92fb..**
> e6cff198b6cd38ab16bc41dd51fec3**a2cbef8091 100644
> --- a/components/autofill/core/**browser/autofill_metrics.cc
> +++ b/components/autofill/core/**browser/autofill_metrics.cc
> @@ -56,6 +56,7 @@ enum FieldTypeGroupForMetrics {
>    CREDIT_CARD_NUMBER,
>    CREDIT_CARD_DATE,
>    CREDIT_CARD_TYPE,
> +  PASSWORD,
>    NUM_FIELD_TYPE_GROUPS_FOR_**METRICS
>  };
>
> @@ -92,6 +93,7 @@ int GetFieldTypeGroupMetric(const ServerFieldType
> field_type,
>        break;
>
>      case ::autofill::NAME:
> +    case ::autofill::NAME_BILLING:
>        group = NAME;
>        break;
>
> @@ -100,7 +102,8 @@ int GetFieldTypeGroupMetric(const ServerFieldType
> field_type,
>        break;
>
>      case ::autofill::ADDRESS_HOME:
> -      switch (field_type) {
> +    case ::autofill::ADDRESS_BILLING:
> +      switch (AutofillType(field_type).**GetStorableType()) {
>          case ADDRESS_HOME_LINE1:
>            group = ADDRESS_LINE_1;
>            break;
> @@ -122,6 +125,7 @@ int GetFieldTypeGroupMetric(const ServerFieldType
> field_type,
>          default:
>            NOTREACHED();
>            group = AMBIGUOUS;
> +          break;
>        }
>        break;
>
> @@ -130,6 +134,7 @@ int GetFieldTypeGroupMetric(const ServerFieldType
> field_type,
>        break;
>
>      case ::autofill::PHONE_HOME:
> +    case ::autofill::PHONE_BILLING:
>        group = PHONE;
>        break;
>
> @@ -144,14 +149,23 @@ int GetFieldTypeGroupMetric(const ServerFieldType
> field_type,
>          case ::autofill::CREDIT_CARD_TYPE:
>            group = CREDIT_CARD_TYPE;
>            break;
> -        default:
> +        case ::autofill::CREDIT_CARD_EXP_**MONTH:
> +        case ::autofill::CREDIT_CARD_EXP_2_**DIGIT_YEAR:
> +        case ::autofill::CREDIT_CARD_EXP_4_**DIGIT_YEAR:
> +        case ::autofill::CREDIT_CARD_EXP_**DATE_2_DIGIT_YEAR:
> +        case ::autofill::CREDIT_CARD_EXP_**DATE_4_DIGIT_YEAR:
>            group = CREDIT_CARD_DATE;
> +          break;
> +        default:
> +          NOTREACHED();
> +          group = AMBIGUOUS;
> +          break;
>        }
>        break;
>
> -    default:
> -      NOTREACHED();
> -      group = AMBIGUOUS;
> +    case ::autofill::PASSWORD_FIELD:
> +      group = PASSWORD;
> +      break;
>    }
>
>    // Interpolate the |metric| with the |group|, so that all metrics for a
> given
> Index: tools/metrics/histograms/**histograms.xml
> diff --git a/tools/metrics/histograms/**histograms.xml
> b/tools/metrics/histograms/**histograms.xml
> index d45b24a4965157ae0c0ac83d6e67c0**a1ee30ca2f..**
> 134e7f5b1d87dfa191a273969cf8ce**c8338d9140 100644
> --- a/tools/metrics/histograms/**histograms.xml
> +++ b/tools/metrics/histograms/**histograms.xml
> @@ -18644,6 +18644,9 @@ other types of suffix sets.
>    <int value="42" label="Credit card: date, Unknown"/>
>    <int value="43" label="Credit card: date, Match"/>
>    <int value="44" label="Credit card: date, Mismatch"/>
> +  <int value="45" label="Password, Unknown"/>
> +  <int value="46" label="Password, Match"/>
> +  <int value="47" label="Password, Mismatch"/>
>  </enum>
>
>  <enum name="AutofillUserHappiness" type="int">
>
>
>

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

[commit-bot: I haz the power, 2013-09-16 18:00:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/isherman@chromium.org/23629046/1

[commit-bot: I haz the power, 2013-09-16 18:36:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-16 19:42:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/isherman@chromium.org/23629046/22001

[commit-bot: I haz the power, 2013-09-16 23:18:45]

Change committed as 223463

[Jim Blackler, 2013-11-14 17:33:15]

https://chromiumcodereview.appspot.com/23629046/diff/22001/tools/metrics/hist...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/23629046/diff/22001/tools/metrics/hist...
tools/metrics/histograms/histograms.xml:18647: +  <int value="45"
label="Password, Unknown"/>
Is it possible this should read '48' and not '45'?

The end of this list now reads..

  <int value="36" label="Credit card: name, Unknown"/>
  <int value="37" label="Credit card: name, Match"/>
  <int value="38" label="Credit card: name, Mismatch"/>
  <int value="39" label="Credit card: number, Unknown"/>
  <int value="40" label="Credit card: number, Match"/>
  <int value="41" label="Credit card: number, Mismatch"/>
  <int value="42" label="Credit card: date, Unknown"/>
  <int value="43" label="Credit card: date, Match"/>
  <int value="44" label="Credit card: date, Mismatch"/>
  <int value="45" label="Password, Unknown"/>
  <int value="46" label="Password, Match"/>
  <int value="47" label="Password, Mismatch"/>

.. ie. with nothing between "Credit card: date" and "Password"

But enum FieldTypeGroupForMetrics reads...

enum FieldTypeGroupForMetrics {
//..
  CREDIT_CARD_NAME,
  CREDIT_CARD_NUMBER,
  CREDIT_CARD_DATE,
  CREDIT_CARD_TYPE,
  PASSWORD,
  NUM_FIELD_TYPE_GROUPS_FOR_METRICS
};

So it looks like we're missing three numbers for 'Credit card: Type" which
should occupy the slots 45 to 47 and the Password metrics should occupy slots 48
to 51.  Since I believe FieldTypeGroupForMetrics::CREDIT_CARD_TYPE = 15 (*3 =
45) and FieldTypeGroupForMetrics::PASSWORD = 16 (*3 = 48).

This would explain the mystery '48' type we are seeing on iOS metrics reports,
see bug https://code.google.com/p/chromium/issues/detail?id=317749

[Ilya Sherman, 2013-11-15 06:35:55]

https://chromiumcodereview.appspot.com/23629046/diff/22001/tools/metrics/hist...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/23629046/diff/22001/tools/metrics/hist...
tools/metrics/histograms/histograms.xml:18647: +  <int value="45"
label="Password, Unknown"/>
On 2013/11/14 17:33:15, Jim Blackler wrote:
> Is it possible this should read '48' and not '45'?
> 
> The end of this list now reads..
> 
>   <int value="36" label="Credit card: name, Unknown"/>
>   <int value="37" label="Credit card: name, Match"/>
>   <int value="38" label="Credit card: name, Mismatch"/>
>   <int value="39" label="Credit card: number, Unknown"/>
>   <int value="40" label="Credit card: number, Match"/>
>   <int value="41" label="Credit card: number, Mismatch"/>
>   <int value="42" label="Credit card: date, Unknown"/>
>   <int value="43" label="Credit card: date, Match"/>
>   <int value="44" label="Credit card: date, Mismatch"/>
>   <int value="45" label="Password, Unknown"/>
>   <int value="46" label="Password, Match"/>
>   <int value="47" label="Password, Mismatch"/>
> 
> .. ie. with nothing between "Credit card: date" and "Password"
> 
> But enum FieldTypeGroupForMetrics reads...
> 
> enum FieldTypeGroupForMetrics {
> //..
>   CREDIT_CARD_NAME,
>   CREDIT_CARD_NUMBER,
>   CREDIT_CARD_DATE,
>   CREDIT_CARD_TYPE,
>   PASSWORD,
>   NUM_FIELD_TYPE_GROUPS_FOR_METRICS
> };
> 
> So it looks like we're missing three numbers for 'Credit card: Type" which
> should occupy the slots 45 to 47 and the Password metrics should occupy slots
48
> to 51.  Since I believe FieldTypeGroupForMetrics::CREDIT_CARD_TYPE = 15 (*3 =
> 45) and FieldTypeGroupForMetrics::PASSWORD = 16 (*3 = 48).
> 
> This would explain the mystery '48' type we are seeing on iOS metrics reports,
> see bug https://code.google.com/p/chromium/issues/detail?id=317749

Yes, I believe you are correct.  Thanks for catching that!