Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(278)

Unified Diff: content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

Issue 11802002: [Android] Fix leak in Java Bridge. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: joth comments Created 7 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/renderer_host/java/java_bound_object.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
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 e19ae11858a2df90c41b5d196005e7b8306606b0..d66f1ca8fa34667b54c7759b04cfc0c0fe22ea0d 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
@@ -47,6 +47,10 @@ import org.chromium.content.common.TraceEvent;
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;
/**
* Provides a Java-side 'wrapper' around a WebContent (native) instance.
@@ -73,6 +77,27 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
// Personality of the ContentView.
private final int mPersonality;
+ // If the embedder adds a JavaScript interface object that contains an indirect reference to
+ // the ContentViewCore, then storing a strong ref to the interface object on the native
+ // side would prevent garbage collection of the ContentViewCore (as that strong ref would
+ // create a new GC root).
+ // For that reason, we store only a weak reference to the interface object on the
+ // native side. However we still need a strong reference on the Java side to
+ // prevent garbage collection if the embedder doesn't maintain their own ref to the
+ // interface object - the Java side ref won't create a new GC root.
+ // This map stores those refernces. We put into the map on addJavaScriptInterface()
+ // and remove from it in removeJavaScriptInterface().
+ private Map<String, Object> mJavaScriptInterfaces = new HashMap<String, Object>();
+
+ // Additionally, we keep track of all Java bound JS objects that are in use on the
+ // 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>();
+
/**
* Interface that consumers of {@link ContentViewCore} must implement to allow the proper
* dispatching of view methods through the containing view.
@@ -492,6 +517,10 @@ 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();
}
};
}
@@ -623,6 +652,8 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
}
mNativeContentViewCore = 0;
mContentSettings = null;
+ mJavaScriptInterfaces.clear();
+ mRetainedJavaScriptObjects.clear();
}
/**
@@ -2129,13 +2160,6 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
return mZoomManager.getZoomControlsViewForTest();
}
- // TODO(joth): Remove in next patch.
- @Deprecated
- public void addJavascriptInterface(Object object, String name, boolean requireAnnotation) {
- addPossiblyUnsafeJavascriptInterface(object, name,
- requireAnnotation ? JavascriptInterface.class : null);
- }
-
/**
* This will mimic {@link #addPossiblyUnsafeJavascriptInterface(Object, String, Class)}
* and automatically pass in {@link JavascriptInterface} as the required annotation.
@@ -2193,6 +2217,7 @@ public class ContentViewCore implements MotionEventDelegate, NavigationClient {
public void addPossiblyUnsafeJavascriptInterface(Object object, String name,
Class<? extends Annotation> requiredAnnotation) {
if (mNativeContentViewCore != 0 && object != null) {
+ mJavaScriptInterfaces.put(name, object);
nativeAddJavascriptInterface(mNativeContentViewCore, object, name, requiredAnnotation);
}
}
@@ -2203,6 +2228,10 @@ 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);
}
« no previous file with comments | « content/browser/renderer_host/java/java_bound_object.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698