[go: up one dir, main page]

Add UMA metrics to track policy atomic groups with conflicts

(cherry picked from commit 92c4a3ed152bf6008ff4cdba7cb06971e1bb7cb0)

Bug: 964013
Change-Id: I284b46b1eda2b27c8c6958afa340af4818a6f6fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628041
Commit-Queue: Yann Dago <ydago@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#666828}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1652542
Reviewed-by: Yann Dago <ydago@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#214}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/components/policy/core/common/policy_statistics_collector.cc b/components/policy/core/common/policy_statistics_collector.cc
index 4c7ce6f..0176205 100644
--- a/components/policy/core/common/policy_statistics_collector.cc
+++ b/components/policy/core/common/policy_statistics_collector.cc
@@ -15,6 +15,7 @@
 #include "base/task_runner.h"
 #include "components/policy/core/common/policy_pref_names.h"
 #include "components/policy/core/common/policy_service.h"
+#include "components/policy/policy_constants.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 
@@ -62,6 +63,11 @@
   base::UmaHistogramSparse("Enterprise.Policies", id);
 }
 
+void PolicyStatisticsCollector::RecordPolicyGroupWithConflicts(int id) {
+  base::UmaHistogramSparse("Enterprise.Policies.AtomicGroupSourceConflicts",
+                           id);
+}
+
 void PolicyStatisticsCollector::RecordPolicyIgnoredByAtomicGroup(int id) {
   base::UmaHistogramSparse("Enterprise.Policies.IgnoredByPolicyGroup", id);
 }
@@ -80,13 +86,27 @@
       else
         NOTREACHED();
     }
-    if (policies.IsPolicyIgnoredByAtomicGroup(it.key())) {
-      const PolicyDetails* details = get_details_.Run(it.key());
-      if (details)
-        RecordPolicyIgnoredByAtomicGroup(details->id);
-      else
-        NOTREACHED();
+  }
+
+  for (size_t i = 0; i < kPolicyAtomicGroupMappingsLength; ++i) {
+    const AtomicGroup& group = kPolicyAtomicGroupMappings[i];
+    bool group_has_conflicts = false;
+    // Find the policy with the highest priority that is both in |policies|
+    // and |group.policies|, an array ending with a nullptr.
+    for (const char* const* policy_name = group.policies; *policy_name;
+         ++policy_name) {
+      if (policies.IsPolicyIgnoredByAtomicGroup(*policy_name)) {
+        group_has_conflicts = true;
+        const PolicyDetails* details = get_details_.Run(*policy_name);
+        if (details)
+          RecordPolicyIgnoredByAtomicGroup(details->id);
+        else
+          NOTREACHED();
+      }
     }
+
+    if (group_has_conflicts)
+      RecordPolicyGroupWithConflicts(group.id);
   }
 
   // Take care of next update.
diff --git a/components/policy/core/common/policy_statistics_collector.h b/components/policy/core/common/policy_statistics_collector.h
index 2328185..1590ebe 100644
--- a/components/policy/core/common/policy_statistics_collector.h
+++ b/components/policy/core/common/policy_statistics_collector.h
@@ -47,8 +47,7 @@
  protected:
   // protected virtual for mocking.
   virtual void RecordPolicyUse(int id);
-
-  // protected virtual for mocking.
+  virtual void RecordPolicyGroupWithConflicts(int id);
   virtual void RecordPolicyIgnoredByAtomicGroup(int id);
 
  private:
diff --git a/components/policy/core/common/policy_statistics_collector_unittest.cc b/components/policy/core/common/policy_statistics_collector_unittest.cc
index 9ab98f3..de9924a 100644
--- a/components/policy/core/common/policy_statistics_collector_unittest.cc
+++ b/components/policy/core/common/policy_statistics_collector_unittest.cc
@@ -18,6 +18,7 @@
 #include "components/policy/core/common/policy_pref_names.h"
 #include "components/policy/core/common/policy_test_utils.h"
 #include "components/policy/core/common/policy_types.h"
