[Android] Manage CVC's retained JS object set on the native side.

Move management of the js retained object set to the native java
bridge.

BUG=169228


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177765
Reverted in https://chromiumcodereview.appspot.com/11817047

Review URL: https://chromiumcodereview.appspot.com/11817047

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@177948 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 239 insertions, 34 deletions

diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index c7ebc41..b122f2b 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -1070,15 +1070,21 @@ void ContentViewCoreImpl::AddJavascriptInterface(
     jobject /* obj */,
     jobject object,
     jstring name,
-    jclass safe_annotation_clazz) {
+    jclass safe_annotation_clazz,
+    jobject retained_object_set) {
   ScopedJavaLocalRef<jobject> scoped_object(env, object);
   ScopedJavaLocalRef<jclass> scoped_clazz(env, safe_annotation_clazz);
+  JavaObjectWeakGlobalRef weak_retained_object_set(env, retained_object_set);
 
   // JavaBoundObject creates the NPObject with a ref count of 1, and
   // JavaBridgeDispatcherHostManager takes its own ref.
-  NPObject* bound_object = JavaBoundObject::Create(scoped_object, scoped_clazz);
-  web_contents_->java_bridge_dispatcher_host_manager()->AddNamedObject(
-      ConvertJavaStringToUTF16(env, name), bound_object);
+  JavaBridgeDispatcherHostManager* java_bridge =
+      web_contents_->java_bridge_dispatcher_host_manager();
+  java_bridge->SetRetainedObjectSet(weak_retained_object_set);
+  NPObject* bound_object = JavaBoundObject::Create(scoped_object, scoped_clazz,
+                                                   java_bridge->AsWeakPtr());
+  java_bridge->AddNamedObject(ConvertJavaStringToUTF16(env, name),
+                              bound_object);
   WebKit::WebBindings::releaseObject(bound_object);
 }
 
diff --git a/content/browser/android/content_view_core_impl.h b/content/browser/android/content_view_core_impl.h
index 96a4601..1c0efce 100644
--- a/content/browser/android/content_view_core_impl.h
+++ b/content/browser/android/content_view_core_impl.h
@@ -189,7 +189,8 @@ class ContentViewCoreImpl : public ContentViewCore,
                               jobject obj,
                               jobject object,
                               jstring name,
-                              jclass safe_annotation_clazz);
+                              jclass safe_annotation_clazz,
+                              jobject retained_object_set);
   void RemoveJavascriptInterface(JNIEnv* env, jobject obj, jstring name);
   int GetNavigationHistory(JNIEnv* env, jobject obj, jobject context);
   void GetDirectedNavigationHistory(JNIEnv* env,
diff --git a/content/browser/android/web_contents_observer_android.h b/content/browser/android/web_contents_observer_android.h
index ee988ad..c81baba 100644
--- a/content/browser/android/web_contents_observer_android.h
+++ b/content/browser/android/web_contents_observer_android.h
@@ -9,6 +9,7 @@
 
 #include "base/android/jni_helper.h"
 #include "base/basictypes.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/frame_navigate_params.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "googleurl/src/gurl.h"
diff --git a/content/browser/renderer_host/java/java_bound_object.cc b/content/browser/renderer_host/java/java_bound_object.cc
index f8b8a75..9d37dc3 100644
--- a/content/browser/renderer_host/java/java_bound_object.cc
+++ b/content/browser/renderer_host/java/java_bound_object.cc
@@ -9,7 +9,9 @@
 #include "base/memory/singleton.h"
 #include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
+#include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
 #include "content/browser/renderer_host/java/java_type.h"
+#include "content/public/browser/browser_thread.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h"
 
 using base::StringPrintf;
@@ -123,9 +125,14 @@ bool JavaNPObject::GetProperty(NPObject* np_object,
 // method returns true and the Java method's return value is provided as an
 // NPVariant. Note that this method does not do any type coercion. The Java
 // return value is simply converted to the corresponding NPAPI type.
-bool CallJNIMethod(jobject object, const JavaType& return_type, jmethodID id,
-                   jvalue* parameters, NPVariant* result,
-                   base::android::JavaRef<jclass>& safe_annotation_clazz) {
+bool CallJNIMethod(
+    jobject object,
+    const JavaType& return_type,
+    jmethodID id,
+    jvalue* parameters,
+    NPVariant* result,
+    const JavaRef<jclass>& safe_annotation_clazz,
+    const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager) {
   JNIEnv* env = AttachCurrentThread();
   switch (return_type.type) {
     case JavaType::TypeBoolean:
@@ -209,7 +216,8 @@ bool CallJNIMethod(jobject object, const JavaType& return_type, jmethodID id,
         break;
       }
       OBJECT_TO_NPVARIANT(JavaBoundObject::Create(scoped_java_object,
-                                                  safe_annotation_clazz),
+                                                  safe_annotation_clazz,
+                                                  manager),
                           *result);
       break;
     }
@@ -727,7 +735,8 @@ jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
 
 NPObject* JavaBoundObject::Create(
     const JavaRef<jobject>& object,
-    base::android::JavaRef<jclass>& safe_annotation_clazz) {
+    const JavaRef<jclass>& safe_annotation_clazz,
+    const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager) {
   // The first argument (a plugin's instance handle) is passed through to the
   // allocate function directly, and we don't use it, so it's ok to be 0.
   // The object is created with a ref count of one.
@@ -735,21 +744,36 @@ NPObject* JavaBoundObject::Create(
       &JavaNPObject::kNPClass));
   // The NPObject takes ownership of the JavaBoundObject.
   reinterpret_cast<JavaNPObject*>(np_object)->bound_object =
-      new JavaBoundObject(object, safe_annotation_clazz);
+      new JavaBoundObject(object, safe_annotation_clazz, manager);
   return np_object;
 }
 
 JavaBoundObject::JavaBoundObject(
     const JavaRef<jobject>& object,
-    base::android::JavaRef<jclass>& safe_annotation_clazz)
+    const JavaRef<jclass>& safe_annotation_clazz,
+    const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager)
     : java_object_(AttachCurrentThread(), object.obj()),
+      manager_(manager),
       are_methods_set_up_(false),
       safe_annotation_clazz_(safe_annotation_clazz) {
-  // We don't do anything with our Java object when first created. We do it all
-  // lazily when a method is first invoked.
+  BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&JavaBridgeDispatcherHostManager::JavaBoundObjectCreated,
+                   manager_,
+                   base::android::ScopedJavaGlobalRef<jobject>(object)));
+  // Other than informing the JavaBridgeDispatcherHostManager that a java bound
+  // object has been created (above), we don't do anything else with our Java
+  // object when first created. We do it all lazily when a method is first
+  // invoked.
 }
 
 JavaBoundObject::~JavaBoundObject() {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&JavaBridgeDispatcherHostManager::JavaBoundObjectDestroyed,
+                 manager_,
+                 base::android::ScopedJavaGlobalRef<jobject>(
+                     java_object_.get(AttachCurrentThread()))));
 }
 
 ScopedJavaLocalRef<jobject> JavaBoundObject::GetJavaObject(NPObject* object) {
@@ -802,7 +826,8 @@ bool JavaBoundObject::Invoke(const std::string& name, const NPVariant* args,
     // Call
     ok = CallJNIMethod(obj.obj(), method->return_type(),
                        method->id(), &parameters[0], result,
-                       safe_annotation_clazz_);
+                       safe_annotation_clazz_,
+                       manager_);
   }
 
   // Now that we're done with the jvalue, release any local references created
