Revert 177765

Failed compile on all Linux bots.

> [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.
> 
> Android only change, android bots green.
> NOTRY=true
> BUG=169228
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11817047

TBR=benm@chromium.org
Review URL: https://codereview.chromium.org/12018025

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

4 files changed, 14 insertions, 141 deletions

diff --git a/content/browser/renderer_host/java/java_bound_object.cc b/content/browser/renderer_host/java/java_bound_object.cc
index 9d37dc3..f8b8a75 100644
--- a/content/browser/renderer_host/java/java_bound_object.cc
+++ b/content/browser/renderer_host/java/java_bound_object.cc
@@ -9,9 +9,7 @@
 #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;
@@ -125,14 +123,9 @@ 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,
-    const JavaRef<jclass>& safe_annotation_clazz,
-    const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager) {
+bool CallJNIMethod(jobject object, const JavaType& return_type, jmethodID id,
+                   jvalue* parameters, NPVariant* result,
+                   base::android::JavaRef<jclass>& safe_annotation_clazz) {
   JNIEnv* env = AttachCurrentThread();
   switch (return_type.type) {
     case JavaType::TypeBoolean:
@@ -216,8 +209,7 @@ bool CallJNIMethod(
         break;
       }
       OBJECT_TO_NPVARIANT(JavaBoundObject::Create(scoped_java_object,
-                                                  safe_annotation_clazz,
-                                                  manager),
+                                                  safe_annotation_clazz),
                           *result);
       break;
     }
@@ -735,8 +727,7 @@ jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
 
 NPObject* JavaBoundObject::Create(
     const JavaRef<jobject>& object,
-    const JavaRef<jclass>& safe_annotation_clazz,
-    const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager) {
+    base::android::JavaRef<jclass>& safe_annotation_clazz) {
   // 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.
@@ -744,36 +735,21 @@ 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, manager);
+      new JavaBoundObject(object, safe_annotation_clazz);
   return np_object;
 }
 
 JavaBoundObject::JavaBoundObject(
     const JavaRef<jobject>& object,
-    const JavaRef<jclass>& safe_annotation_clazz,
-    const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager)
+    base::android::JavaRef<jclass>& safe_annotation_clazz)
     : java_object_(AttachCurrentThread(), object.obj()),
-      manager_(manager),
       are_methods_set_up_(false),
       safe_annotation_clazz_(safe_annotation_clazz) {
-  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.
+  // We don't do anything 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) {
@@ -826,8 +802,7 @@ 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_,
-                       manager_);
+                       safe_annotation_clazz_);
   }
 
   // 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 ff97fdb..56cce29 100644
--- a/content/browser/renderer_host/java/java_bound_object.h
+++ b/content/browser/renderer_host/java/java_bound_object.h
@@ -12,14 +12,11 @@
 #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
@@ -35,11 +32,9 @@ 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,
-      const base::android::JavaRef<jclass>& safe_annotation_clazz,
-      const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager);
+      base::android::JavaRef<jclass>& safe_annotation_clazz);
 
   virtual ~JavaBoundObject();
 
@@ -57,21 +52,13 @@ class JavaBoundObject {
  private:
   explicit JavaBoundObject(
       const base::android::JavaRef<jobject>& object,
-      const base::android::JavaRef<jclass>& safe_annotation_clazz,
-      const base::WeakPtr<JavaBridgeDispatcherHostManager>& manager_);
+      base::android::JavaRef<jclass>& safe_annotation_clazz);
 
   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 c6c256b..0448132 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,16 +4,9 @@
 
 #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 {
@@ -32,7 +25,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
@@ -46,27 +39,6 @@ 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()) {
@@ -110,47 +82,4 @@ 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 b485ae0..0132cad 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,11 +7,8 @@
 
 #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"
 
@@ -24,9 +21,7 @@ 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,
-      public base::SupportsWeakPtr<JavaBridgeDispatcherHostManager> {
+class JavaBridgeDispatcherHostManager : public WebContentsObserver {
  public:
   explicit JavaBridgeDispatcherHostManager(WebContents* web_contents);
   virtual ~JavaBridgeDispatcherHostManager();
@@ -37,22 +32,10 @@ class JavaBridgeDispatcherHostManager
   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> >
@@ -60,7 +43,6 @@ class JavaBridgeDispatcherHostManager
   InstanceMap instances_;
   typedef std::map<string16, NPObject*> ObjectMap;
   ObjectMap objects_;
-  JavaObjectWeakGlobalRef retained_object_set_;
 
   DISALLOW_COPY_AND_ASSIGN(JavaBridgeDispatcherHostManager);
 };