diff options
author | torne <torne@chromium.org> | 2015-04-09 06:25:11 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-09 13:25:36 +0000 |
commit | 33f1cb1b80896a97041e4b87457e4f036d7a7ad9 (patch) | |
tree | 3d0c80e28ee73c1563b39592c36374165b46a796 /android_webview/glue | |
parent | 904ae915f28fa5ab7ce8077b9207602e8ac7b088 (diff) | |
download | chromium_src-33f1cb1b80896a97041e4b87457e4f036d7a7ad9.zip chromium_src-33f1cb1b80896a97041e4b87457e4f036d7a7ad9.tar.gz chromium_src-33f1cb1b80896a97041e4b87457e4f036d7a7ad9.tar.bz2 |
android_webview: stop leaking Contexts.
We were leaking activity Contexts in ResourcesContextWrapperFactory's
cache of wrapped contexts, because WeakHashMap only references its
*keys* weakly, and the ContextWrapper objects used as values in the
hashmap have a strong reference to the Context being used as a key, so
nothing was ever removed from the map.
Fix this by deleting the cache entirely, as it's not really necessary;
the number of wrapped contexts we create is already bounded to two per
WebView (one for the activity context and one for the application
context), and the objects are very small, so it's not worth the
complexity of trying to avoid lifetime issues while reusing them.
We do still need to ensure that WebViewChromiumFactoryProvider always
uses the same wrapper for the application context, though, since in some
initialisation paths we set the application context more than once and
the underlying code only permits that if the same context object is used
each time.
BUG=473146
Review URL: https://codereview.chromium.org/1078693003
Cr-Commit-Position: refs/heads/master@{#324425}
Diffstat (limited to 'android_webview/glue')
2 files changed, 64 insertions, 63 deletions
diff --git a/android_webview/glue/java/src/com/android/webview/chromium/ResourcesContextWrapperFactory.java b/android_webview/glue/java/src/com/android/webview/chromium/ResourcesContextWrapperFactory.java index a6e58a3..a802246 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/ResourcesContextWrapperFactory.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/ResourcesContextWrapperFactory.java @@ -11,87 +11,83 @@ import android.view.LayoutInflater; import org.chromium.base.annotations.SuppressFBWarnings; -import java.util.WeakHashMap; - /** * This class allows us to wrap the application context so that the WebView implementation can * correctly reference both org.chromium.* and application classes which is necessary to properly - * inflate UI. We keep a weak map from contexts to wrapped contexts to avoid constantly re-wrapping - * or doubly wrapping contexts. + * inflate UI. */ public class ResourcesContextWrapperFactory { - private static WeakHashMap<Context, ContextWrapper> sCtxToWrapper = - new WeakHashMap<Context, ContextWrapper>(); - private static final Object sLock = new Object(); - private ResourcesContextWrapperFactory() {} public static Context get(Context ctx) { - ContextWrapper wrappedCtx; - synchronized (sLock) { - wrappedCtx = sCtxToWrapper.get(ctx); - if (wrappedCtx == null) { - wrappedCtx = createWrapper(ctx); - sCtxToWrapper.put(ctx, wrappedCtx); - } + // Avoid double-wrapping a context. + if (ctx instanceof WebViewContextWrapper) { + return ctx; } - return wrappedCtx; + return new WebViewContextWrapper(ctx); } - private static ContextWrapper createWrapper(final Context ctx) { - return new ContextWrapper(ctx) { - private Context mApplicationContext; + private static class WebViewContextWrapper extends ContextWrapper { + private Context mApplicationContext; - @SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED") - @Override - public ClassLoader getClassLoader() { - final ClassLoader appCl = getBaseContext().getClassLoader(); - final ClassLoader webViewCl = this.getClass().getClassLoader(); - return new ClassLoader() { - @Override - protected Class<?> findClass(String name) throws ClassNotFoundException { - // First look in the WebViewProvider class loader. - try { - return webViewCl.loadClass(name); - } catch (ClassNotFoundException e) { - // Look in the app class loader; allowing it to throw - // ClassNotFoundException. - return appCl.loadClass(name); - } - } - }; - } + public WebViewContextWrapper(Context base) { + super(base); + } - @Override - public Object getSystemService(String name) { - if (Context.LAYOUT_INFLATER_SERVICE.equals(name)) { - LayoutInflater i = (LayoutInflater) getBaseContext().getSystemService(name); - return i.cloneInContext(this); - } else { - return getBaseContext().getSystemService(name); + @SuppressFBWarnings("DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED") + @Override + public ClassLoader getClassLoader() { + final ClassLoader appCl = getBaseContext().getClassLoader(); + final ClassLoader webViewCl = this.getClass().getClassLoader(); + return new ClassLoader() { + @Override + protected Class<?> findClass(String name) throws ClassNotFoundException { + // First look in the WebViewProvider class loader. + try { + return webViewCl.loadClass(name); + } catch (ClassNotFoundException e) { + // Look in the app class loader; allowing it to throw + // ClassNotFoundException. + return appCl.loadClass(name); + } } + }; + } + + @Override + public Object getSystemService(String name) { + if (Context.LAYOUT_INFLATER_SERVICE.equals(name)) { + LayoutInflater i = (LayoutInflater) getBaseContext().getSystemService(name); + return i.cloneInContext(this); + } else { + return getBaseContext().getSystemService(name); } + } - @Override - public Context getApplicationContext() { - if (mApplicationContext == null) { - mApplicationContext = get(ctx.getApplicationContext()); + @Override + public Context getApplicationContext() { + if (mApplicationContext == null) { + Context appCtx = getBaseContext().getApplicationContext(); + if (appCtx == getBaseContext()) { + mApplicationContext = this; + } else { + mApplicationContext = get(appCtx); } - return mApplicationContext; } + return mApplicationContext; + } - @Override - public void registerComponentCallbacks(ComponentCallbacks callback) { - // We have to override registerComponentCallbacks and unregisterComponentCallbacks - // since they call getApplicationContext().[un]registerComponentCallbacks() - // which causes us to go into a loop. - ctx.registerComponentCallbacks(callback); - } + @Override + public void registerComponentCallbacks(ComponentCallbacks callback) { + // We have to override registerComponentCallbacks and unregisterComponentCallbacks + // since they call getApplicationContext().[un]registerComponentCallbacks() + // which causes us to go into a loop. + getBaseContext().registerComponentCallbacks(callback); + } - @Override - public void unregisterComponentCallbacks(ComponentCallbacks callback) { - ctx.unregisterComponentCallbacks(callback); - } - }; + @Override + public void unregisterComponentCallbacks(ComponentCallbacks callback) { + getBaseContext().unregisterComponentCallbacks(callback); + } } } diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java index 5d3d668..041ffe2 100644 --- a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java +++ b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java @@ -79,6 +79,7 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider { private WebStorageAdapter mWebStorage; private WebViewDatabaseAdapter mWebViewDatabase; private AwDevToolsServer mDevToolsServer; + private Context mWrappedAppContext; private ArrayList<WeakReference<WebViewChromium>> mWebViewsToStart = new ArrayList<WeakReference<WebViewChromium>>(); @@ -291,7 +292,11 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider { } private Context getWrappedCurrentApplicationContext() { - return ResourcesContextWrapperFactory.get(mWebViewDelegate.getApplication()); + if (mWrappedAppContext == null) { + mWrappedAppContext = ResourcesContextWrapperFactory.get( + mWebViewDelegate.getApplication()); + } + return mWrappedAppContext; } AwBrowserContext getBrowserContext() { |