SuperSize: Fix crash when a class has the same name as a package

This happens for:
   com.google.android.play.core.splitcompat.c

TBR=wnwen  # Fixing official bots

Bug: 887876
Change-Id: I37ce1fbc6f282228a33311136980aba509ad1eb8
Reviewed-on: https://chromium-review.googlesource.com/1240015Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593453}

tools/binary_size/libsupersize/apkanalyzer.py

@@ -50,7 +50,7 @@ def _RunApkAnalyzer(apk_path, output_directory):
     # pylint: disable=unused-variable
     node_type, state, defined_methods, referenced_methods, size, name = (
         vals[0], vals[1], vals[2], vals[3], vals[4], vals[5:])
-    data.append((' '.join(name), int(size)))
+    data.append((node_type, ' '.join(name), int(size)))
   return data
 
 
@@ -73,19 +73,17 @@ def UndoHierarchicalSizing(data):
 
   Example nodes:
     [
-      ('<TOTAL>', 37),
-      ('org', 30),
-      ('org.chromium', 25),
-      ('org.chromium.ClassA', 14),
-      ('org.chromium.ClassA void methodA()', 10),
-      ('org.chromium.ClassA$Proxy', 8),
+      ('P', '<TOTAL>', 37),
+      ('P', 'org', 32),
+      ('P', 'org.chromium', 32),
+      ('C', 'org.chromium.ClassA', 14),
+      ('M', 'org.chromium.ClassA void methodA()', 10),
+      ('C', 'org.chromium.ClassA$Proxy', 8),
     ]
 
   Processed nodes:
     [
-      ('<TOTAL>', 7),
-      ('org', 5),
-      ('org.chromium', 3),
+      ('<TOTAL>', 15),
       ('org.chromium.ClassA', 4),
       ('org.chromium.ClassA void methodA()', 10),
       ('org.chromium.ClassA$Proxy', 8),
@@ -96,14 +94,15 @@ def UndoHierarchicalSizing(data):
 
   def process_node(start_idx):
     assert start_idx < num_nodes, 'Attempting to parse beyond data array.'
-    name, size = data[start_idx]
+    node_type, name, size = data[start_idx]
     total_child_size = 0
     next_idx = start_idx + 1
     name_len = len(name)
     while next_idx < num_nodes:
-      next_name = data[next_idx][0]
+      next_name = data[next_idx][1]
       if name == _TOTAL_NODE_NAME or (
-          next_name.startswith(name) and next_name[name_len] in '. '):
+          len(next_name) > name_len and next_name.startswith(name)
+          and next_name[name_len] in '. '):
         # Child node
         child_next_idx, child_node_size = process_node(next_idx)
         next_idx = child_next_idx
@@ -123,7 +122,14 @@ def UndoHierarchicalSizing(data):
     #    'Child node total size exceeded parent node total size')
 
     node_size = size - total_child_size
-    nodes.append((name, node_size))
+    # It is valid to have a package and a class with the same name.
+    # To avoid having two symbols with the same name in these cases, do not
+    # create symbols for packages (which have no size anyways).
+    if node_type == 'P' and node_size != 0 and name != _TOTAL_NODE_NAME:
+      logging.warning('Unexpected java package that takes up size: %d, %s',
+                      node_size, name)
+    if node_type != 'P' or node_size != 0:
+      nodes.append((node_type, name, node_size))
     return next_idx, size
 
   idx = 0
@@ -137,7 +143,7 @@ def CreateDexSymbols(apk_path, output_directory):
   source_map = _LoadSourceMap(apk_name, output_directory)
   nodes = UndoHierarchicalSizing(_RunApkAnalyzer(apk_path, output_directory))
   dex_expected_size = _ExpectedDexTotalSize(apk_path)
-  total_node_size = sum(map(lambda x: x[1], nodes))
+  total_node_size = sum(map(lambda x: x[2], nodes))
   # TODO(agrieve): Figure out why this log is triggering for
   #     ChromeModernPublic.apk (https://crbug.com/851535).
   # Reporting: dex_expected_size=6546088 total_node_size=6559549
