Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1f4497c9 authored by Adam Lesinski's avatar Adam Lesinski Committed by Android (Google) Code Review
Browse files

Merge changes I8c710af6,Iedf30212 

* changes:
  libandroidfw: Fix mass logspam of ResourceTypes warnings
  NativeActivity JNI: Retain VM reference to AssetManager
parents 60ec4506 ed69ce84

core/jni/android_app_NativeActivity.cpp

+11 −1
Original line number Diff line number Diff line
@@ -128,9 +128,14 @@ struct NativeCode : public ANativeActivity { 
        if (callbacks.onDestroy != NULL) {

            callbacks.onDestroy(this);

        }

        if (env != NULL && clazz != NULL) {

        if (env != NULL) {

          if (clazz != NULL) {

            env->DeleteGlobalRef(clazz);

          }

          if (javaAssetManager != NULL) {

            env->DeleteGlobalRef(javaAssetManager);

          }

        }

        if (messageQueue != NULL && mainWorkRead >= 0) {

            messageQueue->getLooper()->removeFd(mainWorkRead);

        }
@@ -170,6 +175,10 @@ struct NativeCode : public ANativeActivity { 
    int mainWorkRead;

    int mainWorkWrite;

    sp<MessageQueue> messageQueue;



    // Need to hold on to a reference here in case the upper layers destroy our

    // AssetManager.

    jobject javaAssetManager;

};



void android_NativeActivity_finish(ANativeActivity* activity) {
@@ -345,6 +354,7 @@ loadNativeCode_native(JNIEnv* env, jobject clazz, jstring path, jstring funcName 


    code->sdkVersion = sdkVersion;



    code->javaAssetManager = env->NewGlobalRef(jAssetMgr);

    code->assetManager = assetManagerForJavaObject(env, jAssetMgr);



    if (obbDir != NULL) {

libs/androidfw/.clang-format

+5 −0
Original line number Diff line number Diff line 
BasedOnStyle: Google

ColumnLimit: 100

AllowShortBlocksOnASingleLine: false

AllowShortFunctionsOnASingleLine: false

CommentPragmas: NOLINT:.*

DerivePointerAlignment: false

PointerAlignment: Left

libs/androidfw/ResourceTypes.cpp

+52 −33
Original line number Diff line number Diff line
@@ -6431,17 +6431,26 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
            }



            if (newEntryCount > 0) {

                bool addToType = true;

                uint8_t typeIndex = typeSpec->id - 1;

                ssize_t idmapIndex = idmapEntries.indexOfKey(typeSpec->id);

                if (idmapIndex >= 0) {

                    typeIndex = idmapEntries[idmapIndex].targetTypeId() - 1;

                } else if (header->resourceIDMap != NULL) {

                    // This is an overlay, but the types in this overlay are not

                    // overlaying anything according to the idmap. We can skip these

                    // as they will otherwise conflict with the other resources in the package

                    // without a mapping.

                    addToType = false;

                }



                if (addToType) {

                    TypeList& typeList = group->types.editItemAt(typeIndex);

                    if (!typeList.isEmpty()) {

                        const Type* existingType = typeList[0];

                        if (existingType->entryCount != newEntryCount && idmapIndex < 0) {

                        ALOGW("ResTable_typeSpec entry count inconsistent: given %d, previously %d",

                            ALOGW("ResTable_typeSpec entry count inconsistent: "

                                  "given %d, previously %d",

                                  (int) newEntryCount, (int) existingType->entryCount);

                            // We should normally abort here, but some legacy apps declare

                            // resources in the 'android' package (old bug in AAPT).
@@ -6457,6 +6466,7 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
                    }

                    typeList.add(t);

                    group->largestTypeId = max(group->largestTypeId, typeSpec->id);

                }

            } else {

                ALOGV("Skipping empty ResTable_typeSpec for type %d", typeSpec->id);

            }
@@ -6499,12 +6509,20 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
            }



            if (newEntryCount > 0) {

                bool addToType = true;

                uint8_t typeIndex = type->id - 1;

                ssize_t idmapIndex = idmapEntries.indexOfKey(type->id);

                if (idmapIndex >= 0) {

                    typeIndex = idmapEntries[idmapIndex].targetTypeId() - 1;

                } else if (header->resourceIDMap != NULL) {

                    // This is an overlay, but the types in this overlay are not

                    // overlaying anything according to the idmap. We can skip these

                    // as they will otherwise conflict with the other resources in the package

                    // without a mapping.

                    addToType = false;

                }



                if (addToType) {

                    TypeList& typeList = group->types.editItemAt(typeIndex);

                    if (typeList.isEmpty()) {

                        ALOGE("No TypeSpec for type %d", type->id);
@@ -6525,6 +6543,7 @@ status_t ResTable::parsePackage(const ResTable_package* const pkg, 
                        ALOGI("Adding config to type %d: %s\n", type->id,

                                thisConfig.toString().string());

                    }

                }

            } else {

                ALOGV("Skipping empty ResTable_type for type %d", type->id);

            }

libs/androidfw/tests/Idmap_test.cpp

+56 −44
Original line number Diff line number Diff line
@@ -30,25 +30,23 @@ class IdmapTest : public ::testing::Test { 
 protected:

  void SetUp() override {

    std::string contents;

    ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/basic/basic.apk",

                                        "resources.arsc", &contents));

    ASSERT_EQ(NO_ERROR,

              target_table_.add(contents.data(), contents.size(), 0, true));

    ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/basic/basic.apk", "resources.arsc",

                                        &contents));

    ASSERT_EQ(NO_ERROR, target_table_.add(contents.data(), contents.size(), 0, true));



    ASSERT_TRUE(

        ReadFileFromZipToString(GetTestDataPath() + "/overlay/overlay.apk",

    ASSERT_TRUE(ReadFileFromZipToString(GetTestDataPath() + "/overlay/overlay.apk",

                                        "resources.arsc", &overlay_data_));

    ResTable overlay_table;

    ASSERT_EQ(NO_ERROR,

              overlay_table.add(overlay_data_.data(), overlay_data_.size()));

    ASSERT_EQ(NO_ERROR, overlay_table.add(overlay_data_.data(), overlay_data_.size()));



    char target_name[256] = "com.android.basic";

    ASSERT_EQ(NO_ERROR,

              target_table_.createIdmap(overlay_table, 0, 0, target_name,

                                        target_name, &data_, &data_size_));

    ASSERT_EQ(NO_ERROR, target_table_.createIdmap(overlay_table, 0, 0, target_name, target_name,

                                                  &data_, &data_size_));

  }



  void TearDown() override { ::free(data_); }

  void TearDown() override {

    ::free(data_);

  }



  ResTable target_table_;

  std::string overlay_data_;
@@ -56,13 +54,12 @@ class IdmapTest : public ::testing::Test { 
  size_t data_size_ = 0;

};



TEST_F(IdmapTest, canLoadIdmap) {

TEST_F(IdmapTest, CanLoadIdmap) {

  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_,

                              data_size_));

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));

}



TEST_F(IdmapTest, overlayOverridesResourceValue) {

TEST_F(IdmapTest, OverlayOverridesResourceValue) {

  Res_value val;

  ssize_t block = target_table_.getResource(R::string::test2, &val, false);

  ASSERT_GE(block, 0);
@@ -71,45 +68,60 @@ TEST_F(IdmapTest, overlayOverridesResourceValue) { 
  ASSERT_TRUE(pool != NULL);

  ASSERT_LT(val.data, pool->size());



  size_t strLen;

  const char16_t* targetStr16 = pool->stringAt(val.data, &strLen);

  ASSERT_TRUE(targetStr16 != NULL);

  ASSERT_EQ(String16("test2"), String16(targetStr16, strLen));

  size_t str_len;

  const char16_t* target_str16 = pool->stringAt(val.data, &str_len);

  ASSERT_TRUE(target_str16 != NULL);

  ASSERT_EQ(String16("test2"), String16(target_str16, str_len));



  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_,

                              data_size_));

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));



  ssize_t newBlock = target_table_.getResource(R::string::test2, &val, false);

  ASSERT_GE(newBlock, 0);

  ASSERT_NE(block, newBlock);

  ssize_t new_block = target_table_.getResource(R::string::test2, &val, false);

  ASSERT_GE(new_block, 0);

  ASSERT_NE(block, new_block);

  ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);

  pool = target_table_.getTableStringBlock(newBlock);

  pool = target_table_.getTableStringBlock(new_block);

  ASSERT_TRUE(pool != NULL);

  ASSERT_LT(val.data, pool->size());



  targetStr16 = pool->stringAt(val.data, &strLen);

  ASSERT_TRUE(targetStr16 != NULL);

  ASSERT_EQ(String16("test2-overlay"), String16(targetStr16, strLen));

  target_str16 = pool->stringAt(val.data, &str_len);

  ASSERT_TRUE(target_str16 != NULL);

  ASSERT_EQ(String16("test2-overlay"), String16(target_str16, str_len));

}



