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

Commit 6777f54f authored by Diego Perez's avatar Diego Perez
Browse files

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
parent 7a062ed3

tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateClassAdapter.java

+21 −3
Original line number Diff line number Diff line
@@ -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);

    }

}

tools/layoutlib/create/src/com/android/tools/layoutlib/create/DelegateMethodAdapter.java

+6 −2
Original line number Diff line number Diff line
@@ -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.

tools/layoutlib/create/tests/com/android/tools/layoutlib/create/DelegateClassAdapterTest.java

+58 −0
Original line number Diff line number Diff line
@@ -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);

        }

    }



    //-------



    /**

tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass.java

+10 −0
Original line number Diff line number Diff line
@@ -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";

tools/layoutlib/create/tests/com/android/tools/layoutlib/create/dataclass/OuterClass_StaticInnerClass_Delegate.java

0 → 100644
+30 −0
Original line number Diff line number Diff line 
/*

 * 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;

    }

}