@@ -149,7 +155,7 @@ def CreateDexSymbols(apk_path, output_directory):
   # We have more than 100KB of ids for methods, strings
   id_metadata_overhead_size = dex_expected_size - total_node_size
   symbols = []
-  for name, node_size in nodes:
+  for _, name, node_size in nodes:
     package = name.split(' ', 1)[0]
     class_path = package.split('$')[0]
     source_path = source_map.get(class_path, '')

tools/binary_size/libsupersize/apkanalyzer_test.py

@@ -19,7 +19,7 @@ class ApkAnalyzerTest(unittest.TestCase):
 
   def testUndoHierarchicalSizing_TotalSingleRootNode(self):
     data = [
-      ('<TOTAL>', 5),
+      ('P', '<TOTAL>', 5),
     ]
     nodes = apkanalyzer.UndoHierarchicalSizing(data)
     # No changes expected since there are no child nodes.
@@ -27,19 +27,19 @@ class ApkAnalyzerTest(unittest.TestCase):
 
   def testUndoHierarchicalSizing_TotalSizeMinusChildNode(self):
     data = [
-      ('<TOTAL>', 10),
-      ('child1', 7),
+      ('P', '<TOTAL>', 10),
+      ('C', 'child1', 7),
     ]
     nodes = apkanalyzer.UndoHierarchicalSizing(data)
     self.assertEqualLists([
-      ('<TOTAL>', 3),
-      ('child1', 7),
+      ('P', '<TOTAL>', 3),
+      ('C', 'child1', 7),
     ], nodes)
 
   def testUndoHierarchicalSizing_SiblingAnonymousClass(self):
     data = [
-      ('class1', 10),
-      ('class1$inner', 8),
+      ('C', 'class1', 10),
+      ('C', 'class1$inner', 8),
     ]
     nodes = apkanalyzer.UndoHierarchicalSizing(data)
     # No change in size expected since these should be siblings.
@@ -47,39 +47,50 @@ class ApkAnalyzerTest(unittest.TestCase):
 
   def testUndoHierarchicalSizing_MethodsShouldBeChildNodes(self):
     data = [
-      ('class1', 10),
-      ('class1 method', 8),
+      ('C', 'class1', 10),
+      ('M', 'class1 method', 8),
     ]
     nodes = apkanalyzer.UndoHierarchicalSizing(data)
     self.assertEqualLists([
-      ('class1', 2),
-      ('class1 method', 8),
+      ('C', 'class1', 2),
+      ('M', 'class1 method', 8),
     ], nodes)
 
   def testUndoHierarchicalSizing_ClassIsChildNodeOfPackage(self):
     data = [
-      ('package1', 10),
-      ('package1.class1', 3),
+      ('P', 'package1', 10),
+      ('C', 'package1.class1', 10),
     ]
     nodes = apkanalyzer.UndoHierarchicalSizing(data)
     self.assertEqualLists([
-      ('package1', 7),
-      ('package1.class1', 3),
+      ('C', 'package1.class1', 10),
     ], nodes)
 
   def testUndoHierarchicalSizing_TotalIncludesAllPackages(self):
     data = [
-      ('<TOTAL>', 10),
-      ('package1', 3),
-      ('package2', 4),
-      ('package3', 2),
+      ('P', '<TOTAL>', 10),
+      ('C', 'class1', 3),
+      ('C', 'class2', 4),
+      ('C', 'class3', 2),
     ]
     nodes = apkanalyzer.UndoHierarchicalSizing(data)
     self.assertEqualLists([
-      ('<TOTAL>', 1),
-      ('package1', 3),
-      ('package2', 4),
-      ('package3', 2),
+      ('P', '<TOTAL>', 1),
+      ('C', 'class1', 3),
+      ('C', 'class2', 4),
+      ('C', 'class3', 2),
+    ], nodes)
+
+  def testUndoHierarchicalSizing_PackageAndClassSameName(self):
+    data = [
+      ('P', 'name', 4),
+      ('C', 'name.Class', 4),
+      ('C', 'name', 2),
+    ]
+    nodes = apkanalyzer.UndoHierarchicalSizing(data)
+    self.assertEqualLists([
+      ('C', 'name.Class', 4),
+      ('C', 'name', 2),
     ], nodes)