Merge changes from topic 'bug-30683723' am: 9cc20f50cb

am: 7a2d6365f3

Change-Id: Ib6dfe35a42af7647b6f678658b91a3862dffdf0f

6 files changed, 101 insertions, 4 deletions

diff --git a/Android.mk b/Android.mk
index f3eb828..76d69b2 100644
--- a/Android.mk
+++ b/Android.mk
@@ -26,6 +26,19 @@ LOCAL_MODULE := dexmaker
 # Build a static jar file.
 include $(BUILD_STATIC_JAVA_LIBRARY)
 
+# Build Dexmaker's tests
+#
+# Run the tests as follows:
+#   vogar --classpath ${ANDROID_PRODUCT_OUT}/obj/JAVA_LIBRARIES/dexmaker-tests_intermediates/classes.jack \
+        com.google.dexmaker
+
+include $(CLEAR_VARS)
+LOCAL_MODULE := dexmaker-tests
+LOCAL_SDK_VERSION := 10
+LOCAL_SRC_FILES := $(call all-java-files-under, dexmaker/src/test/java)
+LOCAL_STATIC_JAVA_LIBRARIES := dexmaker android-support-test
+include $(BUILD_STATIC_JAVA_LIBRARY)
+
 # Build Dexmaker's MockMaker, a plugin to Mockito
 include $(CLEAR_VARS)
 LOCAL_MODULE := dexmaker-mockmaker
@@ -34,3 +47,18 @@ LOCAL_SRC_FILES := $(call all-java-files-under, mockito/src/main/java)
 LOCAL_JAVA_RESOURCE_DIRS := mockito/src/main/resources
 LOCAL_JAVA_LIBRARIES := dexmaker mockito-api
 include $(BUILD_STATIC_JAVA_LIBRARY)
