Merge changes I446fb6ab,Ife3bbd29 into oc-dev

* changes:
  Valid resource IDs may be negative
  AAPT2: Workaround for findViewById with package ID > 0x7f

core/java/android/app/AlertDialog.java

@@ -215,7 +215,8 @@ public class AlertDialog extends Dialog implements DialogInterface {
             return R.style.Theme_DeviceDefault_Dialog_Alert;
         } else if (themeResId == THEME_DEVICE_DEFAULT_LIGHT) {
             return R.style.Theme_DeviceDefault_Light_Dialog_Alert;
-        } else if (themeResId >= 0x01000000) {   // start of real resource IDs.
+        } else if (Integer.compareUnsigned(themeResId, 0x01000000) >= 0) {
+            // start of real resource IDs.
             return themeResId;
         } else {
             final TypedValue outValue = new TypedValue();

core/java/android/appwidget/AppWidgetProviderInfo.java

@@ -368,11 +368,11 @@ public class AppWidgetProviderInfo implements Parcelable {
         try {
             Resources resources = context.getPackageManager().getResourcesForApplication(
                     providerInfo.applicationInfo);
-            if (resourceId > 0) {
-                if (density <= 0) {
-                    density = context.getResources().getDisplayMetrics().densityDpi;
+            if (resourceId != 0) {
+                if (density < 0) {
+                    density = 0;
                 }
-                return resources.getDrawableForDensity(resourceId, density);
+                return resources.getDrawableForDensity(resourceId, density, null);
             }
         } catch (PackageManager.NameNotFoundException | Resources.NotFoundException e) {
             /* ignore */

tools/aapt2/Main.cpp

@@ -27,7 +27,7 @@ namespace aapt {
 static const char* sMajorVersion = "2";
 
 // Update minor version whenever a feature or flag is added.
-static const char* sMinorVersion = "13";
+static const char* sMinorVersion = "14";
 
 int PrintVersion() {
   std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "."

tools/aapt2/cmd/Link.cpp

@@ -191,6 +191,62 @@ class LinkContext : public IAaptContext {
   int min_sdk_version_ = 0;
 };
 
+// A custom delegate that generates compatible pre-O IDs for use with feature splits.
+// Feature splits use package IDs > 7f, which in Java (since Java doesn't have unsigned ints)
+// is interpreted as a negative number. Some verification was wrongly assuming negative values
+// were invalid.
+//
+// This delegate will attempt to masquerade any '@id/' references with ID 0xPPTTEEEE,
+// where PP > 7f, as 0x7fPPEEEE. Any potential overlapping is verified and an error occurs if such
+// an overlap exists.
+class FeatureSplitSymbolTableDelegate : public DefaultSymbolTableDelegate {
+ public:
+  FeatureSplitSymbolTableDelegate(IAaptContext* context) : context_(context) {
+  }
+
+  virtual ~FeatureSplitSymbolTableDelegate() = default;
+
+  virtual std::unique_ptr<SymbolTable::Symbol> FindByName(
+      const ResourceName& name,
+      const std::vector<std::unique_ptr<ISymbolSource>>& sources) override {
+    std::unique_ptr<SymbolTable::Symbol> symbol =
+        DefaultSymbolTableDelegate::FindByName(name, sources);
+    if (symbol == nullptr) {
+      return {};
+    }
+
+    // Check to see if this is an 'id' with the target package.
+    if (name.type == ResourceType::kId && symbol->id) {
+      ResourceId* id = &symbol->id.value();
+      if (id->package_id() > kAppPackageId) {
+        // Rewrite the resource ID to be compatible pre-O.
+        ResourceId rewritten_id(kAppPackageId, id->package_id(), id->entry_id());
+
+        // Check that this doesn't overlap another resource.
+        if (DefaultSymbolTableDelegate::FindById(rewritten_id, sources) != nullptr) {
+          // The ID overlaps, so log a message (since this is a weird failure) and fail.
+          context_->GetDiagnostics()->Error(DiagMessage() << "Failed to rewrite " << name
+                                                          << " for pre-O feature split support");
+          return {};
+        }
+
+        if (context_->IsVerbose()) {
+          context_->GetDiagnostics()->Note(DiagMessage() << "rewriting " << name << " (" << *id
+                                                         << ") -> (" << rewritten_id << ")");
+        }
+
+        *id = rewritten_id;
+      }
+    }
+    return symbol;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FeatureSplitSymbolTableDelegate);
+
+  IAaptContext* context_;
+};
+
 static bool FlattenXml(xml::XmlResource* xml_res, const StringPiece& path,
                        Maybe<size_t> max_sdk_level, bool keep_raw_values, IArchiveWriter* writer,
                        IAaptContext* context) {
@@ -1464,6 +1520,19 @@ class LinkCommand {
     context_->GetExternalSymbols()->PrependSource(
         util::make_unique<ResourceTableSymbolSource>(&final_table_));
 
+    // Workaround for pre-O runtime that would treat negative resource IDs
+    // (any ID with a package ID > 7f) as invalid. Intercept any ID (PPTTEEEE) with PP > 0x7f
+    // and type == 'id', and return the ID 0x7fPPEEEE. IDs don't need to be real resources, they
+    // are just identifiers.
+    if (context_->GetMinSdkVersion() < SDK_O && context_->GetPackageType() == PackageType::kApp) {
+      if (context_->IsVerbose()) {
+        context_->GetDiagnostics()->Note(DiagMessage()
+                                         << "enabling pre-O feature split ID rewriting");
+      }
+      context_->GetExternalSymbols()->SetDelegate(
+          util::make_unique<FeatureSplitSymbolTableDelegate>(context_));
+    }
+
     ReferenceLinker linker;
     if (!linker.Consume(context_, &final_table_)) {
       context_->GetDiagnostics()->Error(DiagMessage() << "failed linking references");

tools/aapt2/java/JavaClassGenerator.cpp

@@ -31,6 +31,7 @@
 #include "Resource.h"
 #include "ResourceTable.h"
 #include "ResourceValues.h"
+#include "SdkConstants.h"
 #include "ValueVisitor.h"
 #include "java/AnnotationProcessor.h"
 #include "java/ClassDefinition.h"
@@ -430,9 +431,15 @@ void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const Reso
                                          const ResourceEntry& entry, ClassDefinition* out_class_def,
                                          MethodDefinition* out_rewrite_method,
                                          std::ostream* out_r_txt) {
+  ResourceId real_id = id;
+  if (context_->GetMinSdkVersion() < SDK_O && name.type == ResourceType::kId &&
+      id.package_id() > kAppPackageId) {
+    real_id = ResourceId(kAppPackageId, id.package_id(), id.entry_id());
+  }
+
   const std::string field_name = TransformToFieldName(name.entry);
   std::unique_ptr<ResourceMember> resource_member =
-      util::make_unique<ResourceMember>(field_name, id);
+      util::make_unique<ResourceMember>(field_name, real_id);
 
   // Build the comments and annotations for this entry.
   AnnotationProcessor* processor = resource_member->GetCommentBuilder();
@@ -458,7 +465,7 @@ void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const Reso
   out_class_def->AddMember(std::move(resource_member));
 
   if (out_r_txt != nullptr) {
-    *out_r_txt << "int " << name.type << " " << field_name << " " << id << "\n";
+    *out_r_txt << "int " << name.type << " " << field_name << " " << real_id << "\n";
   }
 
   if (out_rewrite_method != nullptr) {

tools/aapt2/process/SymbolTable.cpp

@@ -34,6 +34,21 @@ using android::StringPiece;
 
 namespace aapt {
 
+SymbolTable::SymbolTable(NameMangler* mangler)
+    : mangler_(mangler),
+      delegate_(util::make_unique<DefaultSymbolTableDelegate>()),
+      cache_(200),
+      id_cache_(200) {
+}
+
+void SymbolTable::SetDelegate(std::unique_ptr<ISymbolTableDelegate> delegate) {
+  CHECK(delegate != nullptr) << "can't set a nullptr delegate";
+  delegate_ = std::move(delegate);
+
+  // Clear the cache in case this delegate changes the order of lookup.
+  cache_.clear();
+}
+
 void SymbolTable::AppendSource(std::unique_ptr<ISymbolSource> source) {
   sources_.push_back(std::move(source));
 
@@ -75,28 +90,27 @@ const SymbolTable::Symbol* SymbolTable::FindByName(const ResourceName& name) {
     mangled_name = &mangled_name_impl.value();
   }
 
-  for (auto& symbolSource : sources_) {
-    std::unique_ptr<Symbol> symbol = symbolSource->FindByName(*mangled_name);
-    if (symbol) {
-      // Take ownership of the symbol into a shared_ptr. We do this because
-      // LruCache doesn't support unique_ptr.
-      std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
-
-      // Since we look in the cache with the unmangled, but package prefixed
-      // name, we must put the same name into the cache.
-      cache_.put(*name_with_package, shared_symbol);
-
-      if (shared_symbol->id) {
-        // The symbol has an ID, so we can also cache this!
-        id_cache_.put(shared_symbol->id.value(), shared_symbol);
-      }
-
-      // Returns the raw pointer. Callers are not expected to hold on to this
-      // between calls to Find*.
-      return shared_symbol.get();
-    }
+  std::unique_ptr<Symbol> symbol = delegate_->FindByName(*mangled_name, sources_);
+  if (symbol == nullptr) {
+    return nullptr;
   }
-  return nullptr;
+
+  // Take ownership of the symbol into a shared_ptr. We do this because
+  // LruCache doesn't support unique_ptr.
+  std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
+
+  // Since we look in the cache with the unmangled, but package prefixed
+  // name, we must put the same name into the cache.
+  cache_.put(*name_with_package, shared_symbol);
+
+  if (shared_symbol->id) {
+    // The symbol has an ID, so we can also cache this!
+    id_cache_.put(shared_symbol->id.value(), shared_symbol);
+  }
+
+  // Returns the raw pointer. Callers are not expected to hold on to this
+  // between calls to Find*.
+  return shared_symbol.get();
 }
 
 const SymbolTable::Symbol* SymbolTable::FindById(const ResourceId& id) {
@@ -105,20 +119,19 @@ const SymbolTable::Symbol* SymbolTable::FindById(const ResourceId& id) {
   }
 
   // We did not find it in the cache, so look through the sources.
-  for (auto& symbolSource : sources_) {
-    std::unique_ptr<Symbol> symbol = symbolSource->FindById(id);
-    if (symbol) {
-      // Take ownership of the symbol into a shared_ptr. We do this because LruCache
-      // doesn't support unique_ptr.
-      std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
-      id_cache_.put(id, shared_symbol);
-
-      // Returns the raw pointer. Callers are not expected to hold on to this
-      // between calls to Find*.
-      return shared_symbol.get();
-    }
+  std::unique_ptr<Symbol> symbol = delegate_->FindById(id, sources_);
+  if (symbol == nullptr) {
+    return nullptr;
   }
-  return nullptr;
+
+  // Take ownership of the symbol into a shared_ptr. We do this because LruCache
+  // doesn't support unique_ptr.
+  std::shared_ptr<Symbol> shared_symbol(std::move(symbol));
+  id_cache_.put(id, shared_symbol);
+
+  // Returns the raw pointer. Callers are not expected to hold on to this
+  // between calls to Find*.
+  return shared_symbol.get();
 }
 
 const SymbolTable::Symbol* SymbolTable::FindByReference(const Reference& ref) {
@@ -140,6 +153,28 @@ const SymbolTable::Symbol* SymbolTable::FindByReference(const Reference& ref) {
   return symbol;
 }
 
+std::unique_ptr<SymbolTable::Symbol> DefaultSymbolTableDelegate::FindByName(
+    const ResourceName& name, const std::vector<std::unique_ptr<ISymbolSource>>& sources) {
+  for (auto& source : sources) {
+    std::unique_ptr<SymbolTable::Symbol> symbol = source->FindByName(name);
+    if (symbol) {
+      return symbol;
+    }
+  }
+  return {};
+}
+
+std::unique_ptr<SymbolTable::Symbol> DefaultSymbolTableDelegate::FindById(
+    ResourceId id, const std::vector<std::unique_ptr<ISymbolSource>>& sources) {
+  for (auto& source : sources) {
+    std::unique_ptr<SymbolTable::Symbol> symbol = source->FindById(id);
+    if (symbol) {
+      return symbol;
+    }
+  }
+  return {};
+}
+
 std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName(
     const ResourceName& name) {
   Maybe<ResourceTable::SearchResult> result = table_->FindResource(name);

tools/aapt2/process/SymbolTable.h

@@ -47,6 +47,7 @@ inline android::hash_t hash_type(const ResourceId& id) {
 }
 
 class ISymbolSource;
+class ISymbolTableDelegate;
 class NameMangler;
 
 class SymbolTable {
@@ -73,7 +74,11 @@ class SymbolTable {
     bool is_public = false;
   };
 
-  SymbolTable(NameMangler* mangler) : mangler_(mangler), cache_(200), id_cache_(200) {}
+  SymbolTable(NameMangler* mangler);
+
+  // Overrides the default ISymbolTableDelegate, which allows a custom defined strategy for
+  // looking up resources from a set of sources.
+  void SetDelegate(std::unique_ptr<ISymbolTableDelegate> delegate);
 
   // Appends a symbol source. The cache is not cleared since entries that
   // have already been found would take precedence due to ordering.
@@ -99,6 +104,7 @@ class SymbolTable {
 
  private:
   NameMangler* mangler_;
+  std::unique_ptr<ISymbolTableDelegate> delegate_;
   std::vector<std::unique_ptr<ISymbolSource>> sources_;
 
   // We use shared_ptr because unique_ptr is not supported and
@@ -109,11 +115,41 @@ class SymbolTable {
   DISALLOW_COPY_AND_ASSIGN(SymbolTable);
 };
 
-/**
- * An interface that a symbol source implements in order to surface symbol
- * information
- * to the symbol table.
- */
+// Allows the customization of the lookup strategy/order of a symbol from a set of
+// symbol sources.
+class ISymbolTableDelegate {
+ public:
+  ISymbolTableDelegate() = default;
+  virtual ~ISymbolTableDelegate() = default;
+
+  // The name is already mangled and does not need further processing.
+  virtual std::unique_ptr<SymbolTable::Symbol> FindByName(
+      const ResourceName& name, const std::vector<std::unique_ptr<ISymbolSource>>& sources) = 0;
+
+  virtual std::unique_ptr<SymbolTable::Symbol> FindById(
+      ResourceId id, const std::vector<std::unique_ptr<ISymbolSource>>& sources) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ISymbolTableDelegate);
+};
+
+class DefaultSymbolTableDelegate : public ISymbolTableDelegate {
+ public:
+  DefaultSymbolTableDelegate() = default;
+  virtual ~DefaultSymbolTableDelegate() = default;
+
+  virtual std::unique_ptr<SymbolTable::Symbol> FindByName(
+      const ResourceName& name,
+      const std::vector<std::unique_ptr<ISymbolSource>>& sources) override;
+  virtual std::unique_ptr<SymbolTable::Symbol> FindById(
+      ResourceId id, const std::vector<std::unique_ptr<ISymbolSource>>& sources) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DefaultSymbolTableDelegate);
+};
+
+// An interface that a symbol source implements in order to surface symbol information
+// to the symbol table.
 class ISymbolSource {
  public:
   virtual ~ISymbolSource() = default;
@@ -122,9 +158,7 @@ class ISymbolSource {
       const ResourceName& name) = 0;
   virtual std::unique_ptr<SymbolTable::Symbol> FindById(ResourceId id) = 0;
 
-  /**
-   * Default implementation tries the name if it exists, else the ID.
-   */
+  // Default implementation tries the name if it exists, else the ID.
   virtual std::unique_ptr<SymbolTable::Symbol> FindByReference(
       const Reference& ref) {
     if (ref.name) {
@@ -136,11 +170,9 @@ class ISymbolSource {
   }
 };
 
-/**
- * Exposes the resources in a ResourceTable as symbols for SymbolTable.
- * Instances of this class must outlive the encompassed ResourceTable.
- * Lookups by ID are ignored.
- */
+// Exposes the resources in a ResourceTable as symbols for SymbolTable.
+// Instances of this class must outlive the encompassed ResourceTable.
+// Lookups by ID are ignored.
 class ResourceTableSymbolSource : public ISymbolSource {
  public:
   explicit ResourceTableSymbolSource(ResourceTable* table) : table_(table) {}

tools/aapt2/readme.md

@@ -1,5 +1,19 @@
 # Android Asset Packaging Tool 2.0 (AAPT2) release notes
 
+## Version 2.14
+### `aapt2 link ...`
+- If an app is building with a minSdkVersion < 26 and a --package-id XX where XX > 7F, aapt2
+  will automatically convert any 'id' resource references from the resource ID 0xPPTTEEEE to
+  0x7FPPEEEE.
+- This is done to workaround a bug in previous versions of the platform that would validate
+  a resource ID by assuming it is larger than 0. In Java, a resource ID with package ID greater
+  than 0x7F is interpreted as a negative number, causing valid feature split IDs like 0x80010000
+  to fail the check.
+- '@id/foo' resources are just sentinel values and do not actually need to resolve to anything.
+  Rewriting these resource IDs to use the package ID 7F while maintaining their definitions under
+  the original package ID is safe. Collisions against the base APK are checked to ensure these
+  rewritten IDs to not overlap with the base.
+
 ## Version 2.13
 ### `aapt2 optimize ...`
 - aapt2 optimize can now split a binary APK with the same --split parameters as the link