Android expectations: Improve node sort order

#1 - Ignore XML namespaces when sorting
See https://chromium-review.googlesource.com/c/chromium/src/+/2321522
for an example of where this was causing them to change order.

#2 - Prefer to sort by android:name when present
Prevents order change when attributes are added/removed.

#3 - Don't sort top-level nodes reversed (other than <application>)

Bug: 1064151
Change-Id: If552c2c2baa1f538a00d9ddfa3ead6feb19ce6b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366038
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800203}

android_webview/expectations/system_webview_bundle.AndroidManifest.expected

@@ -6,12 +6,12 @@
     tools:ignore="MissingLeanbackLauncher"
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:tools="http://schemas.android.com/tools">
-  <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="30"/>
-  <uses-permission android:name="android.permission.INTERNET"/>
-  <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
-  <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
-  <uses-feature android:name="android.software.leanback" android:required="false"/>
   <uses-feature android:name="android.hardware.touchscreen" android:required="false"/>
+  <uses-feature android:name="android.software.leanback" android:required="false"/>
+  <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
+  <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
+  <uses-permission android:name="android.permission.INTERNET"/>
+  <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="30"/>
   <application
       android:extractNativeLibs="True"
       android:icon="@$PACKAGE:drawable/icon_webview"
@@ -60,6 +60,12 @@
     <meta-data android:name="com.google.android.gms.version" android:value="@integer/google_play_services_version"/>
     <meta-data android:name="org.chromium.content.browser.NUM_PRIVILEGED_SERVICES" android:value="0"/>
     <meta-data android:name="org.chromium.content.browser.NUM_SANDBOXED_SERVICES" android:value="40"/>
+    <provider  # DIFF-ANCHOR: a5e78e63
+        android:authorities="$PACKAGE.LicenseContentProvider"
+        android:exported="true"
+        android:name="org.chromium.android_webview.nonembedded.LicenseContentProvider"
+        android:process=":webview_apk">
+    </provider>  # DIFF-ANCHOR: a5e78e63
     <provider  # DIFF-ANCHOR: bfe37944
         android:authorities="$PACKAGE.DeveloperModeContentProvider"
         android:exported="true"
@@ -67,34 +73,28 @@
         android:process=":webview_service"
         android:visibleToInstantApps="true">
     </provider>  # DIFF-ANCHOR: bfe37944
-    <provider  # DIFF-ANCHOR: a5e78e63
-        android:authorities="$PACKAGE.LicenseContentProvider"
+    <service  # DIFF-ANCHOR: 3cd6d713
         android:exported="true"
-        android:name="org.chromium.android_webview.nonembedded.LicenseContentProvider"
-        android:process=":webview_apk">
-    </provider>  # DIFF-ANCHOR: a5e78e63
+        android:name="org.chromium.android_webview.services.AwMinidumpUploadJobService"
+        android:permission="android.permission.BIND_JOB_SERVICE"
+        android:process=":webview_service">
+    </service>  # DIFF-ANCHOR: 3cd6d713
     <service  # DIFF-ANCHOR: 65cddb26
         android:exported="false"
         android:name="org.chromium.android_webview.services.AwVariationsSeedFetcher"
         android:permission="android.permission.BIND_JOB_SERVICE"
         android:process=":webview_service">
     </service>  # DIFF-ANCHOR: 65cddb26
-    <service  # DIFF-ANCHOR: adce9ea1
-        android:exported="false"
-        android:name="org.chromium.android_webview.services.DeveloperUiService"
-        android:process=":webview_service">
-    </service>  # DIFF-ANCHOR: adce9ea1
-    <service  # DIFF-ANCHOR: 3cd6d713
-        android:exported="true"
-        android:name="org.chromium.android_webview.services.AwMinidumpUploadJobService"
-        android:permission="android.permission.BIND_JOB_SERVICE"
-        android:process=":webview_service">
-    </service>  # DIFF-ANCHOR: 3cd6d713
     <service  # DIFF-ANCHOR: 5cda9608
         android:exported="true"
         android:name="org.chromium.android_webview.services.CrashReceiverService"
         android:process=":webview_service">
     </service>  # DIFF-ANCHOR: 5cda9608
+    <service  # DIFF-ANCHOR: adce9ea1
+        android:exported="false"
+        android:name="org.chromium.android_webview.services.DeveloperUiService"
+        android:process=":webview_service">
+    </service>  # DIFF-ANCHOR: adce9ea1
     <service  # DIFF-ANCHOR: eecf2fee
         android:exported="true"
         android:name="org.chromium.android_webview.services.MetricsBridgeService"

android_webview/expectations/trichrome_webview_bundle.AndroidManifest.expected

@@ -6,12 +6,12 @@
     tools:ignore="MissingLeanbackLauncher"
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:tools="http://schemas.android.com/tools">
-  <uses-sdk android:minSdkVersion="29" android:targetSdkVersion="30"/>
-  <uses-permission android:name="android.permission.INTERNET"/>
-  <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
-  <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
-  <uses-feature android:name="android.software.leanback" android:required="false"/>
   <uses-feature android:name="android.hardware.touchscreen" android:required="false"/>
+  <uses-feature android:name="android.software.leanback" android:required="false"/>
+  <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE"/>
+  <uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
+  <uses-permission android:name="android.permission.INTERNET"/>
+  <uses-sdk android:minSdkVersion="29" android:targetSdkVersion="30"/>
   <application
       android:extractNativeLibs="False"
       android:icon="@$PACKAGE:drawable/icon_webview"
@@ -60,6 +60,12 @@
     <meta-data android:name="com.google.android.gms.version" android:value="@integer/google_play_services_version"/>
     <meta-data android:name="org.chromium.content.browser.NUM_PRIVILEGED_SERVICES" android:value="0"/>
     <meta-data android:name="org.chromium.content.browser.NUM_SANDBOXED_SERVICES" android:value="40"/>
+    <provider  # DIFF-ANCHOR: a5e78e63
+        android:authorities="$PACKAGE.LicenseContentProvider"
+        android:exported="true"
+        android:name="org.chromium.android_webview.nonembedded.LicenseContentProvider"
+        android:process=":webview_apk">
+    </provider>  # DIFF-ANCHOR: a5e78e63
     <provider  # DIFF-ANCHOR: bfe37944
         android:authorities="$PACKAGE.DeveloperModeContentProvider"
         android:exported="true"
@@ -67,34 +73,28 @@
         android:process=":webview_service"
         android:visibleToInstantApps="true">
     </provider>  # DIFF-ANCHOR: bfe37944
-    <provider  # DIFF-ANCHOR: a5e78e63
-        android:authorities="$PACKAGE.LicenseContentProvider"
+    <service  # DIFF-ANCHOR: 3cd6d713
         android:exported="true"
-        android:name="org.chromium.android_webview.nonembedded.LicenseContentProvider"
-        android:process=":webview_apk">
-    </provider>  # DIFF-ANCHOR: a5e78e63
+        android:name="org.chromium.android_webview.services.AwMinidumpUploadJobService"
+        android:permission="android.permission.BIND_JOB_SERVICE"
+        android:process=":webview_service">
+    </service>  # DIFF-ANCHOR: 3cd6d713
     <service  # DIFF-ANCHOR: 65cddb26
         android:exported="false"
         android:name="org.chromium.android_webview.services.AwVariationsSeedFetcher"
         android:permission="android.permission.BIND_JOB_SERVICE"
         android:process=":webview_service">
     </service>  # DIFF-ANCHOR: 65cddb26
-    <service  # DIFF-ANCHOR: adce9ea1
-        android:exported="false"
-        android:name="org.chromium.android_webview.services.DeveloperUiService"
-        android:process=":webview_service">
-    </service>  # DIFF-ANCHOR: adce9ea1
-    <service  # DIFF-ANCHOR: 3cd6d713
-        android:exported="true"
-        android:name="org.chromium.android_webview.services.AwMinidumpUploadJobService"
-        android:permission="android.permission.BIND_JOB_SERVICE"
-        android:process=":webview_service">
-    </service>  # DIFF-ANCHOR: 3cd6d713
     <service  # DIFF-ANCHOR: 5cda9608
         android:exported="true"
         android:name="org.chromium.android_webview.services.CrashReceiverService"
         android:process=":webview_service">
     </service>  # DIFF-ANCHOR: 5cda9608
