Fix BridgeContext.get*ResourceValue()

getFrameworkResourceValue() and getProjectResourceValue() never really
returned the defValue passed to them, instead creating a new id for
non-existent resources. We now checks for the existence of the resources
before trying to obtain the id.

Change-Id: Ie3103ba32af6186651a5f77c27d1efc33dc2bcc7

4 files changed, 30 insertions, 35 deletions

diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/Bridge.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/Bridge.java
index 825731b..ec78712 100644
--- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/Bridge.java
+++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/Bridge.java
@@ -19,6 +19,7 @@ package com.android.layoutlib.bridge;
 import static com.android.ide.common.rendering.api.Result.Status.ERROR_UNKNOWN;
 import static com.android.ide.common.rendering.api.Result.Status.SUCCESS;
 
+import com.android.annotations.NonNull;
 import com.android.ide.common.rendering.api.Capability;
 import com.android.ide.common.rendering.api.DrawableParams;
 import com.android.ide.common.rendering.api.LayoutLog;
@@ -459,7 +460,7 @@ public final class Bridge extends com.android.ide.common.rendering.api.Bridge {
 
     public static void setLog(LayoutLog log) {
         // check only the thread currently owning the lock can do this.
-        if (sLock.isHeldByCurrentThread() == false) {
+        if (!sLock.isHeldByCurrentThread()) {
             throw new IllegalStateException("scene must be acquired first. see #acquire(long)");
         }
 
@@ -489,7 +490,6 @@ public final class Bridge extends com.android.ide.common.rendering.api.Bridge {
 
     /**
      * Returns the name of a framework resource whose value is an int array.
-     * @param array
      */
     public static String resolveResourceId(int[] array) {
         sIntArrayWrapper.set(array);
@@ -502,6 +502,7 @@ public final class Bridge extends com.android.ide.common.rendering.api.Bridge {
      * @param name the name of the resource.
      * @return an {@link Integer} containing the resource id, or null if no resource were found.
      */
+    @NonNull
     public static Integer getResourceId(ResourceType type, String name) {
         Map<String, Integer> map = sRevRMap.get(type);
         Integer value = null;
@@ -509,11 +510,8 @@ public final class Bridge extends com.android.ide.common.rendering.api.Bridge {
             value = map.get(name);
         }
 
-        if (value == null) {
-            value = sDynamicIds.getId(type, name);
-        }
+        return value == null ? sDynamicIds.getId(type, name) : value;
 
-        return value;
     }
 
     /**
diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java
index d804230..3d3afa4 100644
--- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java
+++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/android/BridgeContext.java
@@ -52,7 +52,6 @@ import android.content.res.BridgeTypedArray;
 import android.content.res.Configuration;
 import android.content.res.Resources;
 import android.content.res.Resources.Theme;
-import android.content.res.TypedArray;
 import android.database.DatabaseErrorHandler;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteDatabase.CursorFactory;
@@ -127,7 +126,6 @@ public final class BridgeContext extends Context {
      * @param metrics the {@link DisplayMetrics}.
      * @param renderResources the configured resources (both framework and projects) for this
      * render.
-     * @param projectCallback
      * @param config the Configuration object for this render.
      * @param targetSdkVersion the targetSdkVersion of the application.
      */
@@ -331,7 +329,7 @@ public final class BridgeContext extends Context {
             boolean attachToRoot, boolean skipCallbackParser) {
         boolean isPlatformLayout = resource.isFramework();
 
-        if (isPlatformLayout == false && skipCallbackParser == false) {
+        if (!isPlatformLayout && !skipCallbackParser) {
             // check if the project callback can provide us with a custom parser.
             ILayoutPullParser parser = getParser(resource);
 
@@ -663,7 +661,7 @@ public final class BridgeContext extends Context {
                 }
 
                 String attrName = attribute.getFirst();
-                boolean frameworkAttr = attribute.getSecond().booleanValue();
+                boolean frameworkAttr = attribute.getSecond();
                 String value = null;
                 if (set != null) {
                     value = set.getAttributeValue(
@@ -672,7 +670,7 @@ public final class BridgeContext extends Context {
 
                     // if this is an app attribute, and the first get fails, try with the
                     // new res-auto namespace as well
-                    if (frameworkAttr == false && value == null) {
+                    if (!frameworkAttr && value == null) {
                         value = set.getAttributeValue(BridgeConstants.NS_APP_RES_AUTO, attrName);
                     }
                 }
@@ -789,13 +787,13 @@ public final class BridgeContext extends Context {
         List<Pair<String, Boolean>> results = new ArrayList<Pair<String, Boolean>>(attrs.length);
 
         // for each attribute, get its name so that we can search it in the style
-        for (int i = 0 ; i < attrs.length ; i++) {
-            Pair<ResourceType, String> resolvedResource = Bridge.resolveResourceId(attrs[i]);
+        for (int attr : attrs) {
+            Pair<ResourceType, String> resolvedResource = Bridge.resolveResourceId(attr);
             boolean isFramework = false;
             if (resolvedResource != null) {
                 isFramework = true;
             } else {
-                resolvedResource = mProjectCallback.resolveResourceId(attrs[i]);
+                resolvedResource = mProjectCallback.resolveResourceId(attr);
             }
 
             if (resolvedResource != null) {
@@ -841,7 +839,7 @@ public final class BridgeContext extends Context {
 
         if (id == null) {
             // generate a new id
-            id = Integer.valueOf(++mDynamicIdGenerator);
+            id = ++mDynamicIdGenerator;
 
             // and add it to the maps.
             mDynamicIdToStyleMap.put(id, resValue);
@@ -860,19 +858,24 @@ public final class BridgeContext extends Context {
     }
 
     public int getFrameworkResourceValue(ResourceType resType, String resName, int defValue) {
-        Integer value = Bridge.getResourceId(resType, resName);
-        if (value != null) {
-            return value.intValue();
+        if (getRenderResources().getFrameworkResource(resType, resName) != null) {
+            // Bridge.getResourceId creates a new resource id if an existing one isn't found. So,
+            // we check for the existence of the resource before calling it.
+            return Bridge.getResourceId(resType, resName);
         }
 
         return defValue;
     }
 
     public int getProjectResourceValue(ResourceType resType, String resName, int defValue) {
-        if (mProjectCallback != null) {
-            Integer value = mProjectCallback.getResourceId(resType, resName);
-            if (value != null) {
-                return value.intValue();
+        // getResourceId creates a new resource id if an existing resource id isn't found. So, we
+        // check for the existence of the resource before calling it.
+        if (getRenderResources().getProjectResource(resType, resName) != null) {
+            if (mProjectCallback != null) {
+                Integer value = mProjectCallback.getResourceId(resType, resName);
+                if (value != null) {
+                    return value;
+                }
             }
         }
 
@@ -1455,9 +1458,6 @@ public final class BridgeContext extends Context {
         return null;
     }
 
-    /**
-     * @hide
-     */
     @Override
     public int getUserId() {
         return 0; // not used
diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/bars/CustomActionBarWrapper.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/bars/CustomActionBarWrapper.java
index f283e34..70b9cc3 100644
--- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/bars/CustomActionBarWrapper.java
+++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/bars/CustomActionBarWrapper.java
@@ -130,20 +130,14 @@ public abstract class CustomActionBarWrapper {
         MenuInflater inflater = new MenuInflater(getActionMenuContext());
         MenuBuilder menuBuilder = getMenuBuilder();
         for (String name : mCallback.getMenuIdNames()) {
-            int id = -1;
+            int id;
             if (name.startsWith(ANDROID_NS_NAME_PREFIX)) {
                 // Framework menu.
                 name = name.substring(ANDROID_NS_NAME_PREFIX.length());
-                if (mContext.getRenderResources().getFrameworkResource(MENU, name) != null) {
-                    // We need to check for the existence of the menu first, since getting the id
-                    // never returns the default value. It creates a new value if one is not found.
-                    id = mContext.getFrameworkResourceValue(MENU, name, -1);
-                }
+                id = mContext.getFrameworkResourceValue(MENU, name, -1);
             } else {
                 // Project menu.
-                if (mContext.getRenderResources().getProjectResource(MENU, name) != null) {
-                    id = mContext.getProjectResourceValue(MENU, name, -1);
-                }
+                id = mContext.getProjectResourceValue(MENU, name, -1);
             }
             if (id > -1) {
                 inflater.inflate(id, menuBuilder);
diff --git a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/util/DynamicIdMap.java b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/util/DynamicIdMap.java
index a1fae95..979aa33 100644
--- a/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/util/DynamicIdMap.java
+++ b/tools/layoutlib/bridge/src/com/android/layoutlib/bridge/util/DynamicIdMap.java
@@ -16,6 +16,7 @@
 
 package com.android.layoutlib.bridge.util;
 
+import com.android.annotations.NonNull;
 import com.android.resources.ResourceType;
 import com.android.util.Pair;
 
@@ -48,6 +49,7 @@ public class DynamicIdMap {
      * @param name the name of the resource
      * @return an integer.
      */
+    @NonNull
     public Integer getId(ResourceType type, String name) {
         return getId(Pair.of(type, name));
     }
@@ -59,10 +61,11 @@ public class DynamicIdMap {
      * @param resource the type/name of the resource
      * @return an integer.
      */
+    @NonNull
     public Integer getId(Pair<ResourceType, String> resource) {
         Integer value = mDynamicIds.get(resource);
         if (value == null) {
-            value = Integer.valueOf(++mDynamicSeed);
+            value = ++mDynamicSeed;
             mDynamicIds.put(resource, value);
             mRevDynamicIds.put(value, resource);
         }