+#include "components/policy/policy_constants.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -32,23 +33,27 @@
 // Arbitrary policy names used for testing.
 const char kTestPolicy1[] = "Test Policy 1";
 const char kTestPolicy2[] = "Test Policy 2";
+const char* kTestPolicy3 = key::kExtensionInstallBlacklist;
 
 const int kTestPolicy1Id = 42;
 const int kTestPolicy2Id = 123;
+const int kTestPolicy3Id = 32;
 
 const char kTestChromeSchema[] =
     "{"
     "  \"type\": \"object\","
     "  \"properties\": {"
     "    \"Test Policy 1\": { \"type\": \"string\" },"
-    "    \"Test Policy 2\": { \"type\": \"string\" }"
+    "    \"Test Policy 2\": { \"type\": \"string\" },"
+    "    \"ExtensionInstallBlacklist\": { \"type\": \"string\" },"
     "  }"
     "}";
 
 const PolicyDetails kTestPolicyDetails[] = {
-  // is_deprecated  is_device_policy              id  max_external_data_size
-  {          false,            false, kTestPolicy1Id,                      0 },
-  {          false,            false, kTestPolicy2Id,                      0 },
+    // is_deprecated  is_device_policy              id  max_external_data_size
+    {false, false, kTestPolicy1Id, 0},
+    {false, false, kTestPolicy2Id, 0},
+    {false, false, kTestPolicy3Id, 0},
 };
 
 class TestPolicyStatisticsCollector : public PolicyStatisticsCollector {
@@ -67,6 +72,7 @@
 
   MOCK_METHOD1(RecordPolicyUse, void(int));
   MOCK_METHOD1(RecordPolicyIgnoredByAtomicGroup, void(int));
+  MOCK_METHOD1(RecordPolicyGroupWithConflicts, void(int));
 };
 
 }  // namespace
@@ -86,6 +92,7 @@
 
     policy_details_.SetDetails(kTestPolicy1, &kTestPolicyDetails[0]);
     policy_details_.SetDetails(kTestPolicy2, &kTestPolicyDetails[1]);
+    policy_details_.SetDetails(kTestPolicy3, &kTestPolicyDetails[2]);
 
     prefs_.registry()->RegisterInt64Pref(
         policy_prefs::kLastPolicyStatisticsUpdate, 0);
@@ -194,13 +201,25 @@
 }
 
 TEST_F(PolicyStatisticsCollectorTest, PolicyIgnoredByAtomicGroup) {
-  SetPolicyIgnoredByAtomicGroup(kTestPolicy1);
+  SetPolicyIgnoredByAtomicGroup(kTestPolicy3);
+  const AtomicGroup* extensions = nullptr;
+
+  for (size_t i = 0; i < kPolicyAtomicGroupMappingsLength; ++i) {
+    if (kPolicyAtomicGroupMappings[i].policy_group == group::kExtensions) {
+      extensions = &kPolicyAtomicGroupMappings[i];
+      break;
+    }
+  }
+
+  DCHECK(extensions);
 
   prefs_.SetInt64(policy_prefs::kLastPolicyStatisticsUpdate,
                   (base::Time::Now() - update_delay_).ToInternalValue());
 
   EXPECT_CALL(*policy_statistics_collector_,
-              RecordPolicyIgnoredByAtomicGroup(kTestPolicy1Id));
+              RecordPolicyIgnoredByAtomicGroup(kTestPolicy3Id));
+  EXPECT_CALL(*policy_statistics_collector_,
+              RecordPolicyGroupWithConflicts(extensions->id));
 
   policy_statistics_collector_->Initialize();
   EXPECT_EQ(1u, task_runner_->NumPendingTasks());
diff --git a/components/policy/resources/PRESUBMIT.py b/components/policy/resources/PRESUBMIT.py
index ed94212..a0fd0d0 100644
--- a/components/policy/resources/PRESUBMIT.py
+++ b/components/policy/resources/PRESUBMIT.py
@@ -109,6 +109,48 @@
   return results
 
 
+def _CheckPolicyAtomicGroupsHistograms(input_api, output_api, atomic_groups):
+  root = input_api.change.RepositoryRoot()
+  histograms = input_api.os_path.join(
+      root, 'tools', 'metrics', 'histograms', 'enums.xml')
+  with open(histograms) as f:
+    tree = xml.dom.minidom.parseString(f.read())
+  enums = (tree.getElementsByTagName('histogram-configuration')[0]
+               .getElementsByTagName('enums')[0]
+               .getElementsByTagName('enum'))
+  atomic_group_enum = [e for e in enums
+                 if e.getAttribute('name') == 'PolicyAtomicGroups'][0]
+  atomic_group_enum_ids = frozenset(int(e.getAttribute('value'))
+                              for e in atomic_group_enum
+                                .getElementsByTagName('int'))
+  atomic_group_id_to_name = {policy['id']: policy['name']
+                                    for policy in atomic_groups}
+  atomic_group_ids = frozenset(atomic_group_id_to_name.keys())
+
+  missing_ids = atomic_group_ids - atomic_group_enum_ids
+  extra_ids = atomic_group_enum_ids - atomic_group_ids
+
+  error_missing = ('Policy atomic group \'%s\' (id %d) was added to '
+                   'policy_templates.json but not to '
+                   'src/tools/metrics/histograms/enums.xml. Please update '
+                   'both files. To regenerate the policy part of enums.xml, '
+                   'run:\n'
+                   'python tools/metrics/histograms/update_policies.py')
+  error_extra = ('Policy atomic group id %d was found in '
+                 'src/tools/metrics/histograms/enums.xml, but no policy with '
+                 'this id exists in policy_templates.json. To regenerate the '
+                 'policy part of enums.xml, run:\n'
+                 'python tools/metrics/histograms/update_policies.py')
+  results = []
+  for atomic_group_id in missing_ids:
+    results.append(output_api.PresubmitError(error_missing %
+                              (atomic_group_id_to_name[atomic_group_id],
+                              atomic_group_id)))
+  for atomic_group_id in extra_ids:
+    results.append(output_api.PresubmitError(error_extra % atomic_group_id))
+  return results
+
+
 def _CommonChecks(input_api, output_api):
   results = []
   results.extend(_CheckPolicyTemplatesSyntax(input_api, output_api))
