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

1 files changed, 1 insertions, 12 deletions

diff --git a/content/browser/frame_host/interstitial_page_impl.cc b/content/browser/frame_host/interstitial_page_impl.cc
index 184a561..975e7d6 100644
--- a/content/browser/frame_host/interstitial_page_impl.cc
+++ b/content/browser/frame_host/interstitial_page_impl.cc
@@ -356,18 +356,7 @@ void InterstitialPageImpl::NavigationEntryCommitted(
   OnNavigatingAwayOrTabClosing();
 }
 
-void InterstitialPageImpl::WebContentsWillBeDestroyed() {
-  OnNavigatingAwayOrTabClosing();
-}
-
-void InterstitialPageImpl::WebContentsDestroyed(WebContents* web_contents) {
-  // WebContentsImpl will only call WebContentsWillBeDestroyed for interstitial
-  // pages that it knows about, pages that called
-  // WebContentsImpl::AttachInterstitialPage. But it's possible to have an
-  // interstitial page that never progressed that far. In that case, ensure that
-  // this interstitial page is destroyed. (And if it was attached, and
-  // OnNavigatingAwayOrTabClosing was called, it's safe to call
-  // OnNavigatingAwayOrTabClosing twice.)
+void InterstitialPageImpl::WebContentsDestroyed() {
   OnNavigatingAwayOrTabClosing();
 }