Fix delegation of methods within inner static classes

Currently, delegation of inner static classes methods is broken since
the rewritten method tries to pass an instance to the outer class. This
updates the method generation to only pass the reference if the inner
class is not static.

Change-Id: I8493929cafdbd80968989b422af0f956fa65681a

5 files changed, 125 insertions, 5 deletions

diff --git a/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateClassAdapter.java b/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateClassAdapter.java
index ae4a57d..7ef7566 100644
--- a/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateClassAdapter.java
+++ b/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateClassAdapter.java
@@ -17,6 +17,7 @@
 package com.android.tools.layoutlib.create;
 
 import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.FieldVisitor;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
@@ -40,6 +41,7 @@ public class DelegateClassAdapter extends ClassVisitor {
     private final String mClassName;
     private final Set<String> mDelegateMethods;
     private final Log mLog;
+    private boolean mIsStaticInnerClass;
 
     /**
      * Creates a new {@link DelegateClassAdapter} that can transform some methods
@@ -62,16 +64,30 @@ public class DelegateClassAdapter extends ClassVisitor {
         mLog = log;
         mClassName = className;
         mDelegateMethods = delegateMethods;
+        // If this is an inner class, by default, we assume it's static. If it's not we will detect
+        // by looking at the fields (see visitField)
+        mIsStaticInnerClass = className.contains("$");
     }
 
     //----------------------------------
     // Methods from the ClassAdapter
 
     @Override
+    public FieldVisitor visitField(int access, String name, String desc, String signature,
+            Object value) {
+        if (mIsStaticInnerClass && "this$0".equals(name)) {
+            // Having a "this$0" field, proves that this class is not a static inner class.
+            mIsStaticInnerClass = false;
+        }
+
+        return super.visitField(access, name, desc, signature, value);
+    }
+
+    @Override
     public MethodVisitor visitMethod(int access, String name, String desc,
             String signature, String[] exceptions) {
 
-        boolean isStatic = (access & Opcodes.ACC_STATIC) != 0;
+        boolean isStaticMethod = (access & Opcodes.ACC_STATIC) != 0;
         boolean isNative = (access & Opcodes.ACC_NATIVE) != 0;
 
         boolean useDelegate = (isNative && mDelegateMethods.contains(ALL_NATIVES)) ||
@@ -96,7 +112,8 @@ public class DelegateClassAdapter extends ClassVisitor {
             MethodVisitor mwDelegate = super.visitMethod(access, name, desc, signature, exceptions);
 
             DelegateMethodAdapter a = new DelegateMethodAdapter(
-                    mLog, null, mwDelegate, mClassName, name, desc, isStatic);
+                    mLog, null, mwDelegate, mClassName, name, desc, isStaticMethod,
+                    mIsStaticInnerClass);
 
             // A native has no code to visit, so we need to generate it directly.
             a.generateDelegateCode();
@@ -120,6 +137,7 @@ public class DelegateClassAdapter extends ClassVisitor {
                                                      desc, signature, exceptions);
 
         return new DelegateMethodAdapter(
-                mLog, mwOriginal, mwDelegate, mClassName, name, desc, isStatic);
+                mLog, mwOriginal, mwDelegate, mClassName, name, desc, isStaticMethod,
+                mIsStaticInnerClass);
     }
 }
diff --git a/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateMethodAdapter.java b/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateMethodAdapter.java
index 12690db..cca9e57 100644
--- a/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateMethodAdapter.java
+++ b/tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateMethodAdapter.java
@@ -85,6 +85,8 @@ class DelegateMethodAdapter extends MethodVisitor {
     private String mDesc;
     /** True if the original method is static. */
     private final boolean mIsStatic;
+    /** True if the method is contained in a static inner class */
+    private final boolean mIsStaticInnerClass;
     /** The internal class name (e.g. <code>com/android/SomeClass$InnerClass</code>.) */
     private final String mClassName;
     /** The method name. */
@@ -120,7 +122,8 @@ class DelegateMethodAdapter extends MethodVisitor {
             String className,
             String methodName,
             String desc,
-            boolean isStatic) {
+            boolean isStatic,
+            boolean isStaticClass) {
         super(Opcodes.ASM4);
         mLog = log;
         mOrgWriter = mvOriginal;
@@ -129,6 +132,7 @@ class DelegateMethodAdapter extends MethodVisitor {
         mMethodName = methodName;
         mDesc = desc;
         mIsStatic = isStatic;
+        mIsStaticInnerClass = isStaticClass;
     }
 
     /**
@@ -206,7 +210,7 @@ class DelegateMethodAdapter extends MethodVisitor {
         // by the 'this' of any outer class, if any.
         if (!mIsStatic) {
 
-            if (outerType != null) {
+            if (outerType != null && !mIsStaticInnerClass) {
                 // The first-level inner class has a package-protected member called 'this$0'
                 // that points to the outer class.
 
diff --git a/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/DelegateClassAdapterTest.java b/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/DelegateClassAdapterTest.java
index 648cea4..e37a09b 100644
--- a/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/DelegateClassAdapterTest.java
+++ b/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/DelegateClassAdapterTest.java
@@ -27,6 +27,7 @@ import static org.junit.Assert.fail;
 import com.android.tools.layoutlib.create.dataclass.ClassWithNative;
 import com.android.tools.layoutlib.create.dataclass.OuterClass;
 import com.android.tools.layoutlib.create.dataclass.OuterClass.InnerClass;
+import com.android.tools.layoutlib.create.dataclass.OuterClass.StaticInnerClass;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -56,6 +57,8 @@ public class DelegateClassAdapterTest {
     private static final String OUTER_CLASS_NAME = OuterClass.class.getCanonicalName();
     private static final String INNER_CLASS_NAME = OuterClass.class.getCanonicalName() + "$" +
                                                    InnerClass.class.getSimpleName();
+    private static final String STATIC_INNER_CLASS_NAME =
+            OuterClass.class.getCanonicalName() + "$" + StaticInnerClass.class.getSimpleName();
 
     @Before
     public void setUp() throws Exception {
@@ -294,6 +297,61 @@ public class DelegateClassAdapterTest {
         }
     }
 
+    @Test
+    public void testDelegateStaticInner() throws Throwable {
+        // We'll delegate the "get" method of both the inner and outer class.
+        HashSet<String> delegateMethods = new HashSet<String>();
+        delegateMethods.add("get");
+
+        // Generate the delegate for the outer class.
+        ClassWriter cwOuter = new ClassWriter(0 /*flags*/);
+        String outerClassName = OUTER_CLASS_NAME.replace('.', '/');
+        DelegateClassAdapter cvOuter = new DelegateClassAdapter(
+                mLog, cwOuter, outerClassName, delegateMethods);
+        ClassReader cr = new ClassReader(OUTER_CLASS_NAME);
+        cr.accept(cvOuter, 0 /* flags */);
+
+        // Generate the delegate for the static inner class.
+        ClassWriter cwInner = new ClassWriter(0 /*flags*/);
+        String innerClassName = STATIC_INNER_CLASS_NAME.replace('.', '/');
+        DelegateClassAdapter cvInner = new DelegateClassAdapter(
+                mLog, cwInner, innerClassName, delegateMethods);
+        cr = new ClassReader(STATIC_INNER_CLASS_NAME);
+        cr.accept(cvInner, 0 /* flags */);
+
+        // Load the generated classes in a different class loader and try them
+        ClassLoader2 cl2 = null;
+        try {
+            cl2 = new ClassLoader2() {
+                @Override
+                public void testModifiedInstance() throws Exception {
+
+                    // Check the outer class
+                    Class<?> outerClazz2 = loadClass(OUTER_CLASS_NAME);
+                    Object o2 = outerClazz2.newInstance();
+                    assertNotNull(o2);
+
+                    // Check the inner class. Since it's not a static inner class, we need
+                    // to use the hidden constructor that takes the outer class as first parameter.
+                    Class<?> innerClazz2 = loadClass(STATIC_INNER_CLASS_NAME);
+                    Constructor<?> innerCons = innerClazz2.getConstructor();
+                    Object i2 = innerCons.newInstance();
+                    assertNotNull(i2);
+
+                    // The original StaticInner.get returns 100+10+20,
+                    // but the delegate makes it return 6+10+20
+                    assertEquals(6+10+20, callGet(i2, 10, 20));
+                    assertEquals(100+10+20, callGet_Original(i2, 10, 20));
+                }
+            };
+            cl2.add(OUTER_CLASS_NAME, cwOuter.toByteArray());
+            cl2.add(STATIC_INNER_CLASS_NAME, cwInner.toByteArray());
+            cl2.testModifiedInstance();
+        } catch (Throwable t) {
+            throw dumpGeneratedClass(t, cl2);
+        }
+    }
+
     //-------
 
     /**
diff --git a/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass.java b/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass.java
index f083e76..6dfb816 100644
--- a/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass.java
+++ b/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass.java
@@ -45,6 +45,16 @@ public class OuterClass {
         }
     }
 
+    public static class StaticInnerClass {
+        public StaticInnerClass() {
+        }
+
+        // StaticInnerClass.get returns 100 + a + b
+        public int get(int a, long b) {
+            return 100 + a + (int) b;
+        }
+    }
+
     @SuppressWarnings("unused")
     private String privateMethod() {
         return "outerPrivateMethod";
diff --git a/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass_StaticInnerClass_Delegate.java b/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass_StaticInnerClass_Delegate.java
new file mode 100644
index 0000000..a29439e
--- /dev/null
+++ b/tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass_StaticInnerClass_Delegate.java
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+package com.android.tools.layoutlib.create.dataclass;
+
+import com.android.tools.layoutlib.create.DelegateClassAdapterTest;
+import com.android.tools.layoutlib.create.dataclass.OuterClass.StaticInnerClass;
+
+/**
+ * Used by {@link DelegateClassAdapterTest}.
+ */
+public class OuterClass_StaticInnerClass_Delegate {
+    // The delegate override of Inner.get return 6 + a + b
+    public static int get(StaticInnerClass inner, int a, long b) {
+        return 6 + a + (int) b;
+    }
+}