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

2 files changed, 2 insertions, 3 deletions

diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
index c07a773..ad77670 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -479,8 +479,7 @@ bool ClientSideDetectionHost::DidPageReceiveSafeBrowsingMatch() const {
   return entry->GetExtraData(kSafeBrowsingMatchKey, &value);
 }
 
-void ClientSideDetectionHost::WebContentsDestroyed(WebContents* tab) {
-  DCHECK(tab);
+void ClientSideDetectionHost::WebContentsDestroyed() {
   // Tell any pending classification request that it is being canceled.
   if (classification_request_.get()) {
     classification_request_->Cancel();
diff --git a/chrome/browser/safe_browsing/client_side_detection_host.h b/chrome/browser/safe_browsing/client_side_detection_host.h
index 1db9f29..24ca81c 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.h
+++ b/chrome/browser/safe_browsing/client_side_detection_host.h
@@ -67,7 +67,7 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   explicit ClientSideDetectionHost(content::WebContents* tab);
 
   // From content::WebContentsObserver.
-  virtual void WebContentsDestroyed(content::WebContents* tab) OVERRIDE;
+  virtual void WebContentsDestroyed() OVERRIDE;
 
   // Used for testing.
   void set_safe_browsing_managers(