am 085a6b42: am 89a3c5ad: am 0cd143ad: Merge "AAPT: Fix regression in resource versioning" into mnc-dev

* commit '085a6b42f49141089c142ae5364515af15a22f62':
  AAPT: Fix regression in resource versioning

tools/aapt/Android.mk

@@ -51,7 +51,8 @@ aaptTests := \
     tests/AaptConfig_test.cpp \
     tests/AaptGroupEntry_test.cpp \
     tests/Pseudolocales_test.cpp \
-    tests/ResourceFilter_test.cpp
+    tests/ResourceFilter_test.cpp \
+    tests/ResourceTable_test.cpp
 
 aaptCIncludes := \
     system/core/base/include \

tools/aapt/ResourceTable.cpp

@@ -4466,7 +4466,8 @@ static int getMinSdkVersion(const Bundle* bundle) {
     return 0;
 }
 
-static bool shouldGenerateVersionedResource(const sp<ResourceTable::ConfigList>& configList,
+bool ResourceTable::shouldGenerateVersionedResource(
+        const sp<ResourceTable::ConfigList>& configList,
         const ConfigDescription& sourceConfig,
         const int sdkVersionToGenerate) {
     assert(sdkVersionToGenerate > sourceConfig.sdkVersion);
@@ -4477,24 +4478,24 @@ static bool shouldGenerateVersionedResource(const sp<ResourceTable::ConfigList>&
     // The source config came from this list, so it should be here.
     assert(idx >= 0);
 
-    idx += 1;
-    if (static_cast<size_t>(idx) >= entries.size()) {
-        // This is the last configuration, so we should generate a versioned resource.
-        return true;
-    }
+    // The next configuration either only varies in sdkVersion, or it is completely different
+    // and therefore incompatible. If it is incompatible, we must generate the versioned resource.
 
-    const ConfigDescription& nextConfig = entries.keyAt(idx);
-
-    // Build a configuration that is the same as the source config,
-    // but with the SDK level of the next config. If they are the same,
-    // then they only differ in SDK level. If the next configs SDK level is
-    // higher than the one we want to generate, we must generate it.
+    // NOTE: The ordering of configurations takes sdkVersion as higher precedence than other
+    // qualifiers, so we need to iterate through the entire list to be sure there
+    // are no higher sdk level versions of this resource.
     ConfigDescription tempConfig(sourceConfig);
+    for (size_t i = static_cast<size_t>(idx) + 1; i < entries.size(); i++) {
+        const ConfigDescription& nextConfig = entries.keyAt(i);
         tempConfig.sdkVersion = nextConfig.sdkVersion;
-    if (nextConfig == tempConfig) {
+        if (tempConfig == nextConfig) {
+            // The two configs are the same, check the sdk version.
             return sdkVersionToGenerate < nextConfig.sdkVersion;
         }
-    return false;
+    }
+
+    // No match was found, so we should generate the versioned resource.
+    return true;
 }
 
 /**

tools/aapt/ResourceTable.h

@@ -99,6 +99,15 @@ public:
     class Package;
     class Type;
     class Entry;
+    class ConfigList;
+
+    /**
+     * Exposed for testing. Determines whether a versioned resource should be generated
+     * based on the other available configurations for that resource.
+     */
+    static bool shouldGenerateVersionedResource(const sp<ConfigList>& configList,
+                                                const ConfigDescription& sourceConfig,
+                                                const int sdkVersionToGenerate);
 
     ResourceTable(Bundle* bundle, const String16& assetsPackage, PackageType type);
 

tools/aapt/tests/ResourceTable_test.cpp

@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <utils/String8.h>
+#include <gtest/gtest.h>
+
+#include "ConfigDescription.h"
+#include "ResourceTable.h"
+#include "TestHelper.h"
+
+using android::String16;
+
+TEST(ResourceTableTest, generateVersionedResources) {
+    sp<ResourceTable::ConfigList> configs(new ResourceTable::ConfigList(String16(), SourcePos()));
+
+    ConfigDescription defaultConfig = {};
+
+    ConfigDescription landConfig = {};
+    landConfig.orientation = ResTable_config::ORIENTATION_LAND;
+
+    ConfigDescription sw600dpLandConfig = {};
+    sw600dpLandConfig.orientation = ResTable_config::ORIENTATION_LAND;
+    sw600dpLandConfig.smallestScreenWidthDp = 600;
+
+    configs->addEntry(defaultConfig, new ResourceTable::Entry(String16(), SourcePos()));
+    configs->addEntry(landConfig, new ResourceTable::Entry(String16(), SourcePos()));
+    configs->addEntry(sw600dpLandConfig, new ResourceTable::Entry(String16(), SourcePos()));
+
+    EXPECT_TRUE(ResourceTable::shouldGenerateVersionedResource(configs, defaultConfig, 17));
+    EXPECT_TRUE(ResourceTable::shouldGenerateVersionedResource(configs, landConfig, 17));
+}
+
+TEST(ResourceTableTest, generateVersionedResourceWhenHigherVersionExists) {
+    sp<ResourceTable::ConfigList> configs(new ResourceTable::ConfigList(String16(), SourcePos()));
+
+    ConfigDescription defaultConfig = {};
+
+    ConfigDescription v21Config = {};
+    v21Config.sdkVersion = 21;
+
+    ConfigDescription sw600dpV13Config = {};
+    sw600dpV13Config.smallestScreenWidthDp = 600;
+    sw600dpV13Config.sdkVersion = 13;
+
+    configs->addEntry(defaultConfig, new ResourceTable::Entry(String16(), SourcePos()));
+    configs->addEntry(v21Config, new ResourceTable::Entry(String16(), SourcePos()));
+    configs->addEntry(sw600dpV13Config, new ResourceTable::Entry(String16(), SourcePos()));
+
+    EXPECT_TRUE(ResourceTable::shouldGenerateVersionedResource(configs, defaultConfig, 17));
+    EXPECT_FALSE(ResourceTable::shouldGenerateVersionedResource(configs, defaultConfig, 22));
+}