Don't leak ContentViewCores via ImeAdapter

Don't leak ContentViewCores via ImeAdapter

The existing strong ref from native to java forms a GC root which
thwarts GC of ImeAdapter.java or any object referenced by it, which
includes ContentViewCore by dint of ViewEmbedder subclass being a
non-static inner class of CVC and hence binding it.
This is problematic for WebView as it requires reliable finalizer
based cleanup of CVC.

BUG=b/7697692


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188584

[Kristian Monsen, 2013-03-14 22:42:38]

lgtm

https://codereview.chromium.org/12666007/diff/3001/content/browser/renderer_h...
File content/browser/renderer_host/ime_adapter_android.cc (right):

https://codereview.chromium.org/12666007/diff/3001/content/browser/renderer_h...
content/browser/renderer_host/ime_adapter_android.cc:181:
base::android::ScopedJavaLocalRef<jobject> obj =
maybe slightly more expressive name than obj? Also above.

[joth, 2013-03-15 02:39:45]

+benm

I started adding tests for this too... https://codereview.chromium.org/12658010/
but they're not ready yet. Hitting a lot of other crazy stuff in the destructor
path. (Now I *finally* forced it to actually destruct!)

https://codereview.chromium.org/12666007/diff/3001/content/browser/renderer_h...
File content/browser/renderer_host/ime_adapter_android.cc (right):

https://codereview.chromium.org/12666007/diff/3001/content/browser/renderer_h...
content/browser/renderer_host/ime_adapter_android.cc:181:
base::android::ScopedJavaLocalRef<jobject> obj =
On 2013/03/14 22:42:38, Kristian Monsen wrote:
> maybe slightly more expressive name than obj? Also above.

yeah.. obj seems to be a fairly common name for "this object over on the java
side, as a local", see aw_content.cc. Short name is handy as it all gets very
verbose else.
But I'm not opposed to changing the pattern if there's something particularly
better

[benm (inactive), 2013-03-15 10:18:55]

lgtm

LGTM

+OWNERS in case anyone's awake and we can land :)

[bulach, 2013-03-15 10:27:05]

lgtm, thanks for fixing it! (nit: commit message, s/ased/based/)

[joth, 2013-03-15 17:38:25]

cc aurimas FYI.

sending to CQ now.

[commit-bot: I haz the power, 2013-03-15 17:38:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/12666007/3001

[commit-bot: I haz the power, 2013-03-15 17:38:45]

Presubmit check for 12666007-3001 failed and returned exit status 1.

INFO:root:Found 3 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    content/browser/renderer_host/ime_adapter_android.cc
    content/browser/renderer_host/ime_adapter_android.h

[no sievers, 2013-03-15 17:42:40]

On 2013/03/15 17:38:45, I haz the power (commit-bot) wrote:
> Presubmit check for 12666007-3001 failed and returned exit status 1.
> 
> INFO:root:Found 3 file(s).
> 
> Running presubmit commit checks ...
> Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for these files:
>     content/browser/renderer_host/ime_adapter_android.cc
>     content/browser/renderer_host/ime_adapter_android.h

rubberstamp lgtm

[commit-bot: I haz the power, 2013-03-15 22:06:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/12666007/3001

[commit-bot: I haz the power, 2013-03-16 16:03:59]

Change committed as 188584