Commit 28f5f238 authored by jdduke's avatar jdduke Committed by Commit bot

Ensure WebContentsObserver.destroy is called

When a Java-based WebContents instance is destroyed, it may yet have
Java-based WebContentsObserver instances. Previously, these instances
would always see a destroy (formerly detach) call, as they each had
a native counterpart.

With the introduction of the proxy WebContentsObserver, destroy was not
always called, as the remaining observers were simply cleared from the
observer list. Explicitly call WebContentsObserver.destroy for these
dangling instances, ensuring proper cleanup.

BUG=464076

Review URL: https://codereview.chromium.org/981703002

Cr-Commit-Position: refs/heads/master@{#319162}
parent b18d23f7
...@@ -331,10 +331,6 @@ import org.chromium.content_public.browser.WebContentsObserver; ...@@ -331,10 +331,6 @@ import org.chromium.content_public.browser.WebContentsObserver;
public void removeObserver(WebContentsObserver observer) { public void removeObserver(WebContentsObserver observer) {
if (mObserverProxy == null) return; if (mObserverProxy == null) return;
mObserverProxy.removeObserver(observer); mObserverProxy.removeObserver(observer);
if (!mObserverProxy.hasObservers()) {
mObserverProxy.destroy();
mObserverProxy = null;
}
} }
// This is static to avoid exposing a public destroy method on the native side of this class. // This is static to avoid exposing a public destroy method on the native side of this class.
......
...@@ -224,7 +224,14 @@ class WebContentsObserverProxy extends WebContentsObserver { ...@@ -224,7 +224,14 @@ class WebContentsObserverProxy extends WebContentsObserver {
// Java-based WebContents) are quite different, so we explicitly avoid // Java-based WebContents) are quite different, so we explicitly avoid
// calling it here. // calling it here.
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
for (mObserversIterator.rewind(); mObserversIterator.hasNext();) {
mObserversIterator.next().destroy();
}
// All observer destroy() implementations should result in their removal
// from the proxy.
assert mObservers.isEmpty();
mObservers.clear(); mObservers.clear();
if (mNativeWebContentsObserverProxy != 0) { if (mNativeWebContentsObserverProxy != 0) {
nativeDestroy(mNativeWebContentsObserverProxy); nativeDestroy(mNativeWebContentsObserverProxy);
mNativeWebContentsObserverProxy = 0; mNativeWebContentsObserverProxy = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment