[usecounter] Add presubmit to check generation of enums entries for css

Add a PRESUMIT check that ensures when the user has updated
css_property_id.mojom they are also updating the enums.xml accordingly
using the expected python script.

The check is similar to the one in web_feature/PRESUMIT.py.

Also removed a defunct PRESUMIT in core/frame that was trying to check
the validity of max value. We no longer need user to manually enter max
value so this is no longer needed.

TEST = manually added a new entry in mojom file and not ran the script.
presubmit check detected this with the following warning. Then running
the script made the warning go away.

** Presubmit Warnings **
MappedCSSProperties enum has been updated and the UMA mapping needs to be regenerated. Please run update_use_counter_css.py in src/tools/metrics/histograms/ to update the mapping.
  third_party/blink/public/mojom/use_counter/css_property_id.mojom


Change-Id: I357bad58682c8ac50d67f4929da5e9bca7c08554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1755413
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723400}

third_party/blink/public/mojom/use_counter/PRESUBMIT.py

+# Copyright 2019 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.
+"""Blink frame presubmit script
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details about the presubmit API built into gcl.
+
+This script generates enums.xml based on the css_property_id.mojom and verifies
+that it is equivalent with the expected file in the patch otherwise warns the
+user.
+"""
+
+import sys
+
+
+def _RunUmaHistogramChecks(input_api, output_api):
+  original_sys_path = sys.path
+  try:
+    sys.path = sys.path + [
+        input_api.os_path.join(input_api.PresubmitLocalPath(), '..', '..', '..',
+                               '..', '..', 'tools', 'metrics', 'histograms')
+    ]
+    import update_histogram_enum  # pylint: disable=F0401
+    import update_use_counter_css  # pylint: disable=F0401
+
+  finally:
+    sys.path = original_sys_path
+
+  source_path = 'third_party/blink/public/mojom/use_counter/'\
+                'css_property_id.mojom'
+
+  if not source_path in [f.LocalPath() for f in input_api.AffectedFiles()]:
+    return []
+
+  # Note: This is similar to update_histogram_enum.CheckPresubmitErrors except
+  # that we use the logic in update_use_counter_css to produce the enum
+  # values.
+
+  histogram_enum_name = 'MappedCSSProperties'
+  update_script_name = 'update_use_counter_css.py'
+
+  def read_values(source_path, start_marker, end_marker, strip_k_prefix):
+    return update_use_counter_css.ReadCssProperties(source_path)
+
+  error = update_histogram_enum.CheckPresubmitErrors(
+      histogram_enum_name,
+      update_script_name,
+      source_path,
+      '',
+      '',
+      histogram_value_reader=read_values)
+
+  if error:
+    return [output_api.PresubmitPromptWarning(error, items=[source_path])]
+  return []
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return _RunUmaHistogramChecks(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return _RunUmaHistogramChecks(input_api, output_api)

third_party/blink/renderer/core/frame/PRESUBMIT.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.
-
-"""Blink frame presubmit script
-
-See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
-for more details about the presubmit API built into gcl.
-"""
-
-CSS_PROPERTY_ID_HEADER_PATH = (
-    'third_party/blink/public/mojom/use_counter/css_property_id.mojom')
-
-def _RunUseCounterChecks(input_api, output_api):
-    for f in input_api.AffectedFiles():
-        if f.LocalPath().endswith('use_counter.cc'):
-            use_counter_cpp_file = f
-            break
-    else:
-        return []
-
-    largest_found_bucket = 0
-    expected_max_bucket = 0
-
-    # Looking for a line like "case CSSPropertyID::kGrid: return 453;"
-    bucket_finder = input_api.re.compile(
-        r'case CSSPropertyID::k\w+:\s*return\s*(\d+);')
-
-    # Looking for a line like "const int32 kMaximumCSSSampleId = 452;"
-    expected_max_finder = input_api.re.compile(
-        r'const int32 kMaximumCSSSampleId = (\d+);')
-
-    for f in input_api.change.AffectedFiles():
-        if f.AbsoluteLocalPath().endswith(CSS_PROPERTY_ID_HEADER_PATH):
-            expected_max_match = expected_max_finder.search(
-                '\n'.join(f.NewContents()))
-            break
-    else:
-        return []
-
-    if expected_max_match:
-        expected_max_bucket = int(expected_max_match.group(1))
-
-    for bucket_match in bucket_finder.finditer(
-            '\n'.join(use_counter_cpp_file.NewContents())):
-
-        bucket = int(bucket_match.group(1))
-        largest_found_bucket = max(largest_found_bucket, bucket)
-
-    if largest_found_bucket != expected_max_bucket:
-        if input_api.is_committing:
-            message_type = output_api.PresubmitError
-        else:
-            message_type = output_api.PresubmitPromptWarning
-
-        return [message_type(
-            'Largest found CSSProperty bucket Id (%d) does not match '
-            'maximumCSSSampleId (%d)' % (
-                largest_found_bucket, expected_max_bucket),
-            items=[use_counter_cpp_file.LocalPath(),
-                   CSS_PROPERTY_ID_HEADER_PATH])]
-
-    return []
-
-
-def CheckChangeOnUpload(input_api, output_api):
-    results = []
-    results.extend(_RunUseCounterChecks(input_api, output_api))
-    return results
-
-
-def CheckChangeOnCommit(input_api, output_api):
-    results = []
-    results.extend(_RunUseCounterChecks(input_api, output_api))
-    return results

tools/metrics/histograms/update_histogram_enum.py

@@ -233,31 +233,46 @@ def _GetOldAndUpdatedXml(histogram_enum_name, source_enum_values,
   return (xml, new_xml)
 
 
-def CheckPresubmitErrors(histogram_enum_name, update_script_name,
-                         source_enum_path, start_marker,
-                         end_marker, strip_k_prefix = False):
-  """Reads a C++ enum from a .h file and checks for presubmit violations:
-  1. Failure to update histograms.xml to match
-  2. Introduction of duplicate values.
+def CheckPresubmitErrors(histogram_enum_name,
+                         update_script_name,
+                         source_enum_path,
+                         start_marker,
+                         end_marker,
+                         strip_k_prefix=False,
+                         histogram_value_reader=ReadHistogramValues):
+  """Extracts histogram enum values from a source file and checks for
+  violations.
+
+  Enum values are extracted from |source_enum_path| using
+  |histogram_value_reader| function. The following presubmit violations are then
+  checked:
+    1. Failure to update histograms.xml to match
+    2. Introduction of duplicate values
 
   Args:
       histogram_enum_name: The name of the XML <enum> attribute to update.
       update_script_name: The name of an update script to run to update the UMA
           mappings for the enum.
       source_enum_path: A unix-style path, relative to src/, giving
-          the C++ header file from which to read the enum.
+          the source file from which to read the enum.
       start_marker: A regular expression that matches the start of the C++ enum.
       end_marker: A regular expression that matches the end of the C++ enum.
       strip_k_prefix: Set to True if enum values are declared as kFoo and the
           'k' should be stripped.
+      histogram_value_reader: A reader function that takes four arguments
+          (source_path, start_marker, end_marker, strip_k_prefix), and returns a
+          list of strings of the extracted enum names. The default is
+          ReadHistogramValues(), which parses the values out of an enum defined
+          in a C++ source file.
+
 
   Returns:
       A string with presubmit failure description, or None (if no failures).
   """
   Log('Reading histogram enum definition from "{0}".'.format(source_enum_path))
   try:
-    source_enum_values = ReadHistogramValues(
-        source_enum_path, start_marker, end_marker, strip_k_prefix)
+    source_enum_values = histogram_value_reader(source_enum_path, start_marker,
+                                                end_marker, strip_k_prefix)
   except DuplicatedValue as duplicated_values:
     return ('%s enum has been updated and there exist '
             'duplicated values between (%s) and (%s)' %
@@ -265,7 +280,7 @@ def CheckPresubmitErrors(histogram_enum_name, update_script_name,
              duplicated_values.second_label))
 
   (xml, new_xml) = _GetOldAndUpdatedXml(histogram_enum_name, source_enum_values,
-                                        source_enum_path, None)
+                                        source_enum_path, update_script_name)
   if xml != new_xml:
     return ('%s enum has been updated and the UMA mapping needs to be '
             'regenerated. Please run %s in src/tools/metrics/histograms/ to '