From a5cc002bfe7ca8a6536549189b44e7143c8611dc Mon Sep 17 00:00:00 2001
From: Adam Lesinski <adamlesinski@google.com>
Date: Fri, 22 Aug 2014 14:10:31 -0700
Subject: [PATCH] AAPT: Filtering resource fix

Previously, when filtering resources from an APK using
-c option, if one qualifier matched, we would keep the resource.
However, in the case of something like

-c fr-FR,sw360dp

and with a resource in the APK like so

drawable-fr-FR-sw600dp-v13

we would want this resource to be excluded, as it does not
match the sw360dp qualifier (must be less than or equal to it).

This CL fixed the behavior of the filter to require that all
defined qualifier axis be matched.

Bug:17142358
Change-Id: Ie48f3d516a0e610abc7ba8a7ced4eb3ab52534d4
---
 tools/aapt/ResourceFilter.cpp            | 18 +++++++++---
 tools/aapt/ResourceFilter.h              |  3 +-
 tools/aapt/tests/ResourceFilter_test.cpp | 37 ++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/tools/aapt/ResourceFilter.cpp b/tools/aapt/ResourceFilter.cpp
index de8b4fccb369a..fc95e148ea416 100644
--- a/tools/aapt/ResourceFilter.cpp
+++ b/tools/aapt/ResourceFilter.cpp
@@ -56,24 +56,34 @@ WeakResourceFilter::match(const ResTable_config& config) const
         return true;
     }
 
+    uint32_t matchedAxis = 0x0;
     const size_t N = mConfigs.size();
     for (size_t i = 0; i < N; i++) {
         const std::pair<ConfigDescription, uint32_t>& entry = mConfigs[i];
         uint32_t diff = entry.first.diff(config);
         if ((diff & entry.second) == 0) {
-            return true;
+            // Mark the axis that was matched.
+            matchedAxis |= entry.second;
         } else if ((diff & entry.second) == ResTable_config::CONFIG_LOCALE) {
             // If the locales differ, but the languages are the same and
             // the locale we are matching only has a language specified,
             // we match.
-            if (config.language[0] && memcmp(config.language, entry.first.language, sizeof(config.language)) == 0) {
+            if (config.language[0] &&
+                    memcmp(config.language, entry.first.language, sizeof(config.language)) == 0) {
                 if (config.country[0] == 0) {
-                    return true;
+                    matchedAxis |= ResTable_config::CONFIG_LOCALE;
                 }
             }
+        } else if ((diff & entry.second) == ResTable_config::CONFIG_SMALLEST_SCREEN_SIZE) {
+            // Special case if the smallest screen width doesn't match. We check that the
+            // config being matched has a smaller screen width than the filter specified.
+            if (config.smallestScreenWidthDp != 0 &&
+                    config.smallestScreenWidthDp < entry.first.smallestScreenWidthDp) {
+                matchedAxis |= ResTable_config::CONFIG_SMALLEST_SCREEN_SIZE;
+            }
         }
     }
-    return false;
+    return matchedAxis == (mConfigMask & mask);
 }
 
 status_t
diff --git a/tools/aapt/ResourceFilter.h b/tools/aapt/ResourceFilter.h
index f459584a1af99..d6430c0409ccf 100644
--- a/tools/aapt/ResourceFilter.h
+++ b/tools/aapt/ResourceFilter.h
@@ -24,8 +24,7 @@ public:
 };
 
 /**
- * Implements logic for parsing and handling "-c" and "--preferred-configurations"
- * options.
+ * Implements logic for parsing and handling "-c" options.
  */
 class WeakResourceFilter : public ResourceFilter {
 public:
diff --git a/tools/aapt/tests/ResourceFilter_test.cpp b/tools/aapt/tests/ResourceFilter_test.cpp
index b55379ebf05d3..bbc2e02a4de2e 100644
--- a/tools/aapt/tests/ResourceFilter_test.cpp
+++ b/tools/aapt/tests/ResourceFilter_test.cpp
@@ -75,6 +75,17 @@ TEST(WeakResourceFilterTest, MatchesConfigWithSameValueAxisAndOtherUnrelatedAxis
     EXPECT_TRUE(filter.match(config));
 }
 
+TEST(WeakResourceFilterTest, MatchesConfigWithOneMatchingAxis) {
+    WeakResourceFilter filter;
+    ASSERT_EQ(NO_ERROR, filter.parse(String8("fr_FR,sw360dp,normal,en_US")));
+
+    ConfigDescription config;
+    config.language[0] = 'e';
+    config.language[1] = 'n';
+
+    EXPECT_TRUE(filter.match(config));
+}
+
 TEST(WeakResourceFilterTest, DoesNotMatchConfigWithDifferentValueAxis) {
     WeakResourceFilter filter;
     ASSERT_EQ(NO_ERROR, filter.parse(String8("fr")));
@@ -86,6 +97,32 @@ TEST(WeakResourceFilterTest, DoesNotMatchConfigWithDifferentValueAxis) {
     EXPECT_FALSE(filter.match(config));
 }
 
+TEST(WeakResourceFilterTest, DoesNotMatchWhenOneQualifierIsExplicitlyNotMatched) {
+    WeakResourceFilter filter;
+    ASSERT_EQ(NO_ERROR, filter.parse(String8("fr_FR,en_US,normal,large,xxhdpi,sw320dp")));
+
+    ConfigDescription config;
+    config.language[0] = 'f';
+    config.language[1] = 'r';
+    config.smallestScreenWidthDp = 600;
+    config.version = 13;
+
+    EXPECT_FALSE(filter.match(config));
+}
+
+TEST(WeakResourceFilterTest, MatchesSmallestWidthWhenSmaller) {
+    WeakResourceFilter filter;
+    ASSERT_EQ(NO_ERROR, filter.parse(String8("sw600dp")));
+
+    ConfigDescription config;
+    config.language[0] = 'f';
+    config.language[1] = 'r';
+    config.smallestScreenWidthDp = 320;
+    config.version = 13;
+
+    EXPECT_TRUE(filter.match(config));
+}
+
 TEST(WeakResourceFilterTest, MatchesConfigWithSameLanguageButNoRegionSpecified) {
     WeakResourceFilter filter;
     ASSERT_EQ(NO_ERROR, filter.parse(String8("de-rDE")));