Add pretty printing for rappor.xml

BUG=381380

Review URL: https://codereview.chromium.org/925753002

Cr-Commit-Position: refs/heads/master@{#316634}

9 files changed, 644 insertions, 159 deletions

diff --git a/tools/metrics/actions/extract_actions.py b/tools/metrics/actions/extract_actions.py
index 94dcf22..dde900a 100755
--- a/tools/metrics/actions/extract_actions.py
+++ b/tools/metrics/actions/extract_actions.py
@@ -819,14 +819,7 @@ def PrettyPrint(actions, actions_dict, comment_nodes=[]):
   return print_style.GetPrintStyle().PrettyPrintNode(doc)
 
 
-def main(argv):
-  presubmit = ('--presubmit' in argv)
-  actions_xml_path = os.path.join(path_utils.ScriptDir(), 'actions.xml')
-
-  # Save the original file content.
-  with open(actions_xml_path, 'rb') as f:
-    original_xml = f.read()
-
+def UpdateXml(original_xml):
   actions, actions_dict, comment_nodes = ParseActionFile(original_xml)
 
   AddComputedActions(actions)
@@ -848,30 +841,12 @@ def main(argv):
   AddExtensionActions(actions)
   AddHistoryPageActions(actions)
 
-  pretty = PrettyPrint(actions, actions_dict, comment_nodes)
-  if original_xml == pretty:
-    print 'actions.xml is correctly pretty-printed.'
-    sys.exit(0)
-  if presubmit:
-    logging.info('actions.xml is not formatted correctly; run '
-                 'extract_actions.py to fix.')
-    sys.exit(1)
-
-  # Prompt user to consent on the change.
-  if not diff_util.PromptUserToAcceptDiff(
-      original_xml, pretty, 'Is the new version acceptable?'):
-    logging.error('Aborting')
-    sys.exit(1)
-
-  print 'Creating backup file: actions.old.xml.'
-  shutil.move(actions_xml_path, 'actions.old.xml')
-
-  with open(actions_xml_path, 'wb') as f:
-    f.write(pretty)
-  print ('Updated %s. Don\'t forget to add it to your changelist' %
-         actions_xml_path)
-  return 0
+  return PrettyPrint(actions, actions_dict, comment_nodes)
 
 
+def main(argv):
+  presubmit_util.DoPresubmitMain(argv, 'actions.xml', 'actions.old.xml',
+                                 'extract_actions.py', UpdateXml)
+
 if '__main__' == __name__:
   sys.exit(main(sys.argv))
diff --git a/tools/metrics/common/models.py b/tools/metrics/common/models.py
new file mode 100644
index 0000000..3ba301c
--- /dev/null
+++ b/tools/metrics/common/models.py
@@ -0,0 +1,214 @@
+# Copyright 2015 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.
+
+"""Model types for describing description xml models."""
+
+from xml.dom import minidom
+
+import sys
+import os
+
+import pretty_print_xml
+
+
+def GetComments(node):
+  """Extracts comments in the current node.
+
+  Args:
+    node: The DOM node to extract comments from.
+  Returns:
+    A list of comment DOM nodes.
+  """
+  return [n for n in node.childNodes if n.nodeType == minidom.Node.COMMENT_NODE]
+
+
+def PutComments(node, comments):
+  """Append comments to the DOM node.
+
+  Args:
+    node: The DOM node to write comments to.
+    comments: A list of comment DOM nodes.
+  """
+  for n in comments:
+    node.appendChild(n)
+
+
+class NodeType(object):
+  """Base type for a type of XML node.
+
+  Args:
+    dont_indent: True iff this node should not have it's children indented
+        when pretty printing.
+    extra_newlines: None or a triple of integers describing the number of
+        newlines that should be printed (after_open, before_close, after_close)
+    single_line: True iff this node may be squashed into a single line.
+  """
+  def __init__(self, tag,
+               dont_indent=False,
+               extra_newlines=None,
+               single_line=False):
+    self.tag = tag
+    self.dont_indent = dont_indent
+    self.extra_newlines = extra_newlines
+    self.single_line = single_line
+
+  def Unmarshall(self, node):
+    return None
+
+  def Marshall(self, doc, obj):
+    return None
+
+  def GetAttributes(self):
+    return []
+
+  def GetNodeTypes(self):
+    return {self.tag: self}
+
+
+class TextNodeType(NodeType):
+  """A type for simple nodes that just have a tag and some text content.
+
+  Unmarshalls nodes to strings.
+
+  Args:
+    tag: The name of XML tag for this type of node.
+  """
+  def __init__(self, tag, **kwargs):
+    NodeType.__init__(self, tag, **kwargs)
+
+  def __str__(self):
+    return 'TextNodeType("%s")' % self.tag
+
+  def Unmarshall(self, node):
+    return node.firstChild.nodeValue.strip()
+
+  def Marshall(self, doc, obj):
+    node = doc.createElement(self.tag)
+    node.appendChild(doc.createTextNode(obj))
+    return node
+
+
+class ChildType(object):
+  """Metadata about a nodes children.
+
+  Args:
+    attr: The field name of the parents model object storing the child's model.
+    node_type: The NodeType of the child.
+    multiple: True if the child can be repeated.
+  """
+  def __init__(self, attr, node_type, multiple):
+    self.attr = attr
+    self.node_type = node_type
+    self.multiple = multiple
+
+
+class ObjectNodeType(NodeType):
+  """A complex node type that has attributes or other nodes as children.
+
+  Unmarshalls nodes to objects.
+
+  Args:
+    tag: The name of XML tag for this type of node.
+    int_attributes: A list of names of integer attributes.
+    float_attributes: A list of names of float attributes.
+    string_attributes: A list of names of string attributes.
+    children: A list of ChildTypes describing the objects children.
+  """
+  def __init__(self, tag,
+               int_attributes=[],
+               float_attributes=[],
+               string_attributes=[],
+               children=[],
+               **kwargs):
+    NodeType.__init__(self, tag, **kwargs)
+    self.int_attributes = int_attributes
+    self.float_attributes = float_attributes
+    self.string_attributes = string_attributes
+    self.children = children
+
+  def __str__(self):
+    return 'ObjectNodeType("%s")' % self.tag
+
+  def Unmarshall(self, node):
+    obj = {}
+
+    obj['comments'] = GetComments(node)
+
+    for attr in self.int_attributes:
+      obj[attr] = int(node.getAttribute(attr))
+
+    for attr in self.float_attributes:
+      obj[attr] = float(node.getAttribute(attr))
+
+    for attr in self.string_attributes:
+      obj[attr] = node.getAttribute(attr)
+
+    for child in self.children:
+      nodes = node.getElementsByTagName(child.node_type.tag)
+      if child.multiple:
+        obj[child.attr] = [child.node_type.Unmarshall(n) for n in nodes]
+      else:
+        obj[child.attr] = child.node_type.Unmarshall(nodes[0])
+    return obj
+
+  def Marshall(self, doc, obj):
+    node = doc.createElement(self.tag)
+    attributes = (self.int_attributes +
+                  self.float_attributes +
+                  self.string_attributes)
+    for attr in attributes:
+      node.setAttribute(attr, str(obj[attr]))
+
+    PutComments(node, obj['comments'])
+
+    for child in self.children:
+      if child.multiple:
+        for o in obj[child.attr]:
+          node.appendChild(child.node_type.Marshall(doc, o))
+      else:
+        node.appendChild(child.node_type.Marshall(doc, obj[child.attr]))
+    return node
+
+  def GetAttributes(self):
+    return self.int_attributes + self.float_attributes + self.string_attributes
+
+  def GetNodeTypes(self):
+    types = {self.tag: self}
+    for child in self.children:
+      types.update(child.node_type.GetNodeTypes())
+    return types
+
+
+class DocumentType(object):
+  """Model for the root of an XML description file.
+
+  Args:
+    root_type: A NodeType describing the root tag of the document.
+  """
+  def __init__(self, root_type):
+    self.root_type = root_type
+
+  def Parse(self, input_file):
+    tree = minidom.parseString(input_file)
+    comments = GetComments(tree)
+    return comments, self.root_type.Unmarshall(
+        tree.getElementsByTagName(self.root_type.tag)[0])
+
+  def GetPrintStyle(self):
+    types = self.root_type.GetNodeTypes()
+    return pretty_print_xml.XmlStyle(
+        {t: types[t].GetAttributes() for t in types},
+        {t: types[t].extra_newlines for t in types if types[t].extra_newlines},
+        [t for t in types if types[t].dont_indent],
+        [t for t in types if types[t].single_line])
+
+  def ToXML(self, comments, obj):
+    doc = minidom.Document()
+    for comment in comments:
+      doc.appendChild(comment)
+    doc.appendChild(self.root_type.Marshall(doc, obj))
+    return doc
+
+  def PrettyPrint(self, comments, obj):
+    return self.GetPrintStyle().PrettyPrintNode(self.ToXML(comments, obj))
diff --git a/tools/metrics/common/presubmit_util.py b/tools/metrics/common/presubmit_util.py
new file mode 100644
index 0000000..fc05d15
--- /dev/null
+++ b/tools/metrics/common/presubmit_util.py
@@ -0,0 +1,79 @@
+# Copyright 2015 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 os
+import sys
+import logging
+import shutil
+
+import diff_util
+
+sys.path.insert(1, os.path.join(sys.path[0], '..', '..', 'python'))
+from google import path_utils
+
+def DoPresubmitMain(argv, original_filename, backup_filename, script_name,
+                    prettyFn):
+  """Execute presubmit/pretty printing for the target file.
+
+  Args:
+    argv: command line arguments
+    original_filename: The filename to read from.
+    backup_filename: When pretty printing, move the old file contents here.
+    script_name: The name of the script to run for pretty printing.
+    prettyFn: A function which takes the original xml content and produces
+        pretty printed xml.
+
+  Returns:
+    An exit status.  Non-zero indicates errors.
+  """
+  logging.basicConfig(level=logging.INFO)
+  presubmit = ('--presubmit' in argv)
+
+  # If there is a description xml in the current working directory, use that.
+  # Otherwise, use the one residing in the same directory as this script.
+  xml_dir = os.getcwd()
+  if not os.path.isfile(os.path.join(xml_dir, original_filename)):
+    xml_dir = path_utils.ScriptDir()
+
+  xml_path = os.path.join(xml_dir, original_filename)
+
+  # Save the original file content.
+  logging.info('Loading %s...', os.path.relpath(xml_path))
+  with open(xml_path, 'rb') as f:
+    original_xml = f.read()
+
+  # Check there are no CR ('\r') characters in the file.
+  if '\r' in original_xml:
+    logging.error('DOS-style line endings (CR characters) detected - these are '
+                  'not allowed. Please run dos2unix %s', original_filename)
+    return 1
+
+  try:
+    pretty = prettyFn(original_xml)
+  except Error:
+    logging.error('Aborting parsing due to fatal errors.')
+    return 1
+
+  if original_xml == pretty:
+    logging.info('%s is correctly pretty-printed.', original_filename)
+    return 0
+  if presubmit:
+    logging.error('%s is not formatted correctly; run %s to fix.',
+                  original_filename, script_name)
+    return 1
+
+  # Prompt user to consent on the change.
+  if not diff_util.PromptUserToAcceptDiff(
+      original_xml, pretty, 'Is the new version acceptable?'):
+    logging.error('Diff not accepted. Aborting.')
+    return 1
+
+  logging.info('Creating backup file: %s', backup_filename)
+  shutil.move(xml_path, os.path.join(xml_dir, backup_filename))
+
+  with open(xml_path, 'wb') as f:
+    f.write(pretty)
+  logging.info('Updated %s. Don\'t forget to add it to your changelist',
+               xml_path)
+  return 0
diff --git a/tools/metrics/histograms/pretty_print.py b/tools/metrics/histograms/pretty_print.py
index d9e0327..41cc2b6 100755
--- a/tools/metrics/histograms/pretty_print.py
+++ b/tools/metrics/histograms/pretty_print.py
@@ -28,6 +28,7 @@ from google import path_utils
 # Import the metrics/common module.
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
 import diff_util
+import presubmit_util
 
 # Tags whose children we want to alphabetize. The key is the parent tag name,
 # and the value is a pair of the tag name of the children we want to sort,
@@ -130,62 +131,10 @@ def PrettyPrint(raw_xml):
   tree = TransformByAlphabetizing(tree)
   return print_style.GetPrintStyle().PrettyPrintNode(tree)
 
-
 def main():
-  logging.basicConfig(level=logging.INFO)
-
-  presubmit = ('--presubmit' in sys.argv)
-
-  histograms_filename = 'histograms.xml'
-  histograms_backup_filename = 'histograms.before.pretty-print.xml'
-
-  # If there is a histograms.xml in the current working directory, use that.
-  # Otherwise, use the one residing in the same directory as this script.
-  histograms_dir = os.getcwd()
-  if not os.path.isfile(os.path.join(histograms_dir, histograms_filename)):
-    histograms_dir = path_utils.ScriptDir()
-
-  histograms_pathname = os.path.join(histograms_dir, histograms_filename)
-  histograms_backup_pathname = os.path.join(histograms_dir,
-                                            histograms_backup_filename)
-
-  logging.info('Loading %s...' % os.path.relpath(histograms_pathname))
-  with open(histograms_pathname, 'rb') as f:
-    xml = f.read()
-
-  # Check there are no CR ('\r') characters in the file.
-  if '\r' in xml:
-    logging.info('DOS-style line endings (CR characters) detected - these are '
-                 'not allowed. Please run dos2unix %s' % histograms_filename)
-    sys.exit(1)
-
-  logging.info('Pretty-printing...')
-  try:
-    pretty = PrettyPrint(xml)
-  except Error:
-    logging.error('Aborting parsing due to fatal errors.')
-    sys.exit(1)
-
-  if xml == pretty:
-    logging.info('%s is correctly pretty-printed.' % histograms_filename)
-    sys.exit(0)
-  if presubmit:
-    logging.info('%s is not formatted correctly; run pretty_print.py to fix.' %
-                 histograms_filename)
-    sys.exit(1)
-  if not diff_util.PromptUserToAcceptDiff(
-      xml, pretty,
-      'Is the prettified version acceptable?'):
-    logging.error('Aborting')
-    return
-
-  logging.info('Creating backup file %s' % histograms_backup_filename)
-  shutil.move(histograms_pathname, histograms_backup_pathname)
-
-  logging.info('Writing new %s file' % histograms_filename)
-  with open(histograms_pathname, 'wb') as f:
-    f.write(pretty)
-
+  presubmit_util.DoPresubmitMain(sys.argv, 'histograms.xml',
+                                 'histograms.before.pretty-print.xml',
+                                 'pretty_print.py', PrettyPrint)
 
 if __name__ == '__main__':
   main()
diff --git a/tools/metrics/rappor/OWNERS b/tools/metrics/rappor/OWNERS
new file mode 100644
index 0000000..52bf7a2
--- /dev/null
+++ b/tools/metrics/rappor/OWNERS
@@ -0,0 +1,2 @@
+asvitkine@chromium.org
+holte@chromium.org
diff --git a/tools/metrics/rappor/PRESUBMIT.py b/tools/metrics/rappor/PRESUBMIT.py
new file mode 100644
index 0000000..c70f50f
--- /dev/null
+++ b/tools/metrics/rappor/PRESUBMIT.py
@@ -0,0 +1,33 @@
+# Copyright 2015 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.
+
+"""Presubmit script for rappor.xml.
+
+See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
+for more details on the presubmit API built into gcl.
+"""
+
+
+def CheckChange(input_api, output_api):
+  """Checks that rappor.xml is pretty-printed and well-formatted."""
+  for f in input_api.AffectedTextFiles():
+    p = f.AbsoluteLocalPath()
+    if (input_api.basename(p) == 'rappor.xml'
+        and input_api.os_path.dirname(p) == input_api.PresubmitLocalPath()):
+      cwd = input_api.os_path.dirname(p)
+      exit_code = input_api.subprocess.call(
+          ['python', 'pretty_print.py', '--presubmit'], cwd=cwd)
+      if exit_code != 0:
+        return [output_api.PresubmitError(
+            'rappor.xml is not formatted correctly; run pretty_print.py '
+            'to fix')]
+  return []
+
+
+def CheckChangeOnUpload(input_api, output_api):
+  return CheckChange(input_api, output_api)
+
+
+def CheckChangeOnCommit(input_api, output_api):
+  return CheckChange(input_api, output_api)
diff --git a/tools/metrics/rappor/pretty_print.py b/tools/metrics/rappor/pretty_print.py
new file mode 100755
index 0000000..7430479
--- /dev/null
+++ b/tools/metrics/rappor/pretty_print.py
@@ -0,0 +1,167 @@
+#!/usr/bin/env python
+# Copyright 2015 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 logging
+import sys
+import os
+
+# Import the metrics/common module for pretty print xml.
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
+import models
+import presubmit_util
+
+
+# Model definitions for rappor.xml content
+_SUMMARY_TYPE = models.TextNodeType('summary')
+
+_PARAMETERS_TYPE = models.ObjectNodeType('parameters',
+    int_attributes=[
+      'num-cohorts',
+      'bytes',
+      'hash-functions',
+    ],
+    float_attributes=[
+      'fake-prob',
+      'fake-one-prob',
+      'one-coin-prob',
+      'zero-coin-prob',
+    ],
+    string_attributes=[
+      'reporting-level'
+    ])
+
+_RAPPOR_PARAMETERS_TYPE = models.ObjectNodeType('rappor-parameters',
+    extra_newlines=(1, 1, 1),
+    string_attributes=['name'],
+    children=[
+      models.ChildType('summary', _SUMMARY_TYPE, False),
+      models.ChildType('parameters', _PARAMETERS_TYPE, False),
+    ])
+
+_RAPPOR_PARAMETERS_TYPES_TYPE = models.ObjectNodeType('rappor-parameter-types',
+    extra_newlines=(1, 1, 1),
+    dont_indent=True,
+    children=[
+      models.ChildType('types', _RAPPOR_PARAMETERS_TYPE, True),
+    ])
+
+_OWNER_TYPE = models.TextNodeType('owner', single_line=True)
+
+_RAPPOR_METRIC_TYPE = models.ObjectNodeType('rappor-metric',
+    extra_newlines=(1, 1, 1),
+    string_attributes=['name', 'type'],
+    children=[
+      models.ChildType('owners', _OWNER_TYPE, True),
+      models.ChildType('summary', _SUMMARY_TYPE, False),
+    ])
+
+_RAPPOR_METRICS_TYPE = models.ObjectNodeType('rappor-metrics',
+    extra_newlines=(1, 1, 1),
+    dont_indent=True,
+    children=[
+      models.ChildType('metrics', _RAPPOR_METRIC_TYPE, True),
+    ])
+
+_RAPPOR_CONFIGURATION_TYPE = models.ObjectNodeType('rappor-configuration',
+    extra_newlines=(1, 1, 1),
+    dont_indent=True,
+    children=[
+      models.ChildType('parameterTypes', _RAPPOR_PARAMETERS_TYPES_TYPE, False),
+      models.ChildType('metrics', _RAPPOR_METRICS_TYPE, False),
+    ])
+
+RAPPOR_XML_TYPE = models.DocumentType(_RAPPOR_CONFIGURATION_TYPE)
+
+
+def GetTypeNames(config):
+  return set(p['name'] for p in config['parameterTypes']['types'])
+
+
+def HasMissingOwners(metrics):
+  """Check that all of the metrics have owners.
+
+  Args:
+    metrics: A list of rappor metric description objects.
+
+  Returns:
+    True iff some metrics are missing owners.
+  """
+  missing_owners = [m for m in metrics if not m['owners']]
+  for metric in missing_owners:
+    logging.error('Rappor metric "%s" is missing an owner.', metric['name'])
+    print metric
+  return bool(missing_owners)
+
+
+def HasInvalidTypes(type_names, metrics):
+  """Check that all of the metrics have valid types.
+
+  Args:
+    type_names: The set of valid type names.
+    metrics: A list of rappor metric description objects.
+
+  Returns:
+    True iff some metrics have invalid types.
+  """
+  invalid_types = [m for m in metrics if m['type'] not in type_names]
+  for metric in invalid_types:
+    logging.error('Rappor metric "%s" has invalid type "%s"',
+        metric['name'], metric['type'])
+  return bool(invalid_types)
+
+
+def HasErrors(config):
+  """Check that rappor.xml passes some basic validation checks.
+
+  Args:
+    config: The parsed rappor.xml contents.
+
+  Returns:
+    True iff there are validation errors.
+  """
+  metrics = config['metrics']['metrics']
+  type_names = GetTypeNames(config)
+  return (HasMissingOwners(metrics) or
+          HasInvalidTypes(type_names, metrics))
+
+
+def Cleanup(config):
+  """Preform cleanup on description contents, such as sorting metrics.
+
+  Args:
+    config: The parsed rappor.xml contents.
+  """
+  types = config['parameterTypes']['types']
+  types.sort(key=lambda x: x['name'])
+  metrics = config['metrics']['metrics']
+  metrics.sort(key=lambda x: x['name'])
+
+
+def UpdateXML(original_xml):
+  """Parse the original xml and return a pretty printed version.
+
+  Args:
+    original_xml: A string containing the original xml file contents.
+
+  Returns:
+    A Pretty printed xml string.
+  """
+  comments, config = RAPPOR_XML_TYPE.Parse(original_xml)
+
+  if HasErrors(config):
+    return None
+
+  Cleanup(config)
+
+  return RAPPOR_XML_TYPE.PrettyPrint(comments, config)
+
+
+def main(argv):
+  presubmit_util.DoPresubmitMain(argv, 'rappor.xml', 'rappor.old.xml',
+                                 'pretty_print.py', UpdateXML)
+
+
+if '__main__' == __name__:
+  sys.exit(main(sys.argv))
diff --git a/tools/metrics/rappor/pretty_print_test.py b/tools/metrics/rappor/pretty_print_test.py
new file mode 100755
index 0000000..428158a
--- /dev/null
+++ b/tools/metrics/rappor/pretty_print_test.py
@@ -0,0 +1,82 @@
+#!/usr/bin/env python
+# Copyright 2015 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 unittest
+
+import pretty_print
+
+
+PRETTY_XML = """
+<!-- Comment1 -->
+
+<rappor-configuration>
+<!-- Comment2 -->
+
+<rappor-parameter-types>
+<!-- Comment3 -->
+
+<rappor-parameters name="TEST_RAPPOR_TYPE">
+  <summary>
+    Fake type for tests.
+  </summary>
+  <parameters num-cohorts="128" bytes="1" hash-functions="2" fake-prob="0.5"
+      fake-one-prob="0.5" one-coin-prob="0.75" zero-coin-prob="0.25"
+      reporting-level="COARSE"/>
+</rappor-parameters>
+
+</rappor-parameter-types>
+
+<rappor-metrics>
+<!-- Comment4 -->
+
+<rappor-metric name="Test.Rappor.Metric" type="TEST_RAPPOR_TYPE">
+  <owner>user1@chromium.org</owner>
+  <owner>user2@chromium.org</owner>
+  <summary>
+    A fake metric summary.
+  </summary>
+</rappor-metric>
+
+</rappor-metrics>
+
+</rappor-configuration>
+""".strip()
+
+BASIC_METRIC = {
+  'comments': [],
+  'name': 'Test.Rappor.Metric',
+  'type': 'TEST_RAPPOR_TYPE',
+  'owners': ['user1@chromium.org', 'user2@chromium.org'],
+  'summary': 'A fake metric summary.',
+}
+
+
+class ActionXmlTest(unittest.TestCase):
+
+  def testIsPretty(self):
+    result = pretty_print.UpdateXML(PRETTY_XML)
+    self.assertEqual(PRETTY_XML, result)
+
+  def testParsing(self):
+    comments, config = pretty_print.RAPPOR_XML_TYPE.Parse(PRETTY_XML)
+    self.assertEqual(BASIC_METRIC, config['metrics']['metrics'][0])
+    self.assertEqual(set(['TEST_RAPPOR_TYPE']),
+                     pretty_print.GetTypeNames(config))
+
+  def testMissingOwners(self):
+    self.assertFalse(pretty_print.HasMissingOwners([BASIC_METRIC]))
+    no_owners = BASIC_METRIC.copy()
+    no_owners['owners'] = []
+    self.assertTrue(pretty_print.HasMissingOwners([no_owners]))
+
+  def testInvalidTypes(self):
+    self.assertFalse(pretty_print.HasInvalidTypes(
+        set(['TEST_RAPPOR_TYPE']), [BASIC_METRIC]))
+    self.assertTrue(pretty_print.HasInvalidTypes(
+        set(['OTHER_TYPE']), [BASIC_METRIC]))
+
+
+if __name__ == '__main__':
+  unittest.main()
diff --git a/tools/metrics/rappor/rappor.xml b/tools/metrics/rappor/rappor.xml
index 2f7cac2..8e52f4a 100644
--- a/tools/metrics/rappor/rappor.xml
+++ b/tools/metrics/rappor/rappor.xml
@@ -1,63 +1,57 @@
 <!--
-Copyright 2014 The Chromium Authors. All rights reserved.
+Copyright 2015 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.
 -->
 
+<rappor-configuration>
 <!--
 This file is used to generate a comprehensive list of Chrome rappor metrics
 along with a detailed description for each histogram. See the design doc at
 http://www.chromium.org/developers/design-documents/rappor
 for a description of rappor metrics.
-
-TODO(holte): Add validation and pretty printing scripts.
 -->
 
-<rappor-configuration>
-
+<rappor-parameter-types>
 <!--
 Parameters that rappor metrics can be collected with.  This list should be
 kept in sync with parameter type definitions in
 components/rappor/rappor_service.cc.
 -->
 
-<rappor-parameter-types>
-
-<rappor-parameters name="ETLD_PLUS_ONE">
+<rappor-parameters name="COARSE_RAPPOR_TYPE">
   <summary>
-    Parameters suitable for collecting the domain and registry of a URL from
-    UMA opt-in users.
+    Stricter parameters for metrics collected from a broader population.
   </summary>
-  <parameters num-cohorts="128"
-              bytes="16"
-              hash-functions="2"
-              fake-prob=".5"
-              fake-one-prob=".5"
-              one-coin-prob=".75"
-              zero-coin-prob=".25"
-              reporting-level="FINE">
-  </parameters>
+  <parameters num-cohorts="128" bytes="1" hash-functions="2" fake-prob="0.5"
+      fake-one-prob="0.5" one-coin-prob="0.75" zero-coin-prob="0.25"
+      reporting-level="COARSE"/>
 </rappor-parameters>
 
-<rappor-parameters name="COARSE_RAPPOR_TYPE">
+<rappor-parameters name="ETLD_PLUS_ONE">
   <summary>
-    Stricter parameters for metrics collected from a broader population.
+    Parameters suitable for collecting the domain and registry of a URL from
+    UMA opt-in users.
   </summary>
-  <parameters num-cohorts="128"
-              bytes="1"
-              hash-functions="2"
-              fake-prob=".5"
-              fake-one-prob=".5"
-              one-coin-prob=".75"
-              zero-coin-prob=".25"
-              reporting-level="COARSE">
+  <parameters num-cohorts="128" bytes="16" hash-functions="2" fake-prob="0.5"
+      fake-one-prob="0.5" one-coin-prob="0.75" zero-coin-prob="0.25"
+      reporting-level="FINE"/>
 </rappor-parameters>
 
 </rappor-parameter-types>
 
+<rappor-metrics>
 <!-- Rappor metric definitions -->
 
-<rappor-metrics>
+<rappor-metric name="ContentSettings.MixedScript.DisplayedShield"
+    type="ETLD_PLUS_ONE">
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    The eTLD+1 of a URL that displayed a mixed script shield. Note: this does
+    *not* include pages with mixed scripts where the user has already clicked
+    through the shield.
+  </summary>
+</rappor-metric>
 
 <rappor-metric name="Extensions.PossibleAdInjection2" type="ETLD_PLUS_ONE">
   <owner>rdevlin.cronin@chromium.org</owner>
@@ -66,13 +60,34 @@ components/rappor/rappor_service.cc.
   </summary>
 </rappor-metric>
 
-<rappor-metric name="ContentSettings.MixedScript.DisplayedShield"
-    type="ETLD_PLUS_ONE">
-  <owner>lgarron@chromium.org</owner>
+<rappor-metric name="Plugins.FlashOriginUrl" type="ETLD_PLUS_ONE">
+  <owner>wfh@chromium.org</owner>
   <summary>
-    The eTLD+1 of a URL that displayed a mixed script shield. Note: this does
-    *not* include pages with mixed scripts where the user has already clicked
-    through the shield.
+    The domain and registry of the top level URL of a page which attempts to
+    launch a Flash NPAPI or PPAPI plugin, if the client has Flash installed
+    and enabled. Recorded when the plugin frame appears for each Flash object
+    found on the page, even if the plugin is click-to-play.
+  </summary>
+</rappor-metric>
+
+<rappor-metric name="Plugins.FlashUrl" type="ETLD_PLUS_ONE">
+  <owner>wfh@chromium.org</owner>
+  <summary>
+    The domain and registry of the URL from where Flash SWF or SPL content is
+    being loaded from, while attempting to launch a Flash (NPAPI or PPAPI)
+    plugin that is installed and enabled.  Recorded when the plugin frame
+    appears for each Flash object found in the page, even if the plugin is
+    click-to-play.
+  </summary>
+</rappor-metric>
+
+<rappor-metric name="Plugins.SilverlightOriginUrl" type="ETLD_PLUS_ONE">
+  <owner>wfh@chromium.org</owner>
+  <summary>
+    The domain and registry of the top level URL of a page which attempts to
+    launch a Silverlight NPAPI plugin, if the client has Silverlight installed
+    and enabled. Recorded when the plugin frame appears for each Silverlight
+    object found on the page, even if the plugin is click-to-play.
   </summary>
 </rappor-metric>
 
@@ -109,34 +124,11 @@ components/rappor/rappor_service.cc.
   </summary>
 </rappor-metric>
 
-<rappor-metric name="Plugins.SilverlightOriginUrl" type="ETLD_PLUS_ONE">
-  <owner>wfh@chromium.org</owner>
-  <summary>
-    The domain and registry of the top level URL of a page which attempts to
-    launch a Silverlight NPAPI plugin, if the client has Silverlight installed
-    and enabled. Recorded when the plugin frame appears for each Silverlight
-    object found on the page, even if the plugin is click-to-play.
-  </summary>
-</rappor-metric>
-
-<rappor-metric name="Plugins.FlashUrl" type="ETLD_PLUS_ONE">
-  <owner>wfh@chromium.org</owner>
-  <summary>
-    The domain and registry of the URL from where Flash SWF or SPL content is
-    being loaded from, while attempting to launch a Flash (NPAPI or PPAPI)
-    plugin that is installed and enabled.  Recorded when the plugin frame
-    appears for each Flash object found in the page, even if the plugin is
-    click-to-play.
-  </summary>
-</rappor-metric>
-
-<rappor-metric name="Plugins.FlashOriginUrl" type="ETLD_PLUS_ONE">
-  <owner>wfh@chromium.org</owner>
+<rappor-metric name="interstitial.harmful.domain" type="COARSE_RAPPOR_TYPE">
+  <owner>nparker@chromium.org</owner>
   <summary>
-    The domain and registry of the top level URL of a page which attempts to
-    launch a Flash NPAPI or PPAPI plugin, if the client has Flash installed
-    and enabled. Recorded when the plugin frame appears for each Flash object
-    found on the page, even if the plugin is click-to-play.
+    The domain+registry of a URL that triggered a safe-browsing UWS
+    interstitial.
   </summary>
 </rappor-metric>
 
@@ -156,19 +148,11 @@ components/rappor/rappor_service.cc.
   </summary>
 </rappor-metric>
 
-<rappor-metric name="interstitial.harmful.domain" type="COARSE_RAPPOR_TYPE">
-  <owner>nparker@chromium.org</owner>
-  <summary>
-    The domain+registry of a URL that triggered a safe-browsing UWS
-    interstitial.
-  </summary>
-</rappor-metric>
-
 <rappor-metric name="interstitial.ssl.domain" type="COARSE_RAPPOR_TYPE">
   <owner>nparker@chromium.org</owner>
   <summary>
-    The domain+registry of a URL that triggered an SSL interstitial.
-    Domains for bad-clock warnings are not reported.
+    The domain+registry of a URL that triggered an SSL interstitial.     Domains
+    for bad-clock warnings are not reported.
   </summary>
 </rappor-metric>