Added of new UMA signals in order to be able to discover early if the "save password" feature gets …

chrome/browser/password_manager/password_manager_metrics_util_unittest.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/password_manager/password_manager_metrics_util.h"
+
+#include <iterator>
+#include <map>
+
+#include "base/basictypes.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(PasswordManagerTest, MonitoredDomainGroupAssigmentTest) {
+  const char* const kMonitoredWebsite[] = {

[Ilya Sherman, 2013/09/12 04:45:45]

nit: kMonitoredWebsites (plural)

[jdomingos, 2013/09/12 12:01:01]

Done.

+      "https://www.google.com",
+      "https://www.yahoo.com",
+      "https://www.baidu.com",
+      "https://www.wikipedia.org",
+      "https://www.linkedin.com",
+      "https://www.twitter.com",
+      "https://www.live.com",
+      "https://www.amazon.com",
+      "https://www.ebay.com",
+      "https://www.tumblr.com",
+  };
+  const size_t kMonitoredWebsiteLength = arraysize(kMonitoredWebsite);
+
+  // The |groups| map contains the group id and the number of times
+  // it get assigned.
+  std::map<unsigned int, unsigned int> groups;

[Ilya Sherman, 2013/09/12 04:45:45]

Please always use "size_t" rather than "unsigned int".

[jdomingos, 2013/09/12 12:01:01]

Done.

+
+  // Provide all possible values of the group id parameter for each monitored
+  // website.
+  for (size_t i = 0; i < kMonitoredWebsiteLength; ++i) {
+    for (size_t j = 0; j < password_manager_metrics_util::kGroupsPerDomain;
+         j++) {

[Ilya Sherman, 2013/09/12 04:45:45]

nit: ++j

[jdomingos, 2013/09/12 12:01:01]

Done.

+      password_manager_metrics_util::SetRandomIdForTesting(j);
+      ++groups[password_manager_metrics_util::MonitoredDomainGroupId(
+            kMonitoredWebsite[i])];
+    }
+  }
+
+  // Check if all groups get assigned the same number of time.

[Ilya Sherman, 2013/09/12 04:45:45]

nit: "time" -> "times"

[jdomingos, 2013/09/12 12:01:01]

Done.

+  unsigned int number_of_assigment = groups[1];

[Ilya Sherman, 2013/09/12 04:45:45]

Why groups[1] rather than, say, groups[0] or groups.begin()?

[jdomingos, 2013/09/12 12:01:01]

Done.

+  for (std::map<unsigned int, unsigned int>::iterator it = groups.begin();
+       it != groups.end(); ++it) {
+    EXPECT_EQ(it->second, number_of_assigment) << " group id = " << it->first;
+  }
+}
+
+TEST(PasswordManagerTest, MonitoredDomainGroupTest) {
+  unsigned int group_id;
+
+  group_id = password_manager_metrics_util::MonitoredDomainGroupId(
+      "https://www.linkedin.com");
+  EXPECT_TRUE(group_id > 0);

[Ilya Sherman, 2013/09/12 04:45:45]

nit: For readability's sake, it might be helpful to decompose an "IsMonitored()"
helper function that you can call here.

[vabr (Chromium), 2013/09/12 08:51:04]

Also, this is better written as
EXPECT_GT(group_id, 0)

The difference is that in the latter case the value of group_id will be part of
the log in case of failure, whereas EXPECT_TRUE only says if the comparison was
true or false (which is already clear from there being a failure).

[jdomingos, 2013/09/12 12:01:01]

Done.

[jdomingos, 2013/09/12 12:01:01]

Done.

+  group_id = password_manager_metrics_util::MonitoredDomainGroupId(
+      "https://www.amazon.com");
+  EXPECT_TRUE(group_id > 0);
+  group_id = password_manager_metrics_util::MonitoredDomainGroupId(
+      "https://www.facebook.com");
+  EXPECT_TRUE(group_id == 0);
+  group_id = password_manager_metrics_util::MonitoredDomainGroupId(
+      "http://wikipedia.org");
+  EXPECT_TRUE(group_id > 0);
+  group_id = password_manager_metrics_util::MonitoredDomainGroupId(
+      "http://thisisnotwikipedia.org");
+  EXPECT_TRUE(group_id == 0);
+}