Address HandlerLeak lint warning when building webview. |
|||
Issue description
The following code has a warning:
/tmp/tmpvhaqpq/SRC_ROOT1/clank/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java:133 This Handler class should be static or leaks might occur (new android.os.Handler(){}): HandlerLeak [warning]
mUiThreadHandler = new Handler() {
^
I think we should make the handler a static inner class and take a weakReference to the upper object.
,
Aug 23 2016
see https://groups.google.com/d/msg/android-developers/1aPZXZG6kWk/lIYDavGYn5UJ basically if there is still messages in the handler, the handler won't get gc-ed. "If the Message lives in the queue for a long time, which happens fairly easily when posting a delayed message for instance, you keep a reference to the Activity and "leak" all the views and resources."
,
Aug 23 2016
,
Aug 24 2016
It seems like in our case this isn't a real problem. We're keeping the WebView alive while the handler exists, but this isn't going to be posted as a delayed message, and we expect a synchronous return from onCreateWindow. What happens if an app returns true from onCreateWindow but doesn't send us the new WebView instance until later? Does that work, or do we expect them to have sent it before they return? If they're allowed to not send it until later then yeah, you would be leaking the parent WebView and everything it references until the app replies, but that really doesn't seem that serious if it's even possible, and I can't imagine apps doing this. Making it a weak reference means we'd then have potentially very weird behaviour in the case where the app *has* managed to do this..
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f148e4abd2cf574fcc82419c4163309a7eb85e1a commit f148e4abd2cf574fcc82419c4163309a7eb85e1a Author: hush <hush@chromium.org> Date: Tue Aug 30 23:07:02 2016 Fix HandlerLeak lint warning Suppress it, because fixing it with static inner classes proved to be troublesome and not worth the effort. BUG= 640400 Review-Url: https://codereview.chromium.org/2268233004 Cr-Commit-Position: refs/heads/master@{#415480} [modify] https://crrev.com/f148e4abd2cf574fcc82419c4163309a7eb85e1a/android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java
,
Sep 2 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/8c96bd9dd3e08264e8917738e8c78c246c0a7f4e commit 8c96bd9dd3e08264e8917738e8c78c246c0a7f4e Author: hush <hush@chromium.org> Date: Tue Aug 30 23:07:02 2016
,
Sep 6 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by boliu@chromium.org
, Aug 23 2016