Refactor/add tests for ObjectStreamClass computeDefaultSUID

b/29064453 added a patch to the computeDefaultSUID() method to maintain
backwards compatibility in how it is calculated for apps that target
<= 23. This change clarifies the behavior by refactoring the code and
adding a comment as well as adding appropriate change markers to
ObjectStreamClass.c. It also adds tests to verify the impact of those
changes on the calculation of the serial version UID.

The original code used a variable/parameter called checkSuperclass that
was confusing as it related to how the native method was implemented
rather than the external effect. The native code works as follows:

1) In order to find the <clinit> method the native code uses a method
   that searches up the super class hierarchy. That means that if a
   class does not have a <clinit> method but a superclass does then it
   will find the superclass' method.

2) If it does not find a method then it returns false which is correct
   behavior. If it does find a method then it checks the value of the
   checkSuperclass parameter.

3) If checkSuperclass is false then the native code returns true
   because it assumes the method it found belongs to the initial class.
   That is incorrect behavior (kept for backwards compatibility) and
   confusing because even though checkSuperclass is false it has
   actually checked the super class.

4) If checkSuperclass is true then the native code will run the same
   search as it did in #1 but on the super class. If no method could
   be found or the method was different to the one found in #1 then it
   returns true as the method found in #1 must have come from the
   original class. Otherwise, it returns false as the method found in
   #1 was actually from the super class. This is confusing because it
   actually checks the super class twice.

Hence the checkSuperclass parameter was inverted and renamed to
inheritStaticInitializer as that describes the behavior and seems
clearer.

That broke the harmony test ObjectStreamClassTest#testHasClinit(). That
test was very low level that does not test any externally visible
behavior but calls the hasStaticInitializer(...) method directly.
Rather than fix that the test was removed and replaced with tests that
check the behavior of that class by its external impact. The test is
described below.

The supported way to access the SUID for a class is to use:
    ObjectStreamClass.lookup(Class).getSerialVersionUID()

If the Class supplied to the lookup() method does not have a
serialVersionUID field then that will call the
computeDefaultSUID(Class) method to compute the default SUID.
Unfortunately, the lookup(Class) method caches the results of the class
lookup (including the computed default SUID) which makes it impossible
to test the behavior of the method on the same Class for version <= 23
and for version > 23 in the same application. Each test has to have its
own Class, with its own default SUID making it difficult to understand
which tests are affected by the version and which are not.

So, instead of using the lookup(Class) method the tests use reflection
to invoke the computeDefaultSUID(Class) method directly, bypassing the
lookup cache and allowing the default SUID to be computed multiple
times for the same class. Each Class has two tests, one for testing
version <= 23 and one for version > 23. For those Classes whose default
SUID is unaffected by the version the tests use a common constant for
the expected SUID.

Tested by running the tests on the device before and after cleaning up
the code.

Test: atest \
    core-tests:libcore.java.io.ObjectStreamClassTest \
    org.apache.harmony.tests.java.io.ObjectStreamClassTest

Bug: 109930347
Change-Id: I245118af8a81ffba123d773e37560ef655e108a3

1 files changed, 0 insertions, 58 deletions

diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java
index cc397078ef..16a7415090 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/io/ObjectStreamClassTest.java
@@ -284,64 +284,6 @@ public class ObjectStreamClassTest extends TestCaseWithRules {
         return newInstance;
     }
 
-    // Class without <clinit> method
-    public static class NoClinitParent {
-    }
-    // Class without <clinit> method
-    public static class NoClinitChildWithNoClinitParent extends NoClinitParent {
-    }
-
-    // Class with <clinit> method
-    public static class ClinitParent {
-        // This field will trigger creation of <clinit> method for this class
-        private static final String TAG = ClinitParent.class.getName();
-        static {
-
-        }
-    }
-    // Class without <clinit> but with parent that has <clinit> method
-    public static class NoClinitChildWithClinitParent extends ClinitParent {
-    }
-
-    // http://b/29064453
-    public void testHasClinit() throws Exception {
-        Method hasStaticInitializer =
-            ObjectStreamClass.class.getDeclaredMethod("hasStaticInitializer", Class.class,
-                                                      boolean.class);
-        hasStaticInitializer.setAccessible(true);
-
-        assertTrue((Boolean)
-                   hasStaticInitializer.invoke(null, ClinitParent.class,
-                                               false /* checkSuperclass */));
-
-        // RI will return correctly False in this case, but android has been returning true
-        // in this particular case. We're returning true to enable deserializing classes
-        // like NoClinitChildWithClinitParent without explicit serialVersionID field.
-        assertTrue((Boolean)
-                   hasStaticInitializer.invoke(null, NoClinitChildWithClinitParent.class,
-                                               false /* checkSuperclass */));
-        assertFalse((Boolean)
-                    hasStaticInitializer.invoke(null, NoClinitParent.class,
-                                                false /* checkSuperclass */));
-        assertFalse((Boolean)
-                    hasStaticInitializer.invoke(null, NoClinitChildWithNoClinitParent.class,
-                                                false /* checkSuperclass */));
-
-
-        assertTrue((Boolean)
-                   hasStaticInitializer.invoke(null, ClinitParent.class,
-                                               true /* checkSuperclass */));
-        assertFalse((Boolean)
-                   hasStaticInitializer.invoke(null, NoClinitChildWithClinitParent.class,
-                                               true /* checkSuperclass */));
-        assertFalse((Boolean)
-                    hasStaticInitializer.invoke(null, NoClinitParent.class,
-                                                true /* checkSuperclass */));
-        assertFalse((Boolean)
-                    hasStaticInitializer.invoke(null, NoClinitChildWithNoClinitParent.class,
-                                                true /* checkSuperclass */));
-    }
-
     // http://b/29721023
     public void testClassWithSameFieldName() throws Exception {
         // Load class from dex, it's not possible to create a class with same-named