We have a problem in the process on destroying WebContentsImpl because

each WebContentsObserver first NULLs its web_contents_ pointer and
performs cleanup code after that.

This could work if we guaranteed that every WebContentsObserver doesn't
directly or indirectly affect any other WebContentsObserver. But
actually WebContentsObservers can do anything and also affect each
other. And if one WebContentsObserver already NULLed its web_contents_
pointer (e.g. ZoomController) other WebContentsObserver
(e.g. InterstitialPageImpl) calls it (indirectly in this case) we have
problems and result depends on ordering of WebContentsObservers in the
list of observers.

The solution splits that process into two phases. First we let each
WebContentsObserver to perform their cleanup code and at that point they
can freely call each other, we completely don't care about the ordering.
We can call this phase as untrusted phase because we deal with some cleanup
code that can do anything. After that we make second pass and NULL each
web_contents_ pointer and this phase is trusted because we call only private
implementation of WebContentsObserver that we definitely can trust.

BUG=363564

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269264 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 4 insertions, 5 deletions

diff --git a/android_webview/native/aw_contents_io_thread_client_impl.cc b/android_webview/native/aw_contents_io_thread_client_impl.cc
index c863971..555cd83 100644
--- a/android_webview/native/aw_contents_io_thread_client_impl.cc
+++ b/android_webview/native/aw_contents_io_thread_client_impl.cc
@@ -112,7 +112,7 @@ class ClientMapEntryUpdater : public content::WebContentsObserver {
 
   virtual void RenderFrameCreated(RenderFrameHost* render_frame_host) OVERRIDE;
   virtual void RenderFrameDeleted(RenderFrameHost* render_frame_host) OVERRIDE;
-  virtual void WebContentsDestroyed(WebContents* web_contents) OVERRIDE;
+  virtual void WebContentsDestroyed() OVERRIDE;
 
  private:
   JavaObjectWeakGlobalRef jdelegate_;
@@ -142,7 +142,7 @@ void ClientMapEntryUpdater::RenderFrameDeleted(RenderFrameHost* rfh) {
   RfhToIoThreadClientMap::GetInstance()->Erase(GetRenderFrameHostIdPair(rfh));
 }
 
-void ClientMapEntryUpdater::WebContentsDestroyed(WebContents* web_contents) {
+void ClientMapEntryUpdater::WebContentsDestroyed() {
   delete this;
 }
 
diff --git a/android_webview/native/aw_settings.cc b/android_webview/native/aw_settings.cc
index 843ebf7..3779cd8 100644
--- a/android_webview/native/aw_settings.cc
+++ b/android_webview/native/aw_settings.cc
@@ -267,7 +267,7 @@ void AwSettings::RenderViewCreated(content::RenderViewHost* render_view_host) {
   UpdateEverything();
 }
 
-void AwSettings::WebContentsDestroyed(content::WebContents* web_contents) {
+void AwSettings::WebContentsDestroyed() {
   delete this;
 }
 
diff --git a/android_webview/native/aw_settings.h b/android_webview/native/aw_settings.h
index 95c0d9c..92bf658 100644
--- a/android_webview/native/aw_settings.h
+++ b/android_webview/native/aw_settings.h
@@ -46,8 +46,7 @@ class AwSettings : public content::WebContentsObserver {
   // WebContentsObserver overrides:
   virtual void RenderViewCreated(
       content::RenderViewHost* render_view_host) OVERRIDE;
-  virtual void WebContentsDestroyed(
-      content::WebContents* web_contents) OVERRIDE;
+  virtual void WebContentsDestroyed() OVERRIDE;
 
   bool accelerated_2d_canvas_disabled_by_switch_;