Ensure ClassPreDefine returned dex file is on the Classpath

Test: mma -j40 test-art-host
Change-Id: Icf70a78f3a1149d0e5bf9aa64f74f2ca8d025802
diff --git a/runtime/openjdkjvmti/Android.bp b/runtime/openjdkjvmti/Android.bp
index 976a1e7..c01e3f4 100644
--- a/runtime/openjdkjvmti/Android.bp
+++ b/runtime/openjdkjvmti/Android.bp
@@ -22,6 +22,7 @@
            "OpenjdkJvmTi.cc",
            "ti_class.cc",
            "ti_class_definition.cc",
+           "ti_class_loader.cc",
            "ti_dump.cc",
            "ti_field.cc",
            "ti_heap.cc",
diff --git a/runtime/openjdkjvmti/ti_class.cc b/runtime/openjdkjvmti/ti_class.cc
index b6de592..35c117d 100644
--- a/runtime/openjdkjvmti/ti_class.cc
+++ b/runtime/openjdkjvmti/ti_class.cc
@@ -54,6 +54,7 @@
 #include "scoped_thread_state_change-inl.h"
 #include "thread-inl.h"
 #include "thread_list.h"
+#include "ti_class_loader.h"
 #include "ti_redefine.h"
 #include "utils.h"
 
@@ -228,12 +229,15 @@
         return;
       }
 
-      // TODO Check Redefined dex file for invariants.
+      // TODO Check Redefined dex file for all invariants.
       LOG(WARNING) << "Dex file created by class-definition time transformation of "
                    << descriptor << " is not checked for all retransformation invariants.";
-      // TODO Put it in classpath
-      LOG(WARNING) << "Dex file created for class-definition time transformation of "
-                   << descriptor << " was not added to any classpaths!";
+
+      if (!ClassLoaderHelper::AddToClassLoader(self, class_loader, dex_file.get())) {
+        LOG(ERROR) << "Unable to add " << descriptor << " to class loader!";
+        return;
+      }
+
       // Actually set the ClassExt's original bytes once we have actually succeeded.
       ext->SetOriginalDexFileBytes(arr.Get());
       // Set the return values
