[Policies] Distinguish deprecated/removed policies in ADM(X) templates

Right now, a policy being "deprecated" can mean one of 2 things:

  (a) The policy still works, but it'll get removed soon.  (deprecated)
  (b) The policy doesn't work anymore.  (removed)

Right now, They both get placed in a "Deprecated Policies" group, in the
ADM/ADMX files.

This patch creates a new group: "Removed Policies". The old group is
for (a), and the new group is for (b).

Bug: 1086425
Change-Id: I728f78f52e114340cadb166b62d54f9bb43c3efd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2255139
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781393}

components/policy/resources/policy_templates.json

@@ -21815,6 +21815,18 @@ The recommended way to configure policy on Windows is via GPO, although provisio
       'desc': '''Description shared by all deprecated policies, in Microsoft Windows' Group Policy Editor.''',
       'text': '''This policy is deprecated. Its usage is discouraged. Read more at https://support.google.com/chrome/a/answer/7643500'''
     },
+    'removed_policy_group_caption': {
+      'desc': '''Localized name for the removed policies folder, for Microsoft's Group Policy Editor.''',
+      'text': '''Removed policies''',
+    },
+    'removed_policy_group_desc': {
+      'desc': '''Localized description for the removed policies folder, for Microsoft's Group Policy Editor.''',
+      'text': '''These policies are included here to make them easy to remove.''',
+    },
+    'removed_policy_desc': {
+      'desc': '''Description shared by all removed policies, in Microsoft Windows' Group Policy Editor.''',
+      'text': '''This policy is removed. It is not compatible with this version of <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph>. Read more at https://support.google.com/chrome/a/answer/7643500'''
+    },
   },
   # Legacy device policies that don't have a 1:1 mapping between template and
   # chrome_device_policy.proto or where the types don't map the same way as for

components/policy/tools/template_writers/writer_configuration.py

@@ -126,6 +126,8 @@ def GetConfigurationForBuild(defines):
     raise Exception('Unknown build')
   if 'version' in defines:
     config['version'] = defines['version']
+  if 'major_version' in defines:
+    config['major_version'] = defines['major_version']
   config['win_supported_os'] = 'SUPPORTED_WIN7'
   config['win_supported_os_win7'] = 'SUPPORTED_WIN7_ONLY'
   if 'mac_bundle_id' in defines:

components/policy/tools/template_writers/writers/adm_writer.py

@@ -188,7 +188,8 @@ class AdmWriter(gpo_editor_writer.GpoEditorWriter):
 
     if policy_desc is not None:
       policy_desc += '\n\n'
-      if not policy.get('deprecated', False):
+      if (not policy.get('deprecated', False) and
+          not self._IsRemovedPolicy(policy)):
         policy_desc += reference_link_text
       return policy_desc
     else:

components/policy/tools/template_writers/writers/adm_writer_unittest.py

@@ -39,6 +39,17 @@ MESSAGES = '''
       'desc': 'bleh',
       'text': 'This policy is deprecated. blah blah blah'
     },
+    'removed_policy_group_caption': {
+      'text': 'Removed policies', 'desc': 'bleh'
+    },
+    'removed_policy_group_desc': {
+      'desc': 'bleh',
+      'text': 'These policies are included here to make them easy to remove.'
+    },
+    'removed_policy_desc': {
+      'desc': 'bleh',
+      'text': 'This policy is removed. blah blah blah'
+    },
   }'''
 
 