TEST_F(IdmapTest, overlaidResourceHasSameName) {

TEST_F(IdmapTest, OverlaidResourceHasSameName) {

  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_,

                              data_size_));

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));



  ResTable::resource_name res_name;

  ASSERT_TRUE(target_table_.getResourceName(R::array::integerArray1, false, &res_name));



  ASSERT_TRUE(res_name.package != NULL);

  ASSERT_TRUE(res_name.type != NULL);

  ASSERT_TRUE(res_name.name != NULL);



  ResTable::resource_name resName;

  ASSERT_TRUE(

      target_table_.getResourceName(R::array::integerArray1, false, &resName));

  EXPECT_EQ(String16("com.android.basic"), String16(res_name.package, res_name.packageLen));

  EXPECT_EQ(String16("array"), String16(res_name.type, res_name.typeLen));

  EXPECT_EQ(String16("integerArray1"), String16(res_name.name, res_name.nameLen));

}



constexpr const uint32_t kNonOverlaidResourceId = 0x7fff0000u;



TEST_F(IdmapTest, OverlayDoesNotIncludeNonOverlaidResources) {

  // First check that the resource we're trying to not include when overlaid is present when

  // the overlay is loaded as a standalone APK.

  ResTable table;

  ASSERT_EQ(NO_ERROR, table.add(overlay_data_.data(), overlay_data_.size(), 0, true));



  ASSERT_TRUE(resName.package != NULL);

  ASSERT_TRUE(resName.type != NULL);

  ASSERT_TRUE(resName.name != NULL);

  Res_value val;

  ssize_t block = table.getResource(kNonOverlaidResourceId, &val, false /*mayBeBag*/);

  ASSERT_GE(block, 0);



  EXPECT_EQ(String16("com.android.basic"),

            String16(resName.package, resName.packageLen));

  EXPECT_EQ(String16("array"), String16(resName.type, resName.typeLen));

  EXPECT_EQ(String16("integerArray1"), String16(resName.name, resName.nameLen));

  // Now add the overlay and verify that the unoverlaid resource is gone.

  ASSERT_EQ(NO_ERROR,

            target_table_.add(overlay_data_.data(), overlay_data_.size(), data_, data_size_));

  block = target_table_.getResource(kNonOverlaidResourceId, &val, false /*mayBeBag*/);

  ASSERT_LT(block, 0);

}



}  // namespace