From 2427dce8205607a107880e13b4cdf1caa07cc425 Mon Sep 17 00:00:00 2001
From: Adam Lesinski <adamlesinski@google.com>
Date: Thu, 30 Nov 2017 15:10:28 -0800
Subject: AAPT2: Fix issue where exported symbols (@+id/*) were not exported

Test: make AaptTestAppOne
Change-Id: If3218c880e83c2cfaf535a099db38504471be676
---
 tools/aapt2/Android.mk                             |  4 ++
 tools/aapt2/cmd/Link.cpp                           | 44 +++++++++++++++-------
 .../StaticLibTest/App/AndroidManifest.xml          |  2 +-
 .../StaticLibTest/App/res/layout/main.xml          |  2 +-
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/tools/aapt2/Android.mk b/tools/aapt2/Android.mk
index e0c9d1c04498..c9987b86cc5c 100644
--- a/tools/aapt2/Android.mk
+++ b/tools/aapt2/Android.mk
@@ -1,7 +1,11 @@
+LOCAL_PATH := $(call my-dir)
+
 include $(CLEAR_VARS)
 
+# Target for running host unit tests on post/pre-submit.
 .PHONY: aapt2_run_host_unit_tests
 aapt2_run_host_unit_tests: PRIVATE_GTEST_OPTIONS := --gtest_output=xml:$(DIST_DIR)/gtest/aapt2_host_unit_tests_result.xml
 aapt2_run_host_unit_tests: $(HOST_OUT_NATIVE_TESTS)/aapt2_tests/aapt2_tests
 	-$(HOST_OUT_NATIVE_TESTS)/aapt2_tests/aapt2_tests $(PRIVATE_GTEST_OPTIONS) > /dev/null 2>&1
 
+include $(call all-makefiles-under,$(LOCAL_PATH))
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index e94c0b42daf1..d2aebfd31bbf 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -36,6 +36,7 @@
 #include "ValueVisitor.h"
 #include "cmd/Util.h"
 #include "compile/IdAssigner.h"
+#include "compile/XmlIdCollector.h"
 #include "filter/ConfigFilter.h"
 #include "format/Archive.h"
 #include "format/Container.h"
@@ -1235,19 +1236,10 @@ class LinkCommand {
     return true;
   }
 
-  bool MergeCompiledFile(const ResourceFile& compiled_file, io::IFile* file, bool override) {
-    if (context_->IsVerbose()) {
-      context_->GetDiagnostics()->Note(DiagMessage()
-                                       << "merging '" << compiled_file.name
-                                       << "' from compiled file " << compiled_file.source);
-    }
-
-    if (!table_merger_->MergeFile(compiled_file, override, file)) {
-      return false;
-    }
-
+  bool MergeExportedSymbols(const Source& source,
+                            const std::vector<SourcedResourceName>& exported_symbols) {
     // Add the exports of this file to the table.
-    for (const SourcedResourceName& exported_symbol : compiled_file.exported_symbols) {
+    for (const SourcedResourceName& exported_symbol : exported_symbols) {
       ResourceName res_name = exported_symbol.name;
       if (res_name.package.empty()) {
         res_name.package = context_->GetCompilationPackage();
@@ -1259,7 +1251,7 @@ class LinkCommand {
       }
 
       std::unique_ptr<Id> id = util::make_unique<Id>();
-      id->SetSource(compiled_file.source.WithLine(exported_symbol.line));
+      id->SetSource(source.WithLine(exported_symbol.line));
       bool result = final_table_.AddResourceAllowMangled(
           res_name, ConfigDescription::DefaultConfig(), std::string(), std::move(id),
           context_->GetDiagnostics());
@@ -1270,6 +1262,19 @@ class LinkCommand {
     return true;
   }
 
+  bool MergeCompiledFile(const ResourceFile& compiled_file, io::IFile* file, bool override) {
+    if (context_->IsVerbose()) {
+      context_->GetDiagnostics()->Note(DiagMessage()
+                                       << "merging '" << compiled_file.name
+                                       << "' from compiled file " << compiled_file.source);
+    }
+
+    if (!table_merger_->MergeFile(compiled_file, override, file)) {
+      return false;
+    }
+    return MergeExportedSymbols(compiled_file.source, compiled_file.exported_symbols);
+  }
+
   // Takes a path to load as a ZIP file and merges the files within into the master ResourceTable.
   // If override is true, conflicting resources are allowed to override each other, in order of last
   // seen.
@@ -1573,6 +1578,19 @@ class LinkCommand {
                                                        context_->GetPackageId()));
     }
 
+    // Extract symbols from AndroidManifest.xml, since this isn't merged like the other XML files
+    // in res/**/*.
+    {
+      XmlIdCollector collector;
+      if (!collector.Consume(context_, manifest_xml.get())) {
+        return false;
+      }
+
+      if (!MergeExportedSymbols(manifest_xml->file.source, manifest_xml->file.exported_symbols)) {
+        return false;
+      }
+    }
+
     for (const std::string& input : input_files) {
       if (!MergePath(input, false)) {
         context_->GetDiagnostics()->Error(DiagMessage() << "failed parsing input");
diff --git a/tools/aapt2/integration-tests/StaticLibTest/App/AndroidManifest.xml b/tools/aapt2/integration-tests/StaticLibTest/App/AndroidManifest.xml
index a5f202dd22fc..c8f4bda9c508 100644
--- a/tools/aapt2/integration-tests/StaticLibTest/App/AndroidManifest.xml
+++ b/tools/aapt2/integration-tests/StaticLibTest/App/AndroidManifest.xml
@@ -21,7 +21,7 @@
     <uses-permission-sdk-23 android:name="android.permission.TEST" android:maxSdkVersion="22" />
 
     <application>
-        <activity android:name=".MyActivity">
+        <activity android:name=".MyActivity" android:id="@+id/sample_generated_id">
             <layout android:defaultHeight="500dp"
                 android:defaultWidth="600dp" />
         </activity>
diff --git a/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/main.xml b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/main.xml
index aaa884bf7084..5a734296698f 100644
--- a/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/main.xml
+++ b/tools/aapt2/integration-tests/StaticLibTest/App/res/layout/main.xml
@@ -19,7 +19,7 @@
     android:layout_width="match_parent"
     android:layout_height="wrap_content">
 
-    <fragment class="android.test.sample.App$Inner" />
+    <fragment class="android.test.sample.App$Inner" android:id="@id/sample_generated_id"/>
 
     <variable name="user" type="com.android.User" />
 
-- 
cgit v1.2.3-59-g8ed1b