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}

2 files changed, 7 insertions, 4 deletions

diff --git a/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java b/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
index f7a82e6..216be3a 100644
--- a/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
+++ b/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java
@@ -331,10 +331,6 @@ import org.chromium.content_public.browser.WebContentsObserver;
     public void removeObserver(WebContentsObserver observer) {
         if (mObserverProxy == null) return;
         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.
diff --git a/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java b/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java
index fdae8f3a..730379a 100644
--- a/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java
+++ b/content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java
@@ -224,7 +224,14 @@ class WebContentsObserverProxy extends WebContentsObserver {
         // Java-based WebContents) are quite different, so we explicitly avoid
         // calling it here.
         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();
+
         if (mNativeWebContentsObserverProxy != 0) {
             nativeDestroy(mNativeWebContentsObserverProxy);
             mNativeWebContentsObserverProxy = 0;