+    <service  # DIFF-ANCHOR: adce9ea1
+        android:exported="false"
+        android:name="org.chromium.android_webview.services.DeveloperUiService"
+        android:process=":webview_service">
+    </service>  # DIFF-ANCHOR: adce9ea1
     <service  # DIFF-ANCHOR: eecf2fee
         android:exported="true"
         android:name="org.chromium.android_webview.services.MetricsBridgeService"

build/android/gyp/util/manifest_utils.py

@@ -6,6 +6,7 @@
 
 import hashlib
 import os
+import re
 import shlex
 import xml.dom.minidom as minidom
 
@@ -121,12 +122,39 @@ def AssertPackage(manifest_node, package):
                                                               package))
 
 
-def _SortAndStripElementTree(tree, reverse_toplevel=False):
-  for node in tree:
-    if node.text and node.text.isspace():
-      node.text = None
-    _SortAndStripElementTree(node)
-  tree[:] = sorted(tree, key=ElementTree.tostring, reverse=reverse_toplevel)
+def _SortAndStripElementTree(root):
+  def sort_key(node):
+    ret = ElementTree.tostring(node)
+    # ElementTree.tostring inserts namespace attributes for any that are needed
+    # for the node or any of its descendants. Remove them so as to prevent a
+    # change to a child that adds/removes a namespace usage from changing sort
+    # order.
+    return re.sub(r' xmlns:.*?".*?"', '', ret)
+
+  def helper(node):
+    for child in node:
+      if child.text and child.text.isspace():
+        child.text = None
+      helper(child)
+    node[:] = sorted(node, key=sort_key)
+
+  def rename_attrs(node, from_name, to_name):
+    value = node.attrib.get(from_name)
+    if value is not None:
+      node.attrib[to_name] = value
+      del node.attrib[from_name]
+    for child in node:
+      rename_attrs(child, from_name, to_name)
+
+  # Sort alphabetically with two exceptions:
+  # 1) Put <application> node last (since it's giant).
+  # 2) Pretend android:name appears before other attributes.
+  app_node = root.find('application')
+  app_node.tag = 'zz'
+  rename_attrs(root, '{%s}name' % ANDROID_NAMESPACE, '__name__')
+  helper(root)
+  rename_attrs(root, '__name__', '{%s}name' % ANDROID_NAMESPACE)
+  app_node.tag = 'application'
 
 
 def _SplitElement(line):
@@ -256,8 +284,7 @@ def NormalizeManifest(manifest_contents):
   for child in root.getchildren():
     blur_package_name(child)
 
-  # Sort nodes alphabetically, recursively.
-  _SortAndStripElementTree(root, reverse_toplevel=True)
+  _SortAndStripElementTree(root)
 
   # Fix up whitespace/indentation.
   dom = minidom.parseString(ElementTree.tostring(root))

build/android/gyp/util/manifest_utils_test.py

@@ -53,9 +53,9 @@ _TEST_MANIFEST_NORMALIZED = """\
     tools:ignore="MissingVersion"
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:tools="http://schemas.android.com/tools">
-  <uses-sdk android:minSdkVersion="24" android:targetSdkVersion="30"/>
   <uses-feature android:name="android.hardware.vr.headtracking" \
 android:required="false" android:version="1"/>
+  <uses-sdk android:minSdkVersion="24" android:targetSdkVersion="30"/>
   <application android:name="testname">
     <activity  # DIFF-ANCHOR: {activity_diff_anchor}
         {extra_activity_attr}android:icon="@drawable/ic_devices_48dp"

chrome/android/expectations/trichrome_library_apk.AndroidManifest.expected

@@ -6,10 +6,10 @@
     tools:ignore="MissingVersion,MissingLeanbackLauncher"
     xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:tools="http://schemas.android.com/tools">
-  <uses-sdk android:minSdkVersion="29" android:targetSdkVersion="30"/>
-  <uses-feature android:name="android.software.leanback" android:required="false"/>
   <uses-feature android:name="android.hardware.touchscreen" android:required="false"/>
+  <uses-feature android:name="android.software.leanback" android:required="false"/>
   <uses-feature android:glEsVersion="0x00020000"/>
+  <uses-sdk android:minSdkVersion="29" android:targetSdkVersion="30"/>
   <application
       android:extractNativeLibs="false"
       android:icon="@drawable/icon_webview"