This repository has been archived by the owner. It is now read-only.

Memory leak in WebViewFragment #75

Closed
vickychijwani opened this Issue Jun 22, 2015 · 16 comments

Comments

Projects
None yet
3 participants
@vickychijwani
Owner

vickychijwani commented Jun 22, 2015

Steps to reproduce:

  1. Open a post preview (the new WebView-based one)
  2. Select some text
  3. Tap back twice to come back to the post list
  4. Open up the post again and watch LeakCanary dump a heap

Note:

  • Test fix on older Android versions (webkit vs chromium)
  • Merge BrowserActivity and WebViewFragment (or fix and test both)

Unfortunately the heap trace (below) is unhelpful:

me.vickychijwani.spectre D/LeakCanary﹕ In me.vickychijwani.spectre:1.0:1.
    * me.vickychijwani.spectre.view.PostViewActivity has leaked:
    * GC ROOT org.chromium.content.browser.input.PopupTouchHandleDrawable.mContext
    * leaks me.vickychijwani.spectre.view.PostViewActivity instance
    * Reference Key: c5347412-4c57-49a2-9df7-9760955cedeb
    * Device: LGE google Nexus 4 occam
    * Android Version: 5.1.1 API: 22
    * Durations: watch=5124ms, gc=162ms, heap dump=11630ms, analysis=43875ms

Maybe analyzing the actual heap dump will help.

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Jun 22, 2015

Got the heap dump, here: https://www.dropbox.com/s/zodudiuacbctjcr/quill_issue_75.hprof?dl=0 (needs to be converted using hprof-conv)

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Jun 24, 2015

Dammit, this is back to haunt me in a different form. Now it happens consistently with PostViewActivity and BrowserActivity, even without selecting text! 😕

me.vickychijwani.spectre D/LeakCanary﹕ In me.vickychijwani.spectre:1.0:1.
    * me.vickychijwani.spectre.view.PostViewActivity has leaked:
    * GC ROOT static me.vickychijwani.spectre.SpectreApplication.sInstance
    * references me.vickychijwani.spectre.DebugSpectreApplication.mComponentCallbacks
    * references java.util.ArrayList.array
    * references array java.lang.Object[].[10]
    * references org.chromium.android_webview.AwContents$AwComponentCallbacks.this$0
    * references org.chromium.android_webview.AwContents.mContext
    * references com.android.webview.chromium.ResourcesContextWrapperFactory$WebViewContextWrapper.mBase
    * leaks me.vickychijwani.spectre.view.PostViewActivity instance
    * Reference Key: e08d6645-ddf7-411c-9898-d4575efd6cd9
    * Device: LGE google Nexus 4 occam
    * Android Version: 5.1.1 API: 22
    * Durations: watch=5022ms, gc=508ms, heap dump=13897ms, analysis=39075ms
@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Jun 24, 2015

@vickychijwani vickychijwani reopened this Aug 17, 2015

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Aug 17, 2015

WebView strikes again:

me.vickychijwani.spectre.debug D/LeakCanary﹕ In me.vickychijwani.spectre.debug:1.0.0-beta:1.
    * me.vickychijwani.spectre.view.PostViewActivity has leaked:
    * GC ROOT android.view.accessibility.AccessibilityManager$1.this$0 (anonymous class extends android.view.accessibility.IAccessibilityManagerClient$Stub)
    * references android.view.accessibility.AccessibilityManager.mAccessibilityStateChangeListeners
    * references java.util.concurrent.CopyOnWriteArrayList.elements
    * references array java.lang.Object[].[1]
    * references org.chromium.content.browser.ContentViewCore.mContext
    * references com.android.webview.chromium.ResourcesContextWrapperFactory$WebViewContextWrapper.mBase
    * leaks me.vickychijwani.spectre.view.PostViewActivity instance
    * Reference Key: 4e7b339e-303c-46a5-bf89-76d165a77dc3
    * Device: LGE google Nexus 4 occam
    * Android Version: 5.1.1 API: 22
    * Durations: watch=5040ms, gc=249ms, heap dump=12780ms, analysis=46400ms

Perhaps the previous fix (read: hack) can be modified to plug this leak as well?

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Aug 17, 2015

The forceUnregisterComponentCallbacks hack seems to be of no help here. I'll have to analyze the heap dump.

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Aug 26, 2015

This may be of use (directly or source code-wise): https://github.com/jjhesk/EZWebView

@bjenning04

This comment has been minimized.

bjenning04 commented Nov 19, 2015

My team is also running into this issue in our project. According to square/leakcanary#92, this is related to https://code.google.com/p/chromium/issues/detail?id=473146, which appears to be fixed in Marshmallow. That said, I'd love to know if you ever found a solution to this for Jelly Bean or greater.

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Nov 19, 2015

@bjenning04 no, unfortunately. Thanks for letting me know of the fix in Marshmallow! Although my latest heap trace above seems to be quite different from the one in the issue you linked to, doesn't it? Which one are you getting?

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Nov 19, 2015

Oh wait, I see your point now. After reading through the Chromium issue comments I found this:

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.

So all ResourcesContextWrapperFactory leaks should indeed be fixed in Marshmallow. Thanks again!

@vickychijwani vickychijwani removed this from the 1.0 milestone May 1, 2016

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented May 22, 2016

@bjenning04 It turns out this was programmer error after all. WebView expects to be detached from the view hierarchy before being destroyed, which was not happening. Check out the docs for WebView#destroy(): https://developer.android.com/reference/android/webkit/WebView.html#destroy%28%29

@jessyuan24

This comment has been minimized.

jessyuan24 commented Aug 23, 2016

Did you solve the memory leaked?

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Aug 23, 2016

@jessyuan24 yes, you can see the code here. Basically you have to make sure you call ((ViewGroup) mWebView.getParent()).removeView(mWebView) before mWebView.destroy(). This is also pointed out in the documentation for WebView#destroy.

@jessyuan24

This comment has been minimized.

jessyuan24 commented Aug 23, 2016

Okay, I try it.

@jessyuan24

This comment has been minimized.

jessyuan24 commented Aug 23, 2016

It doesn't work. I have same problem as you.

me.vickychijwani.spectre.debug D/LeakCanary﹕ In me.vickychijwani.spectre.debug:1.0.0-beta:1. * me.vickychijwani.spectre.view.PostViewActivity has leaked: * GC ROOT android.view.accessibility.AccessibilityManager$1.this$0 (anonymous class extends android.view.accessibility.IAccessibilityManagerClient$Stub) * references android.view.accessibility.AccessibilityManager.mAccessibilityStateChangeListeners * references java.util.concurrent.CopyOnWriteArrayList.elements * references array java.lang.Object[].[1] * references org.chromium.content.browser.ContentViewCore.mContext * references com.android.webview.chromium.ResourcesContextWrapperFactory$WebViewContextWrapper.mBase * leaks me.vickychijwani.spectre.view.PostViewActivity instance * Reference Key: 4e7b339e-303c-46a5-bf89-76d165a77dc3 * Device: LGE google Nexus 4 occam * Android Version: 5.1.1 API: 22 * Durations: watch=5040ms, gc=249ms, heap dump=12780ms, analysis=46400ms

@vickychijwani

This comment has been minimized.

Owner

vickychijwani commented Aug 23, 2016

Maybe you can post a question on StackOverflow with more details? (your exact code and stack trace). I would be happy to take a look there. This thread is really not the right place to discuss this 😕

@jessyuan24

This comment has been minimized.

jessyuan24 commented Aug 23, 2016

No, It works. Hahahaha

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.