diff --git a/content/browser/renderer_host/java/java_bound_object.h b/content/browser/renderer_host/java/java_bound_object.h
index 56cce29..ff97fdb 100644
--- a/content/browser/renderer_host/java/java_bound_object.h
+++ b/content/browser/renderer_host/java/java_bound_object.h
@@ -12,11 +12,14 @@
 #include "base/android/jni_helper.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/memory/linked_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "content/browser/renderer_host/java/java_method.h"
 #include "third_party/npapi/bindings/npruntime.h"
 
 namespace content {
 
+class JavaBridgeDispatcherHostManager;
+
 // Wrapper around a Java object.
 //
 // Represents a Java object for use in the Java bridge. Holds a global ref to
@@ -32,9 +35,11 @@ class JavaBoundObject {
   // propagates to all Objects that get implicitly exposed as return values as
   // well. Returns an NPObject with a ref count of one which owns the
   // JavaBoundObject.
+  // See also comment below for |manager_|.
   static NPObject* Create(
       const base::android::JavaRef<jobject>& object,
-      base::android::JavaRef<jclass>& safe_annotation_clazz);
+      const base::android::JavaRef<jclass>& safe_annotation_clazz,
+      const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager);
 
   virtual ~JavaBoundObject();
 
@@ -52,13 +57,21 @@ class JavaBoundObject {
  private:
   explicit JavaBoundObject(
       const base::android::JavaRef<jobject>& object,
-      base::android::JavaRef<jclass>& safe_annotation_clazz);
+      const base::android::JavaRef<jclass>& safe_annotation_clazz,
+      const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager_);
 
   void EnsureMethodsAreSetUp() const;
 
   // The weak ref to the underlying Java object that this JavaBoundObject
   // instance represents.
   JavaObjectWeakGlobalRef java_object_;
+
+  // Keep a pointer back to the JavaBridgeDispatcherHostManager so that we
+  // can notify it when this JavaBoundObject is destroyed. JavaBoundObjects
+  // may outlive the manager so keep a WeakPtr. Note the WeakPtr may only be
+  // dereferenced on the UI thread.
+  base::WeakPtr<JavaBridgeDispatcherHostManager> manager_;
+
   // Map of public methods, from method name to Method instance. Multiple
   // entries will be present for overloaded methods. Note that we can't use
   // scoped_ptr in STL containers as we can't copy it.
diff --git a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc
index 0448132..c6c256b 100644
--- a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc
+++ b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc
@@ -4,9 +4,16 @@
 
 #include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
 
+#include "base/android/jni_android.h"
+#include "base/android/jni_helper.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
+#include "content/browser/renderer_host/java/java_bound_object.h"
 #include "content/browser/renderer_host/java/java_bridge_dispatcher_host.h"
+#include "content/common/android/hash_set.h"
+#include "content/public/browser/browser_thread.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h"
 
 namespace content {
@@ -25,7 +32,7 @@ JavaBridgeDispatcherHostManager::~JavaBridgeDispatcherHostManager() {
 }
 
 void JavaBridgeDispatcherHostManager::AddNamedObject(const string16& name,
-    NPObject* object) {
+                                                     NPObject* object) {
   // Record this object in a map so that we can add it into RenderViewHosts
   // created later. The JavaBridgeDispatcherHost instances will take a
   // reference to the object, but we take one too, because this method can be
@@ -39,6 +46,27 @@ void JavaBridgeDispatcherHostManager::AddNamedObject(const string16& name,
   }
 }
 
+void JavaBridgeDispatcherHostManager::SetRetainedObjectSet(
+    const JavaObjectWeakGlobalRef& retained_object_set) {
+  // It's an error to replace the retained_object_set_ after it's been set,
+  // so we check that it hasn't already been here.
+  // TODO(benm): It'd be better to pass the set in the constructor to avoid
+  // the chance of this happening; but that's tricky as this get's constructed
+  // before ContentViewCore (which owns the set). Best solution may be to move
+  // ownership of the JavaBridgerDispatchHostManager from WebContents to
+  // ContentViewCore?
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> new_retained_object_set =
+      retained_object_set.get(env);
+  base::android::ScopedJavaLocalRef<jobject> current_retained_object_set =
+      retained_object_set_.get(env);
+  if (!env->IsSameObject(new_retained_object_set.obj(),
+                         current_retained_object_set.obj())) {
+    DCHECK(current_retained_object_set.is_null());
+    retained_object_set_ = retained_object_set;
+  }
+}
+
 void JavaBridgeDispatcherHostManager::RemoveNamedObject(const string16& name) {
   ObjectMap::iterator iter = objects_.find(name);
   if (iter == objects_.end()) {
@@ -82,4 +110,47 @@ void JavaBridgeDispatcherHostManager::WebContentsDestroyed(
   instances_.clear();
 }
 
+void JavaBridgeDispatcherHostManager::DocumentAvailableInMainFrame() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // Called when the window object has been cleared in the main frame.
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> retained_object_set =
+      retained_object_set_.get(env);
+  if (!retained_object_set.is_null()) {
+    JNI_Java_HashSet_clear(env, retained_object_set);
+
+    // We also need to add back the named objects we have so far as they
+    // should survive navigations.
+    ObjectMap::iterator it = objects_.begin();
+    for (; it != objects_.end(); ++it) {
+      JNI_Java_HashSet_add(env, retained_object_set,
+                           JavaBoundObject::GetJavaObject(it->second));
+    }
+  }
+}
+
+void JavaBridgeDispatcherHostManager::JavaBoundObjectCreated(
+    const base::android::JavaRef<jobject>& object) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> retained_object_set =
+      retained_object_set_.get(env);
+  if (!retained_object_set.is_null()) {
+    JNI_Java_HashSet_add(env, retained_object_set, object);
+  }
+}
+
+void JavaBridgeDispatcherHostManager::JavaBoundObjectDestroyed(
+    const base::android::JavaRef<jobject>& object) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jobject> retained_object_set =
+      retained_object_set_.get(env);
+  if (!retained_object_set.is_null()) {
+    JNI_Java_HashSet_remove(env, retained_object_set, object);
+  }
+}
+
 }  // namespace content
