mac: Expose keychain access frequency to Telemetry.

tools/perf/metrics/keychain_metric.py

+# Copyright 2014 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.
+
+import json
+import sys
+
+from metrics import Metric
+from telemetry.page import page_test
+from telemetry.value import scalar
+
+
+class KeychainMetric(Metric):
+  """KeychainMetric gathers keychain statistics from the browser object.
+
+  This includes the number of times that the keychain was accessed.
+  """
+
+  DISPLAY_NAME = 'OSX_Keychain_Access'
+  HISTOGRAM_NAME = 'OSX.Keychain.Access'
+  MAX_KEYCHAIN_ACCESS_COUNT = 10
+
+  @classmethod
+  def CustomizeBrowserOptions(cls, options):
+    """Adds a browser argument that allows for the collection of keychain
+    metrics."""
+    options.AppendExtraBrowserArgs(['--enable-stats-collection-bindings'])
+
+  @staticmethod
+  def GetKeychainAccessCount(tab):

[dtu, 2014/12/05 01:12:54]

telemetry.value.histogram_util.GetHistogramSum

[erikchen, 2014/12/08 23:41:25]

Done.

+    """Retrieves the number of times that the keychain was accessed since the
+    browser was started."""
+    get_histogram_js = 'statsCollectionController.getBrowserHistogram("%s")'
+
+    result = tab.EvaluateJavaScript(get_histogram_js %
+        KeychainMetric.HISTOGRAM_NAME)
+    result = json.loads(result)
+
+    access_count = 0
+    if result:
+      access_count = result['sum']
+    return access_count
+
+  def AddResults(self, tab, results):
+    """Adds the number of times that the keychain was accessed to |results|."""
+    access_count = KeychainMetric.GetKeychainAccessCount(tab)
+    results.AddValue(scalar.ScalarValue(
+        results.current_page, KeychainMetric.DISPLAY_NAME, 'count',
+        access_count))
+    if access_count > KeychainMetric.MAX_KEYCHAIN_ACCESS_COUNT:

[tonyg, 2014/12/05 03:12:03]

Correct me if I'm mistaken, but didn't we land on the idea of just reporting
this to the dashboard to be monitored there rather than failing hard?

[erikchen, 2014/12/08 23:41:25]

Yup, removed this line.

+      raise page_test.Failure('Keychain accessed too many times ({0}).'.
+          format(KeychainMetric.MAX_KEYCHAIN_ACCESS_COUNT))
+
+  @staticmethod
+  def ShouldCollectKeychainMetrics():

[dtu, 2014/12/05 01:12:54]

Since all callers are going to check ShouldCollectKeychainMetrics, maybe it's
safer and more modular if the check goes in this class's methods?

[erikchen, 2014/12/08 23:41:25]

Hm. You're right.

I moved the logic into this file. In addition, I renamed the methods to
CustomizeBrowserOptionsMac and AddResultsMac. AddResults throws an assert. 

(If the methods aren't renamed, then KeychainMetric().AddResults(...) has
different effects based on the platform, which is not at all clear based on the
name of the method.)

[dtu, 2014/12/09 00:25:03]

Makes sense.

+    """Whether Chrome collects Keychain metrics on the OS."""
+    return sys.platform == 'darwin'