Revert "Revert "Add basic implementation of IsModifiableClass""

This reverts commit c66c077d40db58ec239f93a9c42b9939439c85c7.

Reason for revert: Problem with preceding CL fixed.

Test: mma -j40 test-art-host
diff --git a/runtime/openjdkjvmti/OpenjdkJvmTi.cc b/runtime/openjdkjvmti/OpenjdkJvmTi.cc
index d364d6e..c472b54 100644
--- a/runtime/openjdkjvmti/OpenjdkJvmTi.cc
+++ b/runtime/openjdkjvmti/OpenjdkJvmTi.cc
@@ -599,7 +599,7 @@
   static jvmtiError IsModifiableClass(jvmtiEnv* env,
                                       jclass klass,
                                       jboolean* is_modifiable_class_ptr) {
-    return ERR(NOT_IMPLEMENTED);
+    return Redefiner::IsModifiableClass(env, klass, is_modifiable_class_ptr);
   }
 
   static jvmtiError GetClassLoader(jvmtiEnv* env, jclass klass, jobject* classloader_ptr) {
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index 6b0e9c8..adec6c9 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -195,6 +195,45 @@
   std::string* error_msg_;
 };
 
+jvmtiError Redefiner::IsModifiableClass(jvmtiEnv* env ATTRIBUTE_UNUSED,
+                                        jclass klass,
+                                        jboolean* is_redefinable) {
+  // TODO Check for the appropriate feature flags once we have enabled them.
+  art::Thread* self = art::Thread::Current();
+  art::ScopedObjectAccess soa(self);
+  art::StackHandleScope<1> hs(self);
+  art::ObjPtr<art::mirror::Object> obj(self->DecodeJObject(klass));
+  if (obj.IsNull()) {
+    return ERR(INVALID_CLASS);
+  }
+  art::Handle<art::mirror::Class> h_klass(hs.NewHandle(obj->AsClass()));
+  std::string err_unused;
+  *is_redefinable =
+      Redefiner::GetClassRedefinitionError(h_klass, &err_unused) == OK ? JNI_TRUE : JNI_FALSE;
+  return OK;
+}
+
+jvmtiError Redefiner::GetClassRedefinitionError(art::Handle<art::mirror::Class> klass,
+                                                /*out*/std::string* error_msg) {
+  if (klass->IsPrimitive()) {
+    *error_msg = "Modification of primitive classes is not supported";
+    return ERR(UNMODIFIABLE_CLASS);
+  } else if (klass->IsInterface()) {
+    *error_msg = "Modification of Interface classes is currently not supported";
+    return ERR(UNMODIFIABLE_CLASS);
+  } else if (klass->IsArrayClass()) {
+    *error_msg = "Modification of Array classes is not supported";
+    return ERR(UNMODIFIABLE_CLASS);
+  } else if (klass->IsProxyClass()) {
+    *error_msg = "Modification of proxy classes is not supported";
+    return ERR(UNMODIFIABLE_CLASS);
+  }
+
+  // TODO We should check if the class has non-obsoletable methods on the stack
+  LOG(WARNING) << "presence of non-obsoletable methods on stacks is not currently checked";
+  return OK;
+}
+
 // Moves dex data to an anonymous, read-only mmap'd region.
 std::unique_ptr<art::MemMap> Redefiner::MoveDataToMemMap(const std::string& original_location,
                                                          jint data_len,
@@ -625,28 +664,17 @@
 
 // TODO Move this to use IsRedefinable when that function is made.
 bool Redefiner::CheckRedefinable() {
-  art::ObjPtr<art::mirror::Class> klass(GetMirrorClass());
-  if (klass->IsPrimitive()) {
-    RecordFailure(ERR(UNMODIFIABLE_CLASS),
-                  "Modification of primitive classes is not supported");
-    return false;
-  } else if (klass->IsInterface()) {
-    RecordFailure(ERR(UNMODIFIABLE_CLASS),
-                  "Modification of Interface classes is currently not supported");
-    return false;
-  } else if (klass->IsArrayClass()) {
-    RecordFailure(ERR(UNMODIFIABLE_CLASS),
-                  "Modification of Array classes is not supported");
-    return false;
-  } else if (klass->IsProxyClass()) {
-    RecordFailure(ERR(UNMODIFIABLE_CLASS),
-                  "Modification of proxy classes is not supported");
-    return false;
-  }
+  std::string err;
+  art::StackHandleScope<1> hs(self_);
 
-  // TODO We should check if the class has non-obsoletable methods on the stack
-  LOG(WARNING) << "presence of non-obsoletable methods on stacks is not currently checked";
-  return true;
+  art::Handle<art::mirror::Class> h_klass(hs.NewHandle(GetMirrorClass()));
+  jvmtiError res = Redefiner::GetClassRedefinitionError(h_klass, &err);
+  if (res != OK) {
+    RecordFailure(res, err);
+    return false;
+  } else {
+    return true;
+  }
 }
 
 bool Redefiner::CheckRedefinitionIsValid() {
diff --git a/runtime/openjdkjvmti/ti_redefine.h b/runtime/openjdkjvmti/ti_redefine.h
index d6bccb4..01b5eca 100644
--- a/runtime/openjdkjvmti/ti_redefine.h
+++ b/runtime/openjdkjvmti/ti_redefine.h
@@ -80,6 +80,8 @@
                                   unsigned char* dex_data,
                                   std::string* error_msg);
 
+  static jvmtiError IsModifiableClass(jvmtiEnv* env, jclass klass, jboolean* is_redefinable);
+
  private:
   jvmtiError result_;
   art::Runtime* runtime_;
@@ -106,6 +108,10 @@
         error_msg_(error_msg),
         class_sig_(class_sig) { }
 
+  static jvmtiError GetClassRedefinitionError(art::Handle<art::mirror::Class> klass,
+                                              /*out*/std::string* error_msg)
+      REQUIRES_SHARED(art::Locks::mutator_lock_);
+
   static std::unique_ptr<art::MemMap> MoveDataToMemMap(const std::string& original_location,
                                                        jint data_len,
                                                        unsigned char* dex_data,
diff --git a/test/912-classes/classes.cc b/test/912-classes/classes.cc
index 19d82c5..38a4f0e 100644
--- a/test/912-classes/classes.cc
+++ b/test/912-classes/classes.cc
@@ -29,6 +29,20 @@
 namespace art {
 namespace Test912Classes {
 
+extern "C" JNIEXPORT jboolean JNICALL Java_Main_isModifiableClass(
+    JNIEnv* env ATTRIBUTE_UNUSED, jclass Main_klass ATTRIBUTE_UNUSED, jclass klass) {
+  jboolean res = JNI_FALSE;
+  jvmtiError result = jvmti_env->IsModifiableClass(klass, &res);
+  if (result != JVMTI_ERROR_NONE) {
+    char* err;
+    jvmti_env->GetErrorName(result, &err);
+    printf("Failure running IsModifiableClass: %s\n", err);
+    jvmti_env->Deallocate(reinterpret_cast<unsigned char*>(err));
+    return JNI_FALSE;
+  }
+  return res;
+}
+
 extern "C" JNIEXPORT jobjectArray JNICALL Java_Main_getClassSignature(
     JNIEnv* env, jclass Main_klass ATTRIBUTE_UNUSED, jclass klass) {
   char* sig;
diff --git a/test/912-classes/expected.txt b/test/912-classes/expected.txt
index 3507a1a..44c861a 100644
--- a/test/912-classes/expected.txt
+++ b/test/912-classes/expected.txt
@@ -12,13 +12,13 @@
 411
 [[D, null]
 411
-int interface=false array=false
-$Proxy0 interface=false array=false
-java.lang.Runnable interface=true array=false
-java.lang.String interface=false array=false
-[I interface=false array=true
-[Ljava.lang.Runnable; interface=false array=true
-[Ljava.lang.String; interface=false array=true
+int interface=false array=false modifiable=false
+$Proxy0 interface=false array=false modifiable=false
+java.lang.Runnable interface=true array=false modifiable=false
+java.lang.String interface=false array=false modifiable=true
+[I interface=false array=true modifiable=false
+[Ljava.lang.Runnable; interface=false array=true modifiable=false
+[Ljava.lang.String; interface=false array=true modifiable=false
 [public static final int java.lang.Integer.BYTES, static final char[] java.lang.Integer.DigitOnes, static final char[] java.lang.Integer.DigitTens, public static final int java.lang.Integer.MAX_VALUE, public static final int java.lang.Integer.MIN_VALUE, public static final int java.lang.Integer.SIZE, private static final java.lang.String[] java.lang.Integer.SMALL_NEG_VALUES, private static final java.lang.String[] java.lang.Integer.SMALL_NONNEG_VALUES, public static final java.lang.Class java.lang.Integer.TYPE, static final char[] java.lang.Integer.digits, private static final long java.lang.Integer.serialVersionUID, static final int[] java.lang.Integer.sizeTable, private final int java.lang.Integer.value]
 []
 []
diff --git a/test/912-classes/src/Main.java b/test/912-classes/src/Main.java
index 69e5a4c..e627d42 100644
--- a/test/912-classes/src/Main.java
+++ b/test/912-classes/src/Main.java
@@ -107,7 +107,9 @@
   private static void testClassType(Class<?> c) throws Exception {
     boolean isInterface = isInterface(c);
     boolean isArray = isArrayClass(c);
-    System.out.println(c.getName() + " interface=" + isInterface + " array=" + isArray);
+    boolean isModifiable = isModifiableClass(c);
+    System.out.println(c.getName() + " interface=" + isInterface + " array=" + isArray +
+        " modifiable=" + isModifiable);
   }
 
   private static void testClassFields(Class<?> c) throws Exception {
@@ -149,6 +151,7 @@
     }
   }
 
+  private static native boolean isModifiableClass(Class<?> c);
   private static native String[] getClassSignature(Class<?> c);
 
   private static native boolean isInterface(Class<?> c);