Skip to content

Commit d4c25db

Browse files
Adam LesinskiAndroid (Google) Code Review
Adam Lesinski
authored and
Android (Google) Code Review
committed
Merge "Fix shared library bug in bag attributes" into lmp-dev
2 parents 0fe5bf9 + ccf25c7 commit d4c25db

File tree

11 files changed

+232
-61
lines changed

11 files changed

+232
-61
lines changed

‎core/jni/android_util_AssetManager.cpp‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,7 +1021,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
10211021
return JNI_FALSE;
10221022
}
10231023

1024-
DEBUG_STYLES(LOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x",
1024+
DEBUG_STYLES(ALOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x",
10251025
themeToken, defStyleAttr, defStyleRes));
10261026

10271027
ResTable::Theme* theme = reinterpret_cast<ResTable::Theme*>(themeToken);
@@ -1089,7 +1089,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
10891089
for (jsize ii=0; ii<NI; ii++) {
10901090
const uint32_t curIdent = (uint32_t)src[ii];
10911091

1092-
DEBUG_STYLES(LOGI("RETRIEVING ATTR 0x%08x...", curIdent));
1092+
DEBUG_STYLES(ALOGI("RETRIEVING ATTR 0x%08x...", curIdent));
10931093

10941094
// Try to find a value for this attribute... we prioritize values
10951095
// coming from, first XML attributes, then XML style, then default
@@ -1104,7 +1104,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
11041104
block = -1;
11051105
value.dataType = Res_value::TYPE_ATTRIBUTE;
11061106
value.data = srcValues[ii];
1107-
DEBUG_STYLES(LOGI("-> From values: type=0x%x, data=0x%08x",
1107+
DEBUG_STYLES(ALOGI("-> From values: type=0x%x, data=0x%08x",
11081108
value.dataType, value.data));
11091109
}
11101110

@@ -1118,7 +1118,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
11181118
block = defStyleEnt->stringBlock;
11191119
typeSetFlags = defStyleTypeSetFlags;
11201120
value = defStyleEnt->map.value;
1121-
DEBUG_STYLES(LOGI("-> From def style: type=0x%x, data=0x%08x",
1121+
DEBUG_STYLES(ALOGI("-> From def style: type=0x%x, data=0x%08x",
11221122
value.dataType, value.data));
11231123
}
11241124
defStyleEnt++;
@@ -1130,14 +1130,14 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
11301130
ssize_t newBlock = theme->resolveAttributeReference(&value, block,
11311131
&resid, &typeSetFlags, &config);
11321132
if (newBlock >= 0) block = newBlock;
1133-
DEBUG_STYLES(LOGI("-> Resolved attr: type=0x%x, data=0x%08x",
1133+
DEBUG_STYLES(ALOGI("-> Resolved attr: type=0x%x, data=0x%08x",
11341134
value.dataType, value.data));
11351135
} else {
11361136
// If we still don't have a value for this attribute, try to find
11371137
// it in the theme!
11381138
ssize_t newBlock = theme->getAttribute(curIdent, &value, &typeSetFlags);
11391139
if (newBlock >= 0) {
1140-
DEBUG_STYLES(LOGI("-> From theme: type=0x%x, data=0x%08x",
1140+
DEBUG_STYLES(ALOGI("-> From theme: type=0x%x, data=0x%08x",
11411141
value.dataType, value.data));
11421142
newBlock = res.resolveReference(&value, block, &resid,
11431143
&typeSetFlags, &config);
@@ -1148,19 +1148,19 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
11481148
}
11491149
#endif
11501150
if (newBlock >= 0) block = newBlock;
1151-
DEBUG_STYLES(LOGI("-> Resolved theme: type=0x%x, data=0x%08x",
1151+
DEBUG_STYLES(ALOGI("-> Resolved theme: type=0x%x, data=0x%08x",
11521152
value.dataType, value.data));
11531153
}
11541154
}
11551155

11561156
// Deal with the special @null value -- it turns back to TYPE_NULL.
11571157
if (value.dataType == Res_value::TYPE_REFERENCE && value.data == 0) {
1158-
DEBUG_STYLES(LOGI("-> Setting to @null!"));
1158+
DEBUG_STYLES(ALOGI("-> Setting to @null!"));
11591159
value.dataType = Res_value::TYPE_NULL;
11601160
block = -1;
11611161
}
11621162

1163-
DEBUG_STYLES(LOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
1163+
DEBUG_STYLES(ALOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
11641164
curIdent, value.dataType, value.data));
11651165

11661166
// Write the final value back to Java.
@@ -1215,7 +1215,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
12151215
return JNI_FALSE;
12161216
}
12171217

1218-
DEBUG_STYLES(LOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x xml=0x%x",
1218+
DEBUG_STYLES(ALOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x xml=0x%x",
12191219
themeToken, defStyleAttr, defStyleRes, xmlParserToken));
12201220

12211221
ResTable::Theme* theme = reinterpret_cast<ResTable::Theme*>(themeToken);
@@ -1313,7 +1313,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
13131313
for (jsize ii=0; ii<NI; ii++) {
13141314
const uint32_t curIdent = (uint32_t)src[ii];
13151315

1316-
DEBUG_STYLES(LOGI("RETRIEVING ATTR 0x%08x...", curIdent));
1316+
DEBUG_STYLES(ALOGI("RETRIEVING ATTR 0x%08x...", curIdent));
13171317

13181318
// Try to find a value for this attribute... we prioritize values
13191319
// coming from, first XML attributes, then XML style, then default
@@ -1334,7 +1334,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
13341334
xmlParser->getAttributeValue(ix, &value);
13351335
ix++;
13361336
curXmlAttr = xmlParser->getAttributeNameResID(ix);
1337-
DEBUG_STYLES(LOGI("-> From XML: type=0x%x, data=0x%08x",
1337+
DEBUG_STYLES(ALOGI("-> From XML: type=0x%x, data=0x%08x",
13381338
value.dataType, value.data));
13391339
}
13401340

@@ -1348,7 +1348,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
13481348
block = styleEnt->stringBlock;
13491349
typeSetFlags = styleTypeSetFlags;
13501350
value = styleEnt->map.value;
1351-
DEBUG_STYLES(LOGI("-> From style: type=0x%x, data=0x%08x",
1351+
DEBUG_STYLES(ALOGI("-> From style: type=0x%x, data=0x%08x",
13521352
value.dataType, value.data));
13531353
}
13541354
styleEnt++;
@@ -1364,7 +1364,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
13641364
block = defStyleEnt->stringBlock;
13651365
typeSetFlags = defStyleTypeSetFlags;
13661366
value = defStyleEnt->map.value;
1367-
DEBUG_STYLES(LOGI("-> From def style: type=0x%x, data=0x%08x",
1367+
DEBUG_STYLES(ALOGI("-> From def style: type=0x%x, data=0x%08x",
13681368
value.dataType, value.data));
13691369
}
13701370
defStyleEnt++;
@@ -1376,14 +1376,14 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
13761376
ssize_t newBlock = theme->resolveAttributeReference(&value, block,
13771377
&resid, &typeSetFlags, &config);
13781378
if (newBlock >= 0) block = newBlock;
1379-
DEBUG_STYLES(LOGI("-> Resolved attr: type=0x%x, data=0x%08x",
1379+
DEBUG_STYLES(ALOGI("-> Resolved attr: type=0x%x, data=0x%08x",
13801380
value.dataType, value.data));
13811381
} else {
13821382
// If we still don't have a value for this attribute, try to find
13831383
// it in the theme!
13841384
ssize_t newBlock = theme->getAttribute(curIdent, &value, &typeSetFlags);
13851385
if (newBlock >= 0) {
1386-
DEBUG_STYLES(LOGI("-> From theme: type=0x%x, data=0x%08x",
1386+
DEBUG_STYLES(ALOGI("-> From theme: type=0x%x, data=0x%08x",
13871387
value.dataType, value.data));
13881388
newBlock = res.resolveReference(&value, block, &resid,
13891389
&typeSetFlags, &config);
@@ -1394,19 +1394,19 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
13941394
}
13951395
#endif
13961396
if (newBlock >= 0) block = newBlock;
1397-
DEBUG_STYLES(LOGI("-> Resolved theme: type=0x%x, data=0x%08x",
1397+
DEBUG_STYLES(ALOGI("-> Resolved theme: type=0x%x, data=0x%08x",
13981398
value.dataType, value.data));
13991399
}
14001400
}
14011401

14021402
// Deal with the special @null value -- it turns back to TYPE_NULL.
14031403
if (value.dataType == Res_value::TYPE_REFERENCE && value.data == 0) {
1404-
DEBUG_STYLES(LOGI("-> Setting to @null!"));
1404+
DEBUG_STYLES(ALOGI("-> Setting to @null!"));
14051405
value.dataType = Res_value::TYPE_NULL;
14061406
block = kXmlBlock;
14071407
}
14081408

1409-
DEBUG_STYLES(LOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
1409+
DEBUG_STYLES(ALOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
14101410
curIdent, value.dataType, value.data));
14111411

14121412
// Write the final value back to Java.

‎libs/androidfw/ResourceTypes.cpp‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3907,7 +3907,17 @@ ssize_t ResTable::getBagLocked(uint32_t resID, const bag_entry** outBag,
39073907
map = (const ResTable_map*)(((const uint8_t*)entry.type) + curOff);
39083908
N++;
39093909

3910-
const uint32_t newName = htodl(map->name.ident);
3910+
uint32_t newName = htodl(map->name.ident);
3911+
if (!Res_INTERNALID(newName)) {
3912+
// Attributes don't have a resource id as the name. They specify
3913+
// other data, which would be wrong to change via a lookup.
3914+
if (grp->dynamicRefTable.lookupResourceId(&newName) != NO_ERROR) {
3915+
ALOGE("Failed resolving ResTable_map name at %d with ident 0x%08x",
3916+
(int) curOff, (int) newName);
3917+
return UNKNOWN_ERROR;
3918+
}
3919+
}
3920+
39113921
bool isInside;
39123922
uint32_t oldName = 0;
39133923
while ((isInside=(curEntry < set->numAttrs))
@@ -5856,11 +5866,11 @@ status_t DynamicRefTable::lookupResourceId(uint32_t* resId) const {
58565866
// Do a proper lookup.
58575867
uint8_t translatedId = mLookupTable[packageId];
58585868
if (translatedId == 0) {
5859-
ALOGV("DynamicRefTable(0x%02x): No mapping for build-time package ID 0x%02x.",
5869+
ALOGE("DynamicRefTable(0x%02x): No mapping for build-time package ID 0x%02x.",
58605870
(uint8_t)mAssignedPackageId, (uint8_t)packageId);
58615871
for (size_t i = 0; i < 256; i++) {
58625872
if (mLookupTable[i] != 0) {
5863-
ALOGV("e[0x%02x] -> 0x%02x", (uint8_t)i, mLookupTable[i]);
5873+
ALOGE("e[0x%02x] -> 0x%02x", (uint8_t)i, mLookupTable[i]);
58645874
}
58655875
}
58665876
return UNKNOWN_ERROR;

‎libs/androidfw/tests/Idmap_test.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
#include <utils/String8.h>
2020
#include <utils/String16.h>
2121
#include "TestHelpers.h"
22-
#include "data/R.h"
22+
#include "data/basic/R.h"
2323

2424
#include <gtest/gtest.h>
2525

@@ -70,7 +70,7 @@ TEST_F(IdmapTest, canLoadIdmap) {
7070

7171
TEST_F(IdmapTest, overlayOverridesResourceValue) {
7272
Res_value val;
73-
ssize_t block = mTargetTable.getResource(R::string::test2, &val, false);
73+
ssize_t block = mTargetTable.getResource(base::R::string::test2, &val, false);
7474
ASSERT_GE(block, 0);
7575
ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
7676
const ResStringPool* pool = mTargetTable.getTableStringBlock(block);
@@ -84,7 +84,7 @@ TEST_F(IdmapTest, overlayOverridesResourceValue) {
8484

8585
ASSERT_EQ(NO_ERROR, mTargetTable.add(overlay_arsc, overlay_arsc_len, mData, mDataSize));
8686

87-
ssize_t newBlock = mTargetTable.getResource(R::string::test2, &val, false);
87+
ssize_t newBlock = mTargetTable.getResource(base::R::string::test2, &val, false);
8888
ASSERT_GE(newBlock, 0);
8989
ASSERT_NE(block, newBlock);
9090
ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
@@ -101,7 +101,7 @@ TEST_F(IdmapTest, overlaidResourceHasSameName) {
101101
ASSERT_EQ(NO_ERROR, mTargetTable.add(overlay_arsc, overlay_arsc_len, mData, mDataSize));
102102

103103
ResTable::resource_name resName;
104-
ASSERT_TRUE(mTargetTable.getResourceName(R::array::integerArray1, false, &resName));
104+
ASSERT_TRUE(mTargetTable.getResourceName(base::R::array::integerArray1, false, &resName));
105105

106106
ASSERT_TRUE(resName.package != NULL);
107107
ASSERT_TRUE(resName.type != NULL);

‎libs/androidfw/tests/ResTable_test.cpp‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
#include <utils/String8.h>
2020
#include <utils/String16.h>
2121
#include "TestHelpers.h"
22-
#include "data/R.h"
22+
#include "data/basic/R.h"
23+
#include "data/lib/R.h"
2324

2425
#include <gtest/gtest.h>
2526

@@ -34,6 +35,8 @@ namespace {
3435
*/
3536
#include "data/basic/basic_arsc.h"
3637

38+
#include "data/lib/lib_arsc.h"
39+
3740
enum { MAY_NOT_BE_BAG = false };
3841

3942
TEST(ResTableTest, shouldLoadSuccessfully) {
@@ -46,7 +49,7 @@ TEST(ResTableTest, simpleTypeIsRetrievedCorrectly) {
4649
ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
4750

4851
Res_value val;
49-
ssize_t block = table.getResource(R::string::test1, &val, MAY_NOT_BE_BAG);
52+
ssize_t block = table.getResource(base::R::string::test1, &val, MAY_NOT_BE_BAG);
5053

5154
ASSERT_GE(block, 0);
5255
ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
@@ -66,76 +69,91 @@ TEST(ResTableTest, resourceNameIsResolved) {
6669
0, 0,
6770
defPackage.string(), defPackage.size());
6871
ASSERT_NE(uint32_t(0x00000000), resID);
69-
ASSERT_EQ(R::string::test1, resID);
72+
ASSERT_EQ(base::R::string::test1, resID);
7073
}
7174

7275
TEST(ResTableTest, noParentThemeIsAppliedCorrectly) {
7376
ResTable table;
7477
ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
7578

7679
ResTable::Theme theme(table);
77-
ASSERT_EQ(NO_ERROR, theme.applyStyle(R::style::Theme1));
80+
ASSERT_EQ(NO_ERROR, theme.applyStyle(base::R::style::Theme1));
7881

7982
Res_value val;
8083
uint32_t specFlags = 0;
81-
ssize_t index = theme.getAttribute(R::attr::attr1, &val, &specFlags);
84+
ssize_t index = theme.getAttribute(base::R::attr::attr1, &val, &specFlags);
8285
ASSERT_GE(index, 0);
8386
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
8487
ASSERT_EQ(uint32_t(100), val.data);
8588

86-
index = theme.getAttribute(R::attr::attr2, &val, &specFlags);
89+
index = theme.getAttribute(base::R::attr::attr2, &val, &specFlags);
8790
ASSERT_GE(index, 0);
8891
ASSERT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
89-
ASSERT_EQ(R::integer::number1, val.data);
92+
ASSERT_EQ(base::R::integer::number1, val.data);
9093
}
9194

9295
TEST(ResTableTest, parentThemeIsAppliedCorrectly) {
9396
ResTable table;
9497
ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
9598

9699
ResTable::Theme theme(table);
97-
ASSERT_EQ(NO_ERROR, theme.applyStyle(R::style::Theme2));
100+
ASSERT_EQ(NO_ERROR, theme.applyStyle(base::R::style::Theme2));
98101

99102
Res_value val;
100103
uint32_t specFlags = 0;
101-
ssize_t index = theme.getAttribute(R::attr::attr1, &val, &specFlags);
104+
ssize_t index = theme.getAttribute(base::R::attr::attr1, &val, &specFlags);
102105
ASSERT_GE(index, 0);
103106
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
104107
ASSERT_EQ(uint32_t(300), val.data);
105108

106-
index = theme.getAttribute(R::attr::attr2, &val, &specFlags);
109+
index = theme.getAttribute(base::R::attr::attr2, &val, &specFlags);
107110
ASSERT_GE(index, 0);
108111
ASSERT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
109-
ASSERT_EQ(R::integer::number1, val.data);
112+
ASSERT_EQ(base::R::integer::number1, val.data);
113+
}
114+
115+
TEST(ResTableTest, libraryThemeIsAppliedCorrectly) {
116+
ResTable table;
117+
ASSERT_EQ(NO_ERROR, table.add(lib_arsc, lib_arsc_len));
118+
119+
ResTable::Theme theme(table);
120+
ASSERT_EQ(NO_ERROR, theme.applyStyle(lib::R::style::Theme));
121+
122+
Res_value val;
123+
uint32_t specFlags = 0;
124+
ssize_t index = theme.getAttribute(lib::R::attr::attr1, &val, &specFlags);
125+
ASSERT_GE(index, 0);
126+
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
127+
ASSERT_EQ(uint32_t(700), val.data);
110128
}
111129

112130
TEST(ResTableTest, referenceToBagIsNotResolved) {
113131
ResTable table;
114132
ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
115133

116134
Res_value val;
117-
ssize_t block = table.getResource(R::integer::number2, &val, MAY_NOT_BE_BAG);
135+
ssize_t block = table.getResource(base::R::integer::number2, &val, MAY_NOT_BE_BAG);
118136
ASSERT_GE(block, 0);
119137
ASSERT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
120-
ASSERT_EQ(R::array::integerArray1, val.data);
138+
ASSERT_EQ(base::R::array::integerArray1, val.data);
121139

122140
ssize_t newBlock = table.resolveReference(&val, block);
123141
EXPECT_EQ(block, newBlock);
124142
EXPECT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
125-
EXPECT_EQ(R::array::integerArray1, val.data);
143+
EXPECT_EQ(base::R::array::integerArray1, val.data);
126144
}
127145

128146
TEST(ResTableTest, resourcesStillAccessibleAfterParameterChange) {
129147
ResTable table;
130148
ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
131149

132150
Res_value val;
133-
ssize_t block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
151+
ssize_t block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
134152
ASSERT_GE(block, 0);
135153
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
136154

137155
const ResTable::bag_entry* entry;
138-
ssize_t count = table.lockBag(R::array::integerArray1, &entry);
156+
ssize_t count = table.lockBag(base::R::array::integerArray1, &entry);
139157
ASSERT_GE(count, 0);
140158
table.unlockBag(entry);
141159

@@ -144,11 +162,11 @@ TEST(ResTableTest, resourcesStillAccessibleAfterParameterChange) {
144162
param.density = 320;
145163
table.setParameters(&param);
146164

147-
block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
165+
block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
148166
ASSERT_GE(block, 0);
149167
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
150168

151-
count = table.lockBag(R::array::integerArray1, &entry);
169+
count = table.lockBag(base::R::array::integerArray1, &entry);
152170
ASSERT_GE(count, 0);
153171
table.unlockBag(entry);
154172
}
@@ -158,7 +176,7 @@ TEST(ResTableTest, resourceIsOverridenWithBetterConfig) {
158176
ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
159177

160178
Res_value val;
161-
ssize_t block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
179+
ssize_t block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
162180
ASSERT_GE(block, 0);
163181
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
164182
ASSERT_EQ(uint32_t(200), val.data);
@@ -171,7 +189,7 @@ TEST(ResTableTest, resourceIsOverridenWithBetterConfig) {
171189
param.country[1] = 'E';
172190
table.setParameters(&param);
173191

174-
block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
192+
block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
175193
ASSERT_GE(block, 0);
176194
ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
177195
ASSERT_EQ(uint32_t(400), val.data);

0 commit comments

Comments
 (0)