+
+# Build a test APK
+#
+# Run the tests as follows:
+# m -j32 DexmakerTests && \
+        am install -r -g $OUT/data/app/DexmakerTests/DexmakerTests.apk \
+        adb shell am instrument -w com.google.dexmaker.tests
+#
+include $(CLEAR_VARS)
+LOCAL_MODULE_TAGS := tests
+LOCAL_PACKAGE_NAME := DexmakerTests
+LOCAL_STATIC_JAVA_LIBRARIES := \
+        dexmaker-tests
+
+include $(BUILD_PACKAGE)
diff --git a/AndroidManifest.xml b/AndroidManifest.xml
new file mode 100644
index 0000000..2c88308
--- /dev/null
+++ b/AndroidManifest.xml
@@ -0,0 +1,12 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.google.dexmaker.tests" >
+
+    <application>
+        <uses-library android:name="android.test.runner" />
+    </application>
+
+    <instrumentation android:name="android.support.test.runner.AndroidJUnitRunner"
+                     android:targetPackage="com.google.dexmaker.tests"
+                     android:label="Dexmaker Tests"/>
+
+</manifest>
diff --git a/dexmaker/src/main/java/com/google/dexmaker/Code.java b/dexmaker/src/main/java/com/google/dexmaker/Code.java
index 54409a5..d85dece 100644
--- a/dexmaker/src/main/java/com/google/dexmaker/Code.java
+++ b/dexmaker/src/main/java/com/google/dexmaker/Code.java
@@ -853,7 +853,7 @@ public final class Code {
      * Releases the held lock on {@code monitor}.
      */
     public void monitorExit(Local<?> monitor) {
-        addInstruction(new ThrowingInsn(Rops.MONITOR_ENTER, sourcePosition,
+        addInstruction(new ThrowingInsn(Rops.MONITOR_EXIT, sourcePosition,
                 RegisterSpecList.make(monitor.spec()), catches));
     }
 
diff --git a/dexmaker/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java b/dexmaker/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
index 49d1224..c42d24d 100644
--- a/dexmaker/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
+++ b/dexmaker/src/main/java/com/google/dexmaker/stock/ProxyBuilder.java
@@ -619,9 +619,18 @@ public final class ProxyBuilder<T> {
     private Method[] getMethodsToProxyRecursive() {
         Set<MethodSetEntry> methodsToProxy = new HashSet<MethodSetEntry>();
         Set<MethodSetEntry> seenFinalMethods = new HashSet<MethodSetEntry>();
+        // Traverse the class hierarchy to ensure that all concrete methods (which could be marked
+        // as final) are visited before any abstract methods from interfaces.
         for (Class<?> c = baseClass; c != null; c = c.getSuperclass()) {
             getMethodsToProxy(methodsToProxy, seenFinalMethods, c);
         }
+        // Now traverse the interface hierarchy, starting with the ones implemented by the class,
+        // followed by any extra interfaces.
+        for (Class<?> c = baseClass; c != null; c = c.getSuperclass()) {
+            for (Class<?> i : c.getInterfaces()) {
+                getMethodsToProxy(methodsToProxy, seenFinalMethods, i);
+            }
+        }
         for (Class<?> c : interfaces) {
             getMethodsToProxy(methodsToProxy, seenFinalMethods, c);
         }
@@ -689,8 +698,12 @@ public final class ProxyBuilder<T> {
             sink.add(entry);
         }
 
-        for (Class<?> i : c.getInterfaces()) {
-            getMethodsToProxy(sink, seenFinalMethods, i);
+        // Only visit the interfaces of this class if it is itself an interface. That prevents
+        // visiting interfaces of a class before its super classes.
+        if (c.isInterface()) {
+            for (Class<?> i : c.getInterfaces()) {
+                getMethodsToProxy(sink, seenFinalMethods, i);
+            }
         }
     }
 
diff --git a/dexmaker/src/test/java/com/google/dexmaker/DexMakerTest.java b/dexmaker/src/test/java/com/google/dexmaker/DexMakerTest.java
index 2b7a27b..36e6c00 100644
--- a/dexmaker/src/test/java/com/google/dexmaker/DexMakerTest.java
+++ b/dexmaker/src/test/java/com/google/dexmaker/DexMakerTest.java
@@ -16,6 +16,7 @@
 
 package com.google.dexmaker;
 
+import android.support.test.InstrumentationRegistry;
 import java.io.File;
 import java.io.FilenameFilter;
 import java.io.IOException;
@@ -2007,6 +2008,18 @@ public final class DexMakerTest extends TestCase {
     }
 
     public static File getDataDirectory() {
+        // Assume that this is being run as an APK by AndroidJUnitRunner and so get the directory
+        // where the APK can create files.
+        try {
+            // This will fail on Vogar as it will not register an Instrumentation.
+            return InstrumentationRegistry.getContext().getFilesDir();
+        } catch (IllegalStateException t) {
+            // Drop through to an alternate mechanism for getting a directory that works on Vogar.
+            if (!t.getMessage().contains("No instrumentation registered!")) {
+                // The exception wasn't expected.
+                throw t;
+            }
+        }
         String envVariable = "ANDROID_DATA";
         String defaultLoc = "/data";
         String path = System.getenv(envVariable);
diff --git a/dexmaker/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java b/dexmaker/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java
index 2dfac5e..9c4cffe 100644
--- a/dexmaker/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java
+++ b/dexmaker/src/test/java/com/google/dexmaker/stock/ProxyBuilderTest.java
@@ -37,7 +37,9 @@ public class ProxyBuilderTest extends TestCase {
 
     public void setUp() throws Exception {
         super.setUp();
-        versionedDxDir.mkdirs();
+        if (!versionedDxDir.exists() && !versionedDxDir.mkdirs()) {
+            throw new IOException("Could not create " + versionedDxDir);
+        }
         clearVersionedDxDir();
         getGeneratedProxyClasses().clear();
     }
@@ -882,6 +884,35 @@ public class ProxyBuilderTest extends TestCase {
         assertEquals("no proxy", proxyFor(ExtenstionOfFinalInterfaceImpl.class).build().foo());
     }
 
+    // https://code.google.com/p/dexmaker/issues/detail?id=9
+    public interface DeclaresMethodLate {
+        void thisIsTheMethod();
+    }
+
+    public static class MakesMethodFinalEarly {
+        public final void thisIsTheMethod() {}
+    }
+
+    public static class YouDoNotChooseYourFamily
+            extends MakesMethodFinalEarly implements DeclaresMethodLate {}
+
+    public void testInterfaceMethodMadeFinalBeforeActualInheritance() throws Exception {
+        proxyFor(YouDoNotChooseYourFamily.class).build();
+    }
+
+    public interface ExtendsAnotherInterface extends FooReturnsString {
+
+    }
+
+    public void testExtraInterfaceExtendsInterface() throws Exception {
+        ExtendsAnotherInterface proxy = (ExtendsAnotherInterface)
+                proxyFor(SimpleClass.class)
+                        .implementing(ExtendsAnotherInterface.class)
+                        .build();
+        fakeHandler.setFakeResult(ExtendsAnotherInterface.class.getName());
+        assertEquals(ExtendsAnotherInterface.class.getName(), proxy.foo());
+    }
+
     /** Simple helper to add the most common args for this test to the proxy builder. */
     private <T> ProxyBuilder<T> proxyFor(Class<T> clazz) throws Exception {
         return ProxyBuilder.forClass(clazz)