diff --git a/components/policy/resources/policy_templates.json b/components/policy/resources/policy_templates.json
index df19711..afe450b 100644
--- a/components/policy/resources/policy_templates.json
+++ b/components/policy/resources/policy_templates.json
@@ -16393,6 +16393,7 @@
   },
   'policy_atomic_group_definitions': [
     {
+      'id': 1,
       'name': 'Homepage',
       'policies': [
         'HomepageLocation',
@@ -16402,6 +16403,7 @@
       ],
     },
     {
+      'id': 2,
       'name': 'RemoteAccess',
       'policies': [
         'RemoteAccessClientFirewallTraversal',
@@ -16427,6 +16429,7 @@
       ],
     },
     {
+      'id': 3,
       'name': 'PasswordManager',
       'policies': [
         'PasswordManagerEnabled',
@@ -16434,6 +16437,7 @@
       ],
     },
     {
+      'id': 4,
       'name': 'Proxy',
       'policies': [
         'ProxyMode',
@@ -16445,6 +16449,7 @@
       ],
     },
     {
+      'id': 5,
       'name': 'Extensions',
       'policies': [
         'ExtensionInstallBlacklist',
@@ -16457,6 +16462,7 @@
       ],
     },
     {
+      'id': 6,
       'name': 'RestoreOnStartupGroup',
       'policies': [
         'RestoreOnStartup',
@@ -16464,6 +16470,7 @@
       ],
     },
     {
+      'id': 7,
       'name': 'DefaultSearchProvider',
       'policies': [
         'DefaultSearchProviderEnabled',
@@ -16485,6 +16492,7 @@
       ],
     },
     {
+      'id': 8,
       'name': 'ImageSettings',
       'policies': [
         'DefaultImagesSetting',
@@ -16493,6 +16501,7 @@
       ],
     },
     {
+      'id': 9,
       'name': 'CookiesSettings',
       'policies': [
         'DefaultCookiesSetting',
@@ -16502,6 +16511,7 @@
       ],
     },
     {
+      'id': 10,
       'name': 'JavascriptSettings',
       'policies': [
         'DefaultJavaScriptSetting',
@@ -16510,6 +16520,7 @@
       ],
     },
     {
+      'id': 11,
       'name': 'PluginsSettings',
       'policies': [
         'DefaultPluginsSetting',
@@ -16518,6 +16529,7 @@
       ],
     },
     {
+      'id': 12,
       'name': 'PopupsSettings',
       'policies': [
         'DefaultPopupsSetting',
@@ -16526,6 +16538,7 @@
       ],
     },
     {
+      'id': 13,
       'name': 'KeygenSettings',
       'policies': [
         'DefaultKeygenSetting',
@@ -16534,6 +16547,7 @@
       ],
     },
     {
+      'id': 14,
       'name': 'NotificationsSettings',
       'policies': [
         'DefaultNotificationsSetting',
@@ -16542,6 +16556,7 @@
       ],
     },
     {
+      'id': 15,
       'name': 'WebUsbSettings',
       'policies': [
         'DefaultWebUsbGuardSetting',
@@ -16551,6 +16566,7 @@
       ],
     },
     {
+      'id': 16,
       'name': 'NativeMessaging',
       'policies': [
         'NativeMessagingBlacklist',
@@ -16559,6 +16575,7 @@
       ],
     },
     {
+      'id': 17,
       'name': 'Drive',
       'policies': [
         'DriveDisabled',
@@ -16566,6 +16583,7 @@
       ],
     },
     {
+      'id': 18,
       'name': 'Attestation',
       'policies': [
         'AttestationEnabledForDevice',
@@ -16575,6 +16593,7 @@
       ],
     },
     {
+      'id': 19,
       'name': 'ContentPack',
       'policies': [
         'ContentPackDefaultFilteringBehavior',
@@ -16583,6 +16602,7 @@
       ],
     },
     {
+      'id': 20,
       'name': 'SupervisedUsers',
       'policies': [
         'SupervisedUsersEnabled',
@@ -16591,6 +16611,7 @@
       ],
     },
     {
+      'id': 21,
       'name': 'GoogleCast',
       'policies': [
         'CastReceiverEnabled',
@@ -16598,6 +16619,7 @@
       ],
     },
     {
+      'id': 22,
       'name': 'QuickUnlock',
       'policies': [
         'QuickUnlockModeWhitelist',
@@ -16605,6 +16627,7 @@
       ],
     },
     {
+      'id': 23,
       'name': 'PinUnlock',
       'policies': [
         'PinUnlockMinimumLength',
@@ -16613,6 +16636,7 @@
       ],
     },
     {
+      'id': 24,
       'name': 'SafeBrowsing',
       'policies': [
         'SafeBrowsingEnabled',
@@ -16622,6 +16646,7 @@
       ],
     },
     {
+      'id': 25,
       'name': 'PasswordProtection',
       'policies': [
         'PasswordProtectionWarningTrigger',
@@ -16630,6 +16655,7 @@
       ],
     },
     {
+      'id': 26,
       'name': 'NetworkFileShares',
       'policies': [
         'NetworkFileSharesAllowed',
@@ -16639,6 +16665,7 @@
       ],
     },
     {
+      'id': 27,
       'name': 'ChromeReportingExtension',
       'policies': [
         'ReportVersionData',
@@ -16651,6 +16678,7 @@
       ],
     },
     {
+      'id': 28,
       'name': 'BrowserSwitcher',
       'policies': [
         'AlternativeBrowserPath',
@@ -16668,6 +16696,7 @@
       ],
     },
     {
+      'id': 29,
       'name': 'PluginVm',
       'policies': [
         'PluginVmAllowed',
@@ -16676,6 +16705,7 @@
       ],
     },
     {
+      'id': 30,
       'name': 'DeviceSAML',
       'policies': [
         'DeviceSamlLoginAuthenticationType',
@@ -16683,6 +16713,7 @@
       ],
     },
     {
+      'id': 31,
       'name': 'DeviceLoginScreenOrigins',
       'policies': [
         'DeviceLoginScreenIsolateOrigins',
@@ -16690,6 +16721,7 @@
       ],
     },
     {
+      'id': 32,
       'name': 'UserAndDeviceReporting',
       'policies': [
         'ReportDeviceVersionInfo',
@@ -16712,6 +16744,7 @@
       ],
     },
     {
+      'id': 33,
       'name': 'DeviceWiFi',
       'policies': [
         'DeviceWiFiFastTransitionEnabled',
@@ -16719,6 +16752,7 @@
       ],
     },
     {
+      'id': 34,
       'name': 'Kiosk',
       'policies': [
         'DeviceLocalAccounts',
@@ -16729,6 +16763,7 @@
       ],
     },
     {
+      'id': 35,
       'name': 'DateAndTime',
       'policies': [
         'SystemTimezone',
@@ -16736,6 +16771,7 @@
       ]
     },
     {
+      'id': 36,
       'name': 'Display',
       'policies': [
         'DeviceDisplayResolution',
@@ -16743,6 +16779,7 @@
       ]
     },
     {
+      'id': 37,
       'name': 'ActiveDirectoryManagement',
       'policies': [
         'DeviceMachinePasswordChangeRate',
@@ -16755,5 +16792,6 @@
   ],
   'placeholders': [],
   'deleted_policy_ids': [412, 546],
-  'highest_id_currently_used': 568
+  'highest_id_currently_used': 568,
+  'highest_atomic_group_id_currently_used': 37
 }
diff --git a/components/policy/tools/generate_policy_source.py b/components/policy/tools/generate_policy_source.py
index 8411d46..1a6292f 100755
--- a/components/policy/tools/generate_policy_source.py
+++ b/components/policy/tools/generate_policy_source.py
@@ -164,6 +164,7 @@
 
   def __init__(self, policy_group, available_policies,
                policies_already_in_group):
+    self.id = policy_group['id']
     self.name = policy_group['name']
     self.policies = policy_group.get('policies', None)
     self._CheckPoliciesValidity(available_policies, policies_already_in_group)
@@ -439,6 +440,7 @@
   f.write('\n}  // namespace group\n\n')
 
   f.write('struct AtomicGroup {\n'
+          '  const short id;\n'
           '  const char* policy_group;\n'
           '  const char* const* policies;\n'
           '};\n\n')
@@ -1123,7 +1125,8 @@
   for group in policy_atomic_groups:
     atomic_groups_length += 1
     f.write('  {')
-    f.write('  group::k{name}, group::{name}'.format(name=group.name))
+    f.write('  {id}, group::k{name}, group::{name}'.format(
+        id=group.id, name=group.name))
     f.write('  },\n')
   f.write('};\n\n')
   f.write('const size_t kPolicyAtomicGroupMappingsLength = %s;\n\n' %
diff --git a/components/policy/tools/syntax_check_policy_template_json.py b/components/policy/tools/syntax_check_policy_template_json.py
index 6be4b4f..e628b63 100755
--- a/components/policy/tools/syntax_check_policy_template_json.py
+++ b/components/policy/tools/syntax_check_policy_template_json.py
@@ -713,6 +713,26 @@
       with open(filename, 'w') as f:
         f.writelines(fixed_lines)
 
+  def _ValidatePolicyAtomicGroups(self, atomic_groups, max_id):
+    ids = [x['id'] for x in atomic_groups]
+    actual_highest_id = max(ids)
+    if actual_highest_id != max_id:
+      self._Error(
+          ("\'highest_atomic_group_id_currently_used\' must be set to the "
+           "highest atomic group id in use, which is currently %s (vs %s).") %
+          (actual_highest_id, max_id))
+      return
+
+    ids_set = set()
+    for i in range(len(ids)):
+      if (ids[i] in ids_set):
+        self._Error('Duplicate atomic group id %s' % (ids[i]))
+        return
+      ids_set.add(ids[i])
+      if i + 1 != ids[i]:
+        self._Error('Missing atomic group id %s' % (i + 1))
+        return
+
   def Main(self, filename, options):
     try:
       with open(filename, "rb") as f:
@@ -765,6 +785,13 @@
         parent_element=None,
         container_name='The root element',
         offending=None)
+    highest_atomic_group_id = self._CheckContains(
+        data,
+        'highest_atomic_group_id_currently_used',
+        int,
+        parent_element=None,
+        container_name='The root element',
+        offending=None)
     device_policy_proto_map = self._CheckContains(
         data,
         'device_policy_proto_map',
@@ -787,6 +814,8 @@
         container_name='The root element',
         offending=None)
 
+    self._ValidatePolicyAtomicGroups(policy_atomic_group_definitions,
+                                     highest_atomic_group_id)
     self._CheckDevicePolicyProtoMappingUniqueness(
         device_policy_proto_map, legacy_device_policy_proto_map)
 
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index fc1cd03..d37fc22 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -46206,6 +46206,48 @@
   <int value="5" label="5"/>
 </enum>
 
+<enum name="PolicyAtomicGroups">
+<!-- Generated from components/policy/resources/policy_templates.json -->
+
+  <int value="1" label="Homepage"/>
+  <int value="2" label="RemoteAccess"/>
+  <int value="3" label="PasswordManager"/>
+  <int value="4" label="Proxy"/>
+  <int value="5" label="Extensions"/>
+  <int value="6" label="RestoreOnStartupGroup"/>
+  <int value="7" label="DefaultSearchProvider"/>
+  <int value="8" label="ImageSettings"/>
+  <int value="9" label="CookiesSettings"/>
+  <int value="10" label="JavascriptSettings"/>
+  <int value="11" label="PluginsSettings"/>
+  <int value="12" label="PopupsSettings"/>
+  <int value="13" label="KeygenSettings"/>
+  <int value="14" label="NotificationsSettings"/>
+  <int value="15" label="WebUsbSettings"/>
+  <int value="16" label="NativeMessaging"/>
+  <int value="17" label="Drive"/>
+  <int value="18" label="Attestation"/>
+  <int value="19" label="ContentPack"/>
+  <int value="20" label="SupervisedUsers"/>
+  <int value="21" label="GoogleCast"/>
+  <int value="22" label="QuickUnlock"/>
+  <int value="23" label="PinUnlock"/>
+  <int value="24" label="SafeBrowsing"/>
+  <int value="25" label="PasswordProtection"/>
+  <int value="26" label="NetworkFileShares"/>
+  <int value="27" label="ChromeReportingExtension"/>
+  <int value="28" label="BrowserSwitcher"/>
+  <int value="29" label="PluginVm"/>
+  <int value="30" label="DeviceSAML"/>
+  <int value="31" label="DeviceLoginScreenOrigins"/>
+  <int value="32" label="UserAndDeviceReporting"/>
+  <int value="33" label="DeviceWiFi"/>
+  <int value="34" label="Kiosk"/>
+  <int value="35" label="DateAndTime"/>
+  <int value="36" label="Display"/>
+  <int value="37" label="ActiveDirectoryManagement"/>
+</enum>
+
 <enum name="PolicyLoadStatus">
   <int value="0" label="Success"/>
   <int value="1" label="No Policy File"/>
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 46ecea0..b80322d 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -31827,6 +31827,17 @@
   </summary>
 </histogram>
 
+<histogram name="Enterprise.PolicyAtomicGroup.SourceConflicts"
+    enum="PolicyAtomicGroups" expires_after="M78">
+  <owner>ydago@chromium.org</owner>
+  <summary>
+    A set of policy atomic groups that have at least one policy disabled because
+    of its source. This is recorded at startup, then every 24 hours. If chrome
+    is not running at the 24 hours mark, this will be recorded at the next
+    startup.
+  </summary>
+</histogram>
+
 <histogram name="Enterprise.PolicyHasVerifiedCachedKey"
     enum="BooleanValidKeyExists" expires_after="M77">
   <owner>atwilson@chromium.org</owner>
diff --git a/tools/metrics/histograms/update_policies.py b/tools/metrics/histograms/update_policies.py
index 40dc23e..c90edc7 100644
--- a/tools/metrics/histograms/update_policies.py
+++ b/tools/metrics/histograms/update_policies.py
@@ -25,7 +25,8 @@
 
 ENUMS_PATH = histogram_paths.ENUMS_XML
 POLICY_TEMPLATES_PATH = 'components/policy/resources/policy_templates.json'
-ENUM_NAME = 'EnterprisePolicies'
+POLICIES_ENUM_NAME = 'EnterprisePolicies'
+POLICY_ATOMIC_GROUPS_ENUM_NAME = 'PolicyAtomicGroups'
 
 class UserError(Exception):
   def __init__(self, message):
@@ -36,7 +37,7 @@
     return self.args[0]
 
 
-def UpdateHistogramDefinitions(policy_templates, doc):
+def UpdatePoliciesHistogramDefinitions(policy_templates, doc):
   """Sets the children of <enum name="EnterprisePolicies" ...> node in |doc| to
   values generated from policy ids contained in |policy_templates|.
 
@@ -49,7 +50,7 @@
   """
   # Find EnterprisePolicies enum.
   for enum_node in doc.getElementsByTagName('enum'):
-    if enum_node.attributes['name'].value == ENUM_NAME:
+    if enum_node.attributes['name'].value == POLICIES_ENUM_NAME:
         policy_enum_node = enum_node
         break
   else:
@@ -74,6 +75,44 @@
     policy_enum_node.appendChild(node)
 
 
+def UpdateAtomicGroupsHistogramDefinitions(policy_templates, doc):
+  """Sets the children of <enum name="PolicyAtomicGroups" ...> node in |doc| to
+  values generated from policy ids contained in |policy_templates|.
+
+  Args:
+    policy_templates: A list of dictionaries, defining policy atomic
+                      groups. The format is exactly the same as in
+                      policy_templates.json file.
+    doc: A minidom.Document object representing parsed histogram definitions
+         XML file.
+  """
+  # Find EnterprisePolicies enum.
+  for enum_node in doc.getElementsByTagName('enum'):
+    if enum_node.attributes['name'].value == POLICY_ATOMIC_GROUPS_ENUM_NAME:
+        atomic_group_enum_node = enum_node
+        break
+  else:
+    raise UserError('No policy atomic group enum node found')
+
+  # Remove existing values.
+  while atomic_group_enum_node.hasChildNodes():
+    atomic_group_enum_node.removeChild(atomic_group_enum_node.lastChild)
+
+  # Add a "Generated from (...)" comment
+  comment = ' Generated from {0} '.format(POLICY_TEMPLATES_PATH)
+  atomic_group_enum_node.appendChild(doc.createComment(comment))
+
+  # Add values generated from policy templates.
+  ordered_atomic_groups = [
+    x for x in policy_templates['policy_atomic_group_definitions']
+  ]
+  ordered_atomic_groups.sort(key=lambda policy: policy['id'])
+  for group in ordered_atomic_groups:
+    node = doc.createElement('int')
+    node.attributes['value'] = str(group['id'])
+    node.attributes['label'] = group['name']
+    atomic_group_enum_node.appendChild(node)
+
 def main():
   if len(sys.argv) > 1:
     print >>sys.stderr, 'No arguments expected!'
@@ -82,12 +121,14 @@
 
   with open(path_util.GetInputFile(POLICY_TEMPLATES_PATH), 'rb') as f:
     policy_templates = literal_eval(f.read())
+
   with open(ENUMS_PATH, 'rb') as f:
     histograms_doc = minidom.parse(f)
     f.seek(0)
     xml = f.read()
 
-  UpdateHistogramDefinitions(policy_templates, histograms_doc)
+  UpdatePoliciesHistogramDefinitions(policy_templates, histograms_doc)
+  UpdateAtomicGroupsHistogramDefinitions(policy_templates, histograms_doc)
   new_xml = histograms_print_style.GetPrintStyle().PrettyPrintXml(
       histograms_doc)
   if PromptUserToAcceptDiff(xml, new_xml, 'Is the updated version acceptable?'):