diff --git a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h
index 0132cad..b485ae0 100644
--- a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h
+++ b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h
@@ -7,8 +7,11 @@
 
 #include <map>
 
+#include "base/android/jni_helper.h"
+#include "base/android/scoped_java_ref.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/string16.h"
 #include "content/public/browser/web_contents_observer.h"
 
@@ -21,7 +24,9 @@ class RenderViewHost;
 // This class handles injecting Java objects into all of the RenderViews
 // associated with a WebContents. It manages a set of JavaBridgeDispatcherHost
 // objects, one per RenderViewHost.
-class JavaBridgeDispatcherHostManager : public WebContentsObserver {
+class JavaBridgeDispatcherHostManager
+    : public WebContentsObserver,
+      public base::SupportsWeakPtr<JavaBridgeDispatcherHostManager> {
  public:
   explicit JavaBridgeDispatcherHostManager(WebContents* web_contents);
   virtual ~JavaBridgeDispatcherHostManager();
@@ -32,10 +37,22 @@ class JavaBridgeDispatcherHostManager : public WebContentsObserver {
   void AddNamedObject(const string16& name, NPObject* object);
   void RemoveNamedObject(const string16& name);
 
+  // Every time a JavaBoundObject backed by a real Java object is
+  // created/destroyed, we insert/remove a strong ref to that Java object into
+  // this set so that it doesn't get garbage collected while it's still
+  // potentially in use. Although the set is managed native side, it's owned
+  // and defined in Java so that pushing refs into it does not create new GC
+  // roots that would prevent ContentViewCore from being garbage collected.
+  void SetRetainedObjectSet(const JavaObjectWeakGlobalRef& retained_object_set);
+
   // WebContentsObserver overrides
   virtual void RenderViewCreated(RenderViewHost* render_view_host) OVERRIDE;
   virtual void RenderViewDeleted(RenderViewHost* render_view_host) OVERRIDE;
   virtual void WebContentsDestroyed(WebContents* web_contents) OVERRIDE;
+  virtual void DocumentAvailableInMainFrame() OVERRIDE;
+
+  void JavaBoundObjectCreated(const base::android::JavaRef<jobject>& object);
+  void JavaBoundObjectDestroyed(const base::android::JavaRef<jobject>& object);
 
  private:
   typedef std::map<RenderViewHost*, scoped_refptr<JavaBridgeDispatcherHost> >
@@ -43,6 +60,7 @@ class JavaBridgeDispatcherHostManager : public WebContentsObserver {
   InstanceMap instances_;
   typedef std::map<string16, NPObject*> ObjectMap;
   ObjectMap objects_;
+  JavaObjectWeakGlobalRef retained_object_set_;
 
   DISALLOW_COPY_AND_ASSIGN(JavaBridgeDispatcherHostManager);
 };
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 43e0215..545ebf3 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -100,6 +100,10 @@
 #include "ui/base/touch/touch_factory.h"
 #endif // defined (USE_AURA) && defined(USE_X11)
 
+#if defined(ENABLE_JAVA_BRIDGE)
+#include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
+#endif
+
 // Cross-Site Navigations
 //
 // If a WebContentsImpl is told to navigate to a different web site (as
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 78c03ae..e3f6ac9 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -14,7 +14,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/process.h"
-#include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_widget_host_delegate.h"
 #include "content/browser/web_contents/navigation_controller_impl.h"
@@ -46,6 +45,7 @@ class ColorChooser;
 class DateTimeChooserAndroid;
 class DownloadItem;
 class InterstitialPageImpl;
+class JavaBridgeDispatcherHostManager;
 class JavaScriptDialogCreator;
 class RenderViewHost;
 class RenderViewHostDelegateView;
@@ -159,9 +159,11 @@ class CONTENT_EXPORT WebContentsImpl
     opener_web_ui_type_ = opener_web_ui_type;
   }
 
+#if defined(ENABLE_JAVA_BRIDGE)
   JavaBridgeDispatcherHostManager* java_bridge_dispatcher_host_manager() const {
     return java_bridge_dispatcher_host_manager_.get();
   }
+#endif
 
   // Expose the render manager for testing.
   RenderViewHostManager* GetRenderManagerForTesting();
@@ -720,10 +722,12 @@ class CONTENT_EXPORT WebContentsImpl
   // Manages creation and swapping of render views.
   RenderViewHostManager render_manager_;
 
+#if defined(ENABLE_JAVA_BRIDGE)
   // Manages injecting Java objects into all RenderViewHosts associated with
   // this WebContentsImpl.
   scoped_ptr<JavaBridgeDispatcherHostManager>
       java_bridge_dispatcher_host_manager_;
+#endif
 
   // SavePackage, lazily created.
   scoped_refptr<SavePackage> save_package_;
diff --git a/content/common/android/common_jni_registrar.cc b/content/common/android/common_jni_registrar.cc
index 9b206cd..d60e9dc 100644
--- a/content/common/android/common_jni_registrar.cc
+++ b/content/common/android/common_jni_registrar.cc
@@ -8,6 +8,7 @@
 #include "base/android/jni_registrar.h"
 #include "content/common/android/command_line.h"
 #include "content/common/android/device_info.h"
+#include "content/common/android/hash_set.h"
 #include "content/common/android/surface_callback.h"
 #include "content/common/android/surface_texture_listener.h"
 #include "content/common/android/trace_event_binding.h"
@@ -16,6 +17,7 @@ namespace {
 base::android::RegistrationMethod kContentRegisteredMethods[] = {
   { "CommandLine", RegisterCommandLine },
   { "DeviceInfo", content::RegisterDeviceInfo },
+  { "HashSet", content::RegisterHashSet },
   { "SurfaceCallback", content::RegisterSurfaceCallback },
   { "SurfaceTextureListener",
         content::SurfaceTextureListener::RegisterSurfaceTextureListener },
diff --git a/content/common/android/hash_set.cc b/content/common/android/hash_set.cc
new file mode 100644
index 0000000..bd73b77
--- /dev/null
+++ b/content/common/android/hash_set.cc
@@ -0,0 +1,30 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "jni/HashSet_jni.h"
+
+namespace content {
+
+bool RegisterHashSet(JNIEnv* env) {
+  return JNI_HashSet::RegisterNativesImpl(env);
+}
+
+void JNI_Java_HashSet_add(JNIEnv* env,
+                          const base::android::JavaRef<jobject>& hash_set,
+                          const base::android::JavaRef<jobject>& object) {
+  JNI_HashSet::Java_HashSet_add(env, hash_set.obj(), object.obj());
+}
+
+void JNI_Java_HashSet_remove(JNIEnv* env,
+                             const base::android::JavaRef<jobject>& hash_set,
+                             const base::android::JavaRef<jobject>& object) {
+  JNI_HashSet::Java_HashSet_remove(env, hash_set.obj(), object.obj());
+}
+
+void JNI_Java_HashSet_clear(JNIEnv* env,
+                            const base::android::JavaRef<jobject>& hash_set) {
+  JNI_HashSet::Java_HashSet_clear(env, hash_set.obj());
+}
+
+}  // namespace content
diff --git a/content/common/android/hash_set.h b/content/common/android/hash_set.h
new file mode 100644
index 0000000..b7a5870
--- /dev/null
+++ b/content/common/android/hash_set.h
@@ -0,0 +1,24 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <jni.h>
+
+#include "base/android/scoped_java_ref.h"
+
+namespace content {
+
+bool RegisterHashSet(JNIEnv* env);
+
+void JNI_Java_HashSet_add(JNIEnv* env,
+                          const base::android::JavaRef<jobject>& hash_set,
+                          const base::android::JavaRef<jobject>& object);
+
+void JNI_Java_HashSet_remove(JNIEnv* env,
+                             const base::android::JavaRef<jobject>& hash_set,
+                             const base::android::JavaRef<jobject>& object);
+
+void JNI_Java_HashSet_clear(JNIEnv* env,
+                            const base::android::JavaRef<jobject>& hash_set);
+
+}  // namespace content
diff --git a/content/content.gyp b/content/content.gyp
index 853d17a..31b8335 100644
--- a/content/content.gyp
+++ b/content/content.gyp
@@ -348,9 +348,21 @@
           'includes': [ '../build/jar_file_jni_generator.gypi' ],
         },
         {
+          'target_name': 'java_set_jni_headers',
+          'type': 'none',
+          'variables': {
+            'jni_gen_dir': 'content',
+            'input_java_class': 'java/util/HashSet.class',
+            'input_jar_file': '<(android_sdk)/android.jar',
+          },
+          'includes': [ '../build/jar_file_jni_generator.gypi' ],
+        },
+
+        {
           'target_name': 'content_jni_headers',
           'type': 'none',
           'dependencies': [
+            'java_set_jni_headers',
             'surface_texture_jni_headers',
             'surface_jni_headers',
           ],
diff --git a/content/content_common.gypi b/content/content_common.gypi
index b2c4953..ff32dd8 100644
--- a/content/content_common.gypi
+++ b/content/content_common.gypi
@@ -113,6 +113,8 @@
     'common/android/common_jni_registrar.h',
     'common/android/device_info.cc',
     'common/android/device_info.h',
+    'common/android/hash_set.cc',
+    'common/android/hash_set.h',
     'common/android/surface_callback.cc',
     'common/android/surface_callback.h',
     'common/android/surface_texture_bridge.cc',
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index 58c3ac9..c750121 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -52,7 +52,6 @@ import org.chromium.ui.gfx.NativeWindow;
 import java.lang.annotation.Annotation;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 import java.util.HashSet;
 
 /**
@@ -100,10 +99,9 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
     // current page to ensure that they are not garbage collected until the page is
     // navigated. This includes interface objects that have been removed
     // via the removeJavaScriptInterface API and transient objects returned from methods
-    // on the interface object.
-    // TODO(benm): Implement the transient object retention - likely by moving the
-    // management of this set into the native Java bridge. (crbug/169228) (crbug/169228)
-    private Set<Object> mRetainedJavaScriptObjects = new HashSet<Object>();
+    // on the interface object. Note we use HashSet rather than Set as the native side
+    // expects HashSet (no bindings for interfaces).
+    private HashSet<Object> mRetainedJavaScriptObjects = new HashSet<Object>();
 
     /**
      * Interface that consumers of {@link ContentViewCore} must implement to allow the proper
@@ -532,10 +530,6 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
             public void didStartLoading(String url) {
                 hidePopupDialog();
                 resetGestureDetectors();
-                // TODO(benm): This isn't quite the right place to do this. Management
-                // of this set should be moving into the native java bridge in crbug/169228
-                // and until that's ready this will do.
-                mRetainedJavaScriptObjects.clear();
             }
         };
     }
@@ -2258,7 +2252,8 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
             Class<? extends Annotation> requiredAnnotation) {
         if (mNativeContentViewCore != 0 && object != null) {
             mJavaScriptInterfaces.put(name, object);
-            nativeAddJavascriptInterface(mNativeContentViewCore, object, name, requiredAnnotation);
+            nativeAddJavascriptInterface(mNativeContentViewCore, object, name, requiredAnnotation,
+                    mRetainedJavaScriptObjects);
         }
     }
 
@@ -2268,9 +2263,6 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
      * @param name The name of the interface to remove.
      */
     public void removeJavascriptInterface(String name) {
-        // TODO(benm): Move the management of this retained object set
-        // into the native java bridge. (crbug/169228)
-        mRetainedJavaScriptObjects.add(mJavaScriptInterfaces.get(name));
         mJavaScriptInterfaces.remove(name);
         if (mNativeContentViewCore != 0) {
             nativeRemoveJavascriptInterface(mNativeContentViewCore, name);
@@ -2597,7 +2589,7 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
     private native void nativeClearSslPreferences(int nativeContentViewCoreImpl);
 
     private native void nativeAddJavascriptInterface(int nativeContentViewCoreImpl, Object object,
-            String name, Class requiredAnnotation);
+            String name, Class requiredAnnotation, HashSet<Object> retainedObjectSet);
 
     private native void nativeRemoveJavascriptInterface(int nativeContentViewCoreImpl, String name);