@@ -1307,6 +1318,82 @@ Policy1_Part="Caption of policy1."
 ''')
     self.CompareOutputs(output, expected_output)
 
+  def testRemovedPolicy(self):
+    # Tests that a deprecated policy gets placed in the special
+    # 'RemovedPolicies' group.
+    policy_json = '''
+      {
+        'policy_definitions': [
+          {
+            'name': 'Policy1',
+            'type': 'string',
+            'deprecated': True,
+            'features': { 'can_be_recommended': True },
+            'supported_on': ['chrome.win:40-83'],
+            'caption': 'Caption of policy1.',
+            'desc': """Description of policy1."""
+          },
+        ],
+        'policy_atomic_group_definitions': [],
+        'placeholders': [],
+        'messages': %s
+      }''' % MESSAGES
+    output = self.GetOutput(policy_json, {'_chromium': '1',
+                                          'major_version': 84}, 'adm')
+    expected_output = self.ConstructOutput(['MACHINE', 'USER'], '''
+  CATEGORY !!chromium
+    KEYNAME "Software\\Policies\\Chromium"
+
+    CATEGORY !!RemovedPolicies_Category
+      POLICY !!Policy1_Policy
+        #if version >= 4
+          SUPPORTED !!SUPPORTED_WIN7
+        #endif
+        EXPLAIN !!Policy1_Explain
+
+        PART !!Policy1_Part  EDITTEXT
+          VALUENAME "Policy1"
+          MAXLEN 1000000
+        END PART
+      END POLICY
+
+    END CATEGORY
+
+  END CATEGORY
+
+  CATEGORY !!chromium_recommended
+    KEYNAME "Software\\Policies\\Chromium\\Recommended"
+
+    CATEGORY !!RemovedPolicies_Category
+      POLICY !!Policy1_Policy
+        #if version >= 4
+          SUPPORTED !!SUPPORTED_WIN7
+        #endif
+        EXPLAIN !!Policy1_Explain
+
+        PART !!Policy1_Part  EDITTEXT
+          VALUENAME "Policy1"
+          MAXLEN 1000000
+        END PART
+      END POLICY
+
+    END CATEGORY
+
+  END CATEGORY
+
+
+''', '''[Strings]
+SUPPORTED_WIN7="Microsoft Windows 7 or later"
+SUPPORTED_WIN7_ONLY="Microsoft Windows 7"
+chromium="Chromium"
+chromium_recommended="Chromium - Recommended"
+RemovedPolicies_Category="Removed policies"
+Policy1_Policy="Caption of policy1."
+Policy1_Explain="This policy is removed. blah blah blah\\n\\n"
+Policy1_Part="Caption of policy1."
+''')
+    self.CompareOutputs(output, expected_output)
+
 
 
 if __name__ == '__main__':

components/policy/tools/template_writers/writers/adml_writer.py

@@ -83,7 +83,8 @@ class ADMLWriter(xml_formatted_writer.XMLFormattedWriter,
     if policy_desc is not None and self.HasExpandedPolicyDescription(policy):
       policy_desc += '\n' + self.GetExpandedPolicyDescription(policy) + '\n'
 
-    if policy_desc is not None and example_value_text is not None:
+    if (policy_desc is not None and example_value_text is not None and
+        not self._IsRemovedPolicy(policy)):
       policy_explain = policy_desc + '\n\n' + example_value_text
     elif policy_desc is not None:
       policy_explain = policy_desc

components/policy/tools/template_writers/writers/adml_writer_unittest.py

@@ -210,6 +210,7 @@ class AdmlWriterUnittest(xml_writer_base_unittest.XmlWriterBaseTest):
         'caption': 'String policy caption',
         'label': 'String policy label',
         'desc': 'This is a test description.',
+        'supported_on': [{'platform': 'win'}, {'platform': 'chrome_os'}],
         'example_value': '01:23:45:67:89:ab',
     }
     self._InitWriterForAddingPolicies(self.writer, string_policy)

components/policy/tools/template_writers/writers/gpo_editor_writer.py

@@ -25,10 +25,13 @@ class GpoEditorWriter(template_writer.TemplateWriter):
     #
     # TODO(crbug.com/463990): Eventually exclude some policies, e.g. if they
     # were deprecated a long time ago.
-    if policy.get('deprecated', False):
+    major_version = self._GetChromiumMajorVersion()
+    if not major_version:
       return True
 
-    return super(GpoEditorWriter, self).IsVersionSupported(policy, supported_on)
+    since_version = supported_on.get('since_version', None)
+
+    return not since_version or since_version >= major_version
 
   def IsPolicyOnWin7Only(self, policy):
     ''' Returns true if the policy is supported on win7 only.'''
@@ -37,18 +40,28 @@ class GpoEditorWriter(template_writer.TemplateWriter):
         return True
     return False
 
+  def _IsRemovedPolicy(self, policy):
+    major_version = self._GetChromiumMajorVersion()
+    for supported_on in policy.get('supported_on', []):
+      if '*' in self.platforms or supported_on['platform'] in self.platforms:
+        until_version = supported_on.get('until_version', None)
+        if not until_version or major_version <= int(until_version):
+          # The policy is still supported, return False.
+          return False
+    # No platform+version combo supports this version, return True.
+    return True
 
-  def _FindDeprecatedPolicies(self, policy_list):
-    deprecated_policies = []
+  def _FilterPolicies(self, predicate, policy_list):
+    filtered_policies = []
     for policy in policy_list:
       if policy['type'] == 'group':
         for p in policy['policies']:
-          if p.get('deprecated', False):
-            deprecated_policies.append(p)
+          if predicate(p):
+            filtered_policies.append(p)
       else:
-        if policy.get('deprecated', False):
-          deprecated_policies.append(policy)
-    return deprecated_policies
+        if predicate(policy):
+          filtered_policies.append(policy)
+    return filtered_policies
 
   def _RemovePoliciesFromList(self, policy_list, policies_to_remove):
     '''Remove policies_to_remove from groups and the top-level list.'''
@@ -64,28 +77,51 @@ class GpoEditorWriter(template_writer.TemplateWriter):
     for group in policy_list:
       if group['type'] != 'group':
         continue
+      # TODO(nicolaso): Remove empty groups
       group['policies'] = [
           p for p in group['policies'] if p['name'] not in policies_to_remove
       ]
 
-  def PreprocessPolicies(self, policy_list):
-    '''Put deprecated policies under the  'DeprecatedPolicies' group.'''
-    deprecated_policies = self._FindDeprecatedPolicies(policy_list)
+  def _MovePolicyGroup(self, policy_list, predicate, policy_desc, group):
+    '''Remove policies from |policy_list| that satisfy |predicate| and add them
+    to |group|.'''
+    filtered_policies = self._FilterPolicies(predicate, policy_list)
+    self._RemovePoliciesFromList(policy_list, filtered_policies)
+
+    for p in filtered_policies:
+      p['desc'] = policy_desc
 
-    self._RemovePoliciesFromList(policy_list, deprecated_policies)
+    group['policies'] = filtered_policies
 
-    for p in deprecated_policies:
-      # TODO(crbug.com/463990): Also include an alternative policy in the
-      # description, if the policy was replaced by a newer one.
-      p['desc'] = self.messages['deprecated_policy_desc']['text']
+  def PreprocessPolicies(self, policy_list):
+    '''Put policies under the DeprecatedPolicies/RemovedPolicies groups.'''
+    removed_policies_group = {
+        'name': 'RemovedPolicies',
+        'type': 'group',
+        'caption': self.messages['removed_policy_group_caption']['text'],
+        'desc': self.messages['removed_policy_group_desc']['text'],
+        'policies': []
+    }
+    self._MovePolicyGroup(
+        policy_list,
+        lambda p: self._IsRemovedPolicy(p),
+        self.messages['removed_policy_desc']['text'],
+        removed_policies_group)
 
-    deprecated_group = {
+    deprecated_policies_group = {
         'name': 'DeprecatedPolicies',
         'type': 'group',
         'caption': self.messages['deprecated_policy_group_caption']['text'],
         'desc': self.messages['deprecated_policy_group_desc']['text'],
-        'policies': deprecated_policies
+        'policies': []
     }
-    policy_list.append(deprecated_group)
+    self._MovePolicyGroup(
+        policy_list,
+        lambda p: p.get('deprecated', False),
+        self.messages['deprecated_policy_desc']['text'],
+        deprecated_policies_group)
+
+    policy_list.append(deprecated_policies_group)
+    policy_list.append(removed_policies_group)
 
     return super(GpoEditorWriter, self).SortPoliciesGroupsFirst(policy_list)