diff --git a/runtime/openjdkjvmti/ti_class_loader.cc b/runtime/openjdkjvmti/ti_class_loader.cc
new file mode 100644
index 0000000..b68fc60
--- /dev/null
+++ b/runtime/openjdkjvmti/ti_class_loader.cc
@@ -0,0 +1,202 @@
+/* Copyright (C) 2017 The Android Open Source Project
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This file implements interfaces from the file jvmti.h. This implementation
+ * is licensed under the same terms as the file jvmti.h.  The
+ * copyright and license information for the file jvmti.h follows.
+ *
+ * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "ti_class_loader.h"
+
+#include <limits>
+
+#include "android-base/stringprintf.h"
+
+#include "art_jvmti.h"
+#include "base/array_slice.h"
+#include "base/logging.h"
+#include "dex_file.h"
+#include "dex_file_types.h"
+#include "events-inl.h"
+#include "gc/allocation_listener.h"
+#include "gc/heap.h"
+#include "instrumentation.h"
+#include "jit/jit.h"
+#include "jit/jit_code_cache.h"
+#include "jni_env_ext-inl.h"
+#include "jvmti_allocator.h"
+#include "mirror/class.h"
+#include "mirror/class_ext.h"
+#include "mirror/object.h"
+#include "object_lock.h"
+#include "runtime.h"
+#include "ScopedLocalRef.h"
+#include "transform.h"
+
+namespace openjdkjvmti {
+
+bool ClassLoaderHelper::AddToClassLoader(art::Thread* self,
+                                         art::Handle<art::mirror::ClassLoader> loader,
+                                         const art::DexFile* dex_file) {
+  art::StackHandleScope<2> hs(self);
+  art::Handle<art::mirror::Object> java_dex_file_obj(hs.NewHandle(FindSourceDexFileObject(self,
+                                                                                          loader)));
+  if (java_dex_file_obj.IsNull()) {
+    return false;
+  }
+  art::Handle<art::mirror::LongArray> cookie(hs.NewHandle(
+      AllocateNewDexFileCookie(self, java_dex_file_obj, dex_file)));
+  if (cookie.IsNull()) {
+    return false;
+  }
+  art::ScopedAssertNoThreadSuspension nts("Replacing cookie fields in j.l.DexFile object");
+  UpdateJavaDexFile(java_dex_file_obj.Get(), cookie.Get());
+  return true;
+}
+
+void ClassLoaderHelper::UpdateJavaDexFile(art::ObjPtr<art::mirror::Object> java_dex_file,
+                                          art::ObjPtr<art::mirror::LongArray> new_cookie) {
+  art::ArtField* internal_cookie_field = java_dex_file->GetClass()->FindDeclaredInstanceField(
+      "mInternalCookie", "Ljava/lang/Object;");
+  art::ArtField* cookie_field = java_dex_file->GetClass()->FindDeclaredInstanceField(
+      "mCookie", "Ljava/lang/Object;");
+  CHECK(internal_cookie_field != nullptr);
+  art::ObjPtr<art::mirror::LongArray> orig_internal_cookie(
+      internal_cookie_field->GetObject(java_dex_file)->AsLongArray());
+  art::ObjPtr<art::mirror::LongArray> orig_cookie(
+      cookie_field->GetObject(java_dex_file)->AsLongArray());
+  internal_cookie_field->SetObject<false>(java_dex_file, new_cookie);
+  if (!orig_cookie.IsNull()) {
+    cookie_field->SetObject<false>(java_dex_file, new_cookie);
+  }
+}
+
+// TODO Really wishing I had that mirror of java.lang.DexFile now.
+art::ObjPtr<art::mirror::LongArray> ClassLoaderHelper::AllocateNewDexFileCookie(
+    art::Thread* self,
+    art::Handle<art::mirror::Object> java_dex_file_obj,
+    const art::DexFile* dex_file) {
+  art::StackHandleScope<2> hs(self);
+  // mCookie is nulled out if the DexFile has been closed but mInternalCookie sticks around until
+  // the object is finalized. Since they always point to the same array if mCookie is not null we
+  // just use the mInternalCookie field. We will update one or both of these fields later.
+  // TODO Should I get the class from the classloader or directly?
+  art::ArtField* internal_cookie_field = java_dex_file_obj->GetClass()->FindDeclaredInstanceField(
+      "mInternalCookie", "Ljava/lang/Object;");
+  // TODO Add check that mCookie is either null or same as mInternalCookie
+  CHECK(internal_cookie_field != nullptr);
+  art::Handle<art::mirror::LongArray> cookie(
+      hs.NewHandle(internal_cookie_field->GetObject(java_dex_file_obj.Get())->AsLongArray()));
+  // TODO Maybe make these non-fatal.
+  CHECK(cookie.Get() != nullptr);
+  CHECK_GE(cookie->GetLength(), 1);
+  art::Handle<art::mirror::LongArray> new_cookie(
+      hs.NewHandle(art::mirror::LongArray::Alloc(self, cookie->GetLength() + 1)));
+  if (new_cookie.Get() == nullptr) {
+    self->AssertPendingOOMException();
+    return nullptr;
+  }
+  // Copy the oat-dex field at the start.
+  // TODO Should I clear this field?
+  // TODO This is a really crappy thing here with the first element being different.
+  new_cookie->SetWithoutChecks<false>(0, cookie->GetWithoutChecks(0));
+  new_cookie->SetWithoutChecks<false>(
+      1, static_cast<int64_t>(reinterpret_cast<intptr_t>(dex_file)));
+  new_cookie->Memcpy(2, cookie.Get(), 1, cookie->GetLength() - 1);
+  return new_cookie.Get();
+}
+
+// TODO This should return the actual source java.lang.DexFile object for the klass being loaded.
+art::ObjPtr<art::mirror::Object> ClassLoaderHelper::FindSourceDexFileObject(
+    art::Thread* self, art::Handle<art::mirror::ClassLoader> loader) {
+  const char* dex_path_list_element_array_name = "[Ldalvik/system/DexPathList$Element;";
+  const char* dex_path_list_element_name = "Ldalvik/system/DexPathList$Element;";
+  const char* dex_file_name = "Ldalvik/system/DexFile;";
+  const char* dex_path_list_name = "Ldalvik/system/DexPathList;";
+  const char* dex_class_loader_name = "Ldalvik/system/BaseDexClassLoader;";
+
+  CHECK(!self->IsExceptionPending());
+  art::StackHandleScope<5> hs(self);
+  art::ClassLinker* class_linker = art::Runtime::Current()->GetClassLinker();
+
+  art::Handle<art::mirror::ClassLoader> null_loader(hs.NewHandle<art::mirror::ClassLoader>(
+      nullptr));
+  art::Handle<art::mirror::Class> base_dex_loader_class(hs.NewHandle(class_linker->FindClass(
+      self, dex_class_loader_name, null_loader)));
+
+  // Get all the ArtFields so we can look in the BaseDexClassLoader
+  art::ArtField* path_list_field = base_dex_loader_class->FindDeclaredInstanceField(
+      "pathList", dex_path_list_name);
+  CHECK(path_list_field != nullptr);
+
+  art::ArtField* dex_path_list_element_field =
+      class_linker->FindClass(self, dex_path_list_name, null_loader)
+        ->FindDeclaredInstanceField("dexElements", dex_path_list_element_array_name);
+  CHECK(dex_path_list_element_field != nullptr);
+
+  art::ArtField* element_dex_file_field =
+      class_linker->FindClass(self, dex_path_list_element_name, null_loader)
+        ->FindDeclaredInstanceField("dexFile", dex_file_name);
+  CHECK(element_dex_file_field != nullptr);
+
+  // Check if loader is a BaseDexClassLoader
+  art::Handle<art::mirror::Class> loader_class(hs.NewHandle(loader->GetClass()));
+  // Currently only base_dex_loader is allowed to actually define classes but if this changes in the
+  // future we should make sure to support all class loader types.
+  if (!loader_class->IsSubClass(base_dex_loader_class.Get())) {
+    LOG(ERROR) << "The classloader is not a BaseDexClassLoader which is currently the only "
+               << "supported class loader type!";
+    return nullptr;
+  }
+  // Start navigating the fields of the loader (now known to be a BaseDexClassLoader derivative)
+  art::Handle<art::mirror::Object> path_list(
+      hs.NewHandle(path_list_field->GetObject(loader.Get())));
+  CHECK(path_list.Get() != nullptr);
+  CHECK(!self->IsExceptionPending());
+  art::Handle<art::mirror::ObjectArray<art::mirror::Object>> dex_elements_list(hs.NewHandle(
+      dex_path_list_element_field->GetObject(path_list.Get())->
+      AsObjectArray<art::mirror::Object>()));
+  CHECK(!self->IsExceptionPending());
+  CHECK(dex_elements_list.Get() != nullptr);
+  size_t num_elements = dex_elements_list->GetLength();
+  // Iterate over the DexPathList$Element to find the right one
+  for (size_t i = 0; i < num_elements; i++) {
+    art::ObjPtr<art::mirror::Object> current_element = dex_elements_list->Get(i);
+    CHECK(!current_element.IsNull());
+    // TODO It would be cleaner to put the art::DexFile into the dalvik.system.DexFile the class
+    // comes from but it is more annoying because we would need to find this class. It is not
+    // necessary for proper function since we just need to be in front of the classes old dex file
+    // in the path.
+    art::ObjPtr<art::mirror::Object> first_dex_file(
+        element_dex_file_field->GetObject(current_element));
+    if (!first_dex_file.IsNull()) {
+      return first_dex_file;
+    }
+  }
+  return nullptr;
+}
+
+}  // namespace openjdkjvmti
diff --git a/runtime/openjdkjvmti/ti_class_loader.h b/runtime/openjdkjvmti/ti_class_loader.h
new file mode 100644
index 0000000..17ed0eb
--- /dev/null
+++ b/runtime/openjdkjvmti/ti_class_loader.h
@@ -0,0 +1,96 @@
+/* Copyright (C) 2017 The Android Open Source Project
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This file implements interfaces from the file jvmti.h. This implementation
+ * is licensed under the same terms as the file jvmti.h.  The
+ * copyright and license information for the file jvmti.h follows.
+ *
+ * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#ifndef ART_RUNTIME_OPENJDKJVMTI_TI_CLASS_LOADER_H_
+#define ART_RUNTIME_OPENJDKJVMTI_TI_CLASS_LOADER_H_
+
+#include <string>
+
+#include <jni.h>
+
+#include "art_jvmti.h"
+#include "art_method.h"
+#include "base/array_slice.h"
+#include "class_linker.h"
+#include "dex_file.h"
+#include "gc_root-inl.h"
+#include "globals.h"
+#include "jni_env_ext-inl.h"
+#include "jvmti.h"
+#include "linear_alloc.h"
+#include "mem_map.h"
+#include "mirror/array-inl.h"
+#include "mirror/array.h"
+#include "mirror/class-inl.h"
+#include "mirror/class.h"
+#include "mirror/class_loader-inl.h"
+#include "mirror/string-inl.h"
+#include "oat_file.h"
+#include "obj_ptr.h"
+#include "scoped_thread_state_change-inl.h"
+#include "stack.h"
+#include "ti_class_definition.h"
+#include "thread_list.h"
+#include "transform.h"
+#include "utf.h"
+#include "utils/dex_cache_arrays_layout-inl.h"
+
+namespace openjdkjvmti {
+
+// Class that can redefine a single class's methods.
+// TODO We should really make this be driven by an outside class so we can do multiple classes at
+// the same time and have less required cleanup.
+class ClassLoaderHelper {
+ public:
+  static bool AddToClassLoader(art::Thread* self,
+                               art::Handle<art::mirror::ClassLoader> loader,
+                               const art::DexFile* dex_file)
+      REQUIRES_SHARED(art::Locks::mutator_lock_);
+
+  // Finds a java.lang.DexFile object that is associated with the given ClassLoader. Each of these
+  // j.l.DexFile objects holds several art::DexFile*s in it.
+  // TODO This should return the actual source java.lang.DexFile object for the klass being loaded.
+  static art::ObjPtr<art::mirror::Object> FindSourceDexFileObject(
+      art::Thread* self, art::Handle<art::mirror::ClassLoader> loader)
+      REQUIRES_SHARED(art::Locks::mutator_lock_);
+
+  static art::ObjPtr<art::mirror::LongArray> AllocateNewDexFileCookie(
+      art::Thread* self,
+      art::Handle<art::mirror::Object> java_dex_file,
+      const art::DexFile* new_dex_file) REQUIRES_SHARED(art::Locks::mutator_lock_);
+
+  static void UpdateJavaDexFile(art::ObjPtr<art::mirror::Object> java_dex_file,
+                                art::ObjPtr<art::mirror::LongArray> new_cookie)
+      REQUIRES(art::Roles::uninterruptible_) REQUIRES_SHARED(art::Locks::mutator_lock_);
+};
+
+}  // namespace openjdkjvmti
+#endif  // ART_RUNTIME_OPENJDKJVMTI_TI_CLASS_LOADER_H_
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index d2ddc21..b76d74a 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -54,6 +54,7 @@
 #include "object_lock.h"
 #include "runtime.h"
 #include "ScopedLocalRef.h"
+#include "ti_class_loader.h"
 #include "transform.h"
 
 namespace openjdkjvmti {
@@ -386,85 +387,6 @@
   return OK;
 }
 
-// TODO *MAJOR* This should return the actual source java.lang.DexFile object for the klass.
-// TODO Make mirror of DexFile and associated types to make this less hellish.
-// TODO Make mirror of BaseDexClassLoader and associated types to make this less hellish.
-art::mirror::Object* Redefiner::ClassRedefinition::FindSourceDexFileObject(
-    art::Handle<art::mirror::ClassLoader> loader) {
-  const char* dex_path_list_element_array_name = "[Ldalvik/system/DexPathList$Element;";
-  const char* dex_path_list_element_name = "Ldalvik/system/DexPathList$Element;";
-  const char* dex_file_name = "Ldalvik/system/DexFile;";
-  const char* dex_path_list_name = "Ldalvik/system/DexPathList;";
-  const char* dex_class_loader_name = "Ldalvik/system/BaseDexClassLoader;";
-
-  CHECK(!driver_->self_->IsExceptionPending());
-  art::StackHandleScope<11> hs(driver_->self_);
-  art::ClassLinker* class_linker = driver_->runtime_->GetClassLinker();
-
-  art::Handle<art::mirror::ClassLoader> null_loader(hs.NewHandle<art::mirror::ClassLoader>(
-      nullptr));
-  art::Handle<art::mirror::Class> base_dex_loader_class(hs.NewHandle(class_linker->FindClass(
-      driver_->self_, dex_class_loader_name, null_loader)));
-
-  // Get all the ArtFields so we can look in the BaseDexClassLoader
-  art::ArtField* path_list_field = base_dex_loader_class->FindDeclaredInstanceField(
-      "pathList", dex_path_list_name);
-  CHECK(path_list_field != nullptr);
-
-  art::ArtField* dex_path_list_element_field =
-      class_linker->FindClass(driver_->self_, dex_path_list_name, null_loader)
-        ->FindDeclaredInstanceField("dexElements", dex_path_list_element_array_name);
-  CHECK(dex_path_list_element_field != nullptr);
-
-  art::ArtField* element_dex_file_field =
-      class_linker->FindClass(driver_->self_, dex_path_list_element_name, null_loader)
-        ->FindDeclaredInstanceField("dexFile", dex_file_name);
-  CHECK(element_dex_file_field != nullptr);
-
-  // Check if loader is a BaseDexClassLoader
-  art::Handle<art::mirror::Class> loader_class(hs.NewHandle(loader->GetClass()));
-  if (!loader_class->IsSubClass(base_dex_loader_class.Get())) {
-    LOG(ERROR) << "The classloader is not a BaseDexClassLoader which is currently the only "
-               << "supported class loader type!";
-    return nullptr;
-  }
-  // Start navigating the fields of the loader (now known to be a BaseDexClassLoader derivative)
-  art::Handle<art::mirror::Object> path_list(
-      hs.NewHandle(path_list_field->GetObject(loader.Get())));
-  CHECK(path_list.Get() != nullptr);
-  CHECK(!driver_->self_->IsExceptionPending());
-  art::Handle<art::mirror::ObjectArray<art::mirror::Object>> dex_elements_list(hs.NewHandle(
-      dex_path_list_element_field->GetObject(path_list.Get())->
-      AsObjectArray<art::mirror::Object>()));
-  CHECK(!driver_->self_->IsExceptionPending());
-  CHECK(dex_elements_list.Get() != nullptr);
-  size_t num_elements = dex_elements_list->GetLength();
-  art::MutableHandle<art::mirror::Object> current_element(
-      hs.NewHandle<art::mirror::Object>(nullptr));
-  art::MutableHandle<art::mirror::Object> first_dex_file(
-      hs.NewHandle<art::mirror::Object>(nullptr));
-  // Iterate over the DexPathList$Element to find the right one
-  // TODO Or not ATM just return the first one.
-  for (size_t i = 0; i < num_elements; i++) {
-    current_element.Assign(dex_elements_list->Get(i));
-    CHECK(current_element.Get() != nullptr);
-    CHECK(!driver_->self_->IsExceptionPending());
-    CHECK(dex_elements_list.Get() != nullptr);
-    CHECK_EQ(current_element->GetClass(), class_linker->FindClass(driver_->self_,
-                                                                  dex_path_list_element_name,
-                                                                  null_loader));
-    // TODO It would be cleaner to put the art::DexFile into the dalvik.system.DexFile the class
-    // comes from but it is more annoying because we would need to find this class. It is not
-    // necessary for proper function since we just need to be in front of the classes old dex file
-    // in the path.
-    first_dex_file.Assign(element_dex_file_field->GetObject(current_element.Get()));
-    if (first_dex_file.Get() != nullptr) {
-      return first_dex_file.Get();
-    }
-  }
-  return nullptr;
-}
-
 art::mirror::Class* Redefiner::ClassRedefinition::GetMirrorClass() {
   return driver_->self_->DecodeJObject(klass_)->AsClass();
 }
@@ -478,39 +400,6 @@
   return driver_->runtime_->GetClassLinker()->RegisterDexFile(*dex_file_, loader.Get());
 }
 
-// TODO Really wishing I had that mirror of java.lang.DexFile now.
-art::mirror::LongArray* Redefiner::ClassRedefinition::AllocateDexFileCookie(
-    art::Handle<art::mirror::Object> java_dex_file_obj) {
-  art::StackHandleScope<2> hs(driver_->self_);
-  // mCookie is nulled out if the DexFile has been closed but mInternalCookie sticks around until
-  // the object is finalized. Since they always point to the same array if mCookie is not null we
-  // just use the mInternalCookie field. We will update one or both of these fields later.
-  // TODO Should I get the class from the classloader or directly?
-  art::ArtField* internal_cookie_field = java_dex_file_obj->GetClass()->FindDeclaredInstanceField(
-      "mInternalCookie", "Ljava/lang/Object;");
-  // TODO Add check that mCookie is either null or same as mInternalCookie
-  CHECK(internal_cookie_field != nullptr);
-  art::Handle<art::mirror::LongArray> cookie(
-      hs.NewHandle(internal_cookie_field->GetObject(java_dex_file_obj.Get())->AsLongArray()));
-  // TODO Maybe make these non-fatal.
-  CHECK(cookie.Get() != nullptr);
-  CHECK_GE(cookie->GetLength(), 1);
-  art::Handle<art::mirror::LongArray> new_cookie(
-      hs.NewHandle(art::mirror::LongArray::Alloc(driver_->self_, cookie->GetLength() + 1)));
-  if (new_cookie.Get() == nullptr) {
-    driver_->self_->AssertPendingOOMException();
-    return nullptr;
-  }
-  // Copy the oat-dex field at the start.
-  // TODO Should I clear this field?
-  // TODO This is a really crappy thing here with the first element being different.
-  new_cookie->SetWithoutChecks<false>(0, cookie->GetWithoutChecks(0));
-  new_cookie->SetWithoutChecks<false>(
-      1, static_cast<int64_t>(reinterpret_cast<intptr_t>(dex_file_.get())));
-  new_cookie->Memcpy(2, cookie.Get(), 1, cookie->GetLength() - 1);
-  return new_cookie.Get();
-}
-
 void Redefiner::RecordFailure(jvmtiError result,
                               const std::string& class_sig,
                               const std::string& error_msg) {
@@ -854,14 +743,18 @@
     RecordFailure(ERR(INTERNAL), "Unable to find class loader!");
     return false;
   }
-  art::Handle<art::mirror::Object> dex_file_obj(hs.NewHandle(FindSourceDexFileObject(loader)));
+  art::Handle<art::mirror::Object> dex_file_obj(hs.NewHandle(
+      ClassLoaderHelper::FindSourceDexFileObject(driver_->self_, loader)));
   holder->SetJavaDexFile(klass_index, dex_file_obj.Get());
   if (dex_file_obj.Get() == nullptr) {
     // TODO Better error msg.
     RecordFailure(ERR(INTERNAL), "Unable to find class loader!");
     return false;
   }
-  holder->SetNewDexFileCookie(klass_index, AllocateDexFileCookie(dex_file_obj));
+  holder->SetNewDexFileCookie(klass_index,
+                              ClassLoaderHelper::AllocateNewDexFileCookie(driver_->self_,
+                                                                          dex_file_obj,
+                                                                          dex_file_.get()).Ptr());
   if (holder->GetNewDexFileCookie(klass_index) == nullptr) {
     driver_->self_->AssertPendingOOMException();
     driver_->self_->ClearException();
@@ -973,8 +866,10 @@
   // TODO We need to decide on & implement semantics for JNI jmethodids when we redefine methods.
   int32_t cnt = 0;
   for (Redefiner::ClassRedefinition& redef : redefinitions_) {
+    art::ScopedAssertNoThreadSuspension nts("Updating runtime objects for redefinition");
     art::mirror::Class* klass = holder.GetMirrorClass(cnt);
-    redef.UpdateJavaDexFile(holder.GetJavaDexFile(cnt), holder.GetNewDexFileCookie(cnt));
+    ClassLoaderHelper::UpdateJavaDexFile(holder.GetJavaDexFile(cnt),
+                                         holder.GetNewDexFileCookie(cnt));
     // TODO Rewrite so we don't do a stack walk for each and every class.
     redef.FindAndAllocateObsoleteMethods(klass);
     redef.UpdateClass(klass, holder.GetNewDexCache(cnt), holder.GetOriginalDexFileBytes(cnt));
@@ -1090,24 +985,6 @@
   ext->SetOriginalDexFileBytes(original_dex_file);
 }
 
-void Redefiner::ClassRedefinition::UpdateJavaDexFile(
-    art::ObjPtr<art::mirror::Object> java_dex_file,
-    art::ObjPtr<art::mirror::LongArray> new_cookie) {
-  art::ArtField* internal_cookie_field = java_dex_file->GetClass()->FindDeclaredInstanceField(
-      "mInternalCookie", "Ljava/lang/Object;");
-  art::ArtField* cookie_field = java_dex_file->GetClass()->FindDeclaredInstanceField(
-      "mCookie", "Ljava/lang/Object;");
-  CHECK(internal_cookie_field != nullptr);
-  art::ObjPtr<art::mirror::LongArray> orig_internal_cookie(
-      internal_cookie_field->GetObject(java_dex_file)->AsLongArray());
-  art::ObjPtr<art::mirror::LongArray> orig_cookie(
-      cookie_field->GetObject(java_dex_file)->AsLongArray());
-  internal_cookie_field->SetObject<false>(java_dex_file, new_cookie);
-  if (!orig_cookie.IsNull()) {
-    cookie_field->SetObject<false>(java_dex_file, new_cookie);
-  }
-}
-
 // This function does all (java) allocations we need to do for the Class being redefined.
 // TODO Change this name maybe?
 bool Redefiner::ClassRedefinition::EnsureClassAllocationsFinished() {
diff --git a/runtime/openjdkjvmti/ti_redefine.h b/runtime/openjdkjvmti/ti_redefine.h
index fdc13ee..85df6e1 100644
--- a/runtime/openjdkjvmti/ti_redefine.h
+++ b/runtime/openjdkjvmti/ti_redefine.h
@@ -127,18 +127,9 @@
     art::mirror::Class* GetMirrorClass() REQUIRES_SHARED(art::Locks::mutator_lock_);
     art::mirror::ClassLoader* GetClassLoader() REQUIRES_SHARED(art::Locks::mutator_lock_);
 
-    // This finds the java.lang.DexFile we will add the native DexFile to as part of the classpath.
-    // TODO Make sure the DexFile object returned is the one that the klass_ actually comes from.
-    art::mirror::Object* FindSourceDexFileObject(art::Handle<art::mirror::ClassLoader> loader)
-        REQUIRES_SHARED(art::Locks::mutator_lock_);
-
     art::mirror::DexCache* CreateNewDexCache(art::Handle<art::mirror::ClassLoader> loader)
         REQUIRES_SHARED(art::Locks::mutator_lock_);
 
-    // Allocates and fills the new DexFileCookie
-    art::mirror::LongArray* AllocateDexFileCookie(art::Handle<art::mirror::Object> j_dex_file_obj)
-        REQUIRES_SHARED(art::Locks::mutator_lock_);
-
     // This may return nullptr with a OOME pending if allocation fails.
     art::mirror::ByteArray* AllocateOrGetOriginalDexFileBytes()
         REQUIRES_SHARED(art::Locks::mutator_lock_);
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index afd1998..6130a8b 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -521,13 +521,10 @@
 # Known broken tests for the interpreter.
 # CFI unwinding expects managed frames.
 # 629 requires compilation.
-# 934 and 935 are broken due to the PreDefine hook not yet inserting them into the classpath. This should be fixed shortly
 TEST_ART_BROKEN_INTERPRETER_RUN_TESTS := \
   137-cfi \
   554-jit-profile-file \
-  629-vdex-speed \
-  934-load-transform \
-  935-non-retransformable \
+  629-vdex-speed
 
 ifneq (,$(filter interpreter,$(COMPILER_TYPES)))
   ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \
@@ -550,7 +547,6 @@
 # resolved but until then just disable them. Test 916 already checks this
 # feature for JIT use cases in a way that is resilient to the jit frames.
 # 912: b/34655682
-# 934 and 935 are broken due to the PreDefine hook not yet inserting them into the classpath. This should be fixed shortly
 TEST_ART_BROKEN_JIT_RUN_TESTS := \
   137-cfi \
   629-vdex-speed \
@@ -563,8 +559,6 @@
   917-fields-transformation \
   919-obsolete-fields \
   926-multi-obsolescence \
-  934-load-transform \
-  935-non-retransformable \
 
 ifneq (,$(filter jit,$(COMPILER_TYPES)))
   ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \