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, 2 insertions, 2 deletions

diff --git a/chrome/browser/infobars/infobar_service.cc b/chrome/browser/infobars/infobar_service.cc
index 3d03544..030035b 100644
--- a/chrome/browser/infobars/infobar_service.cc
+++ b/chrome/browser/infobars/infobar_service.cc
@@ -107,11 +107,11 @@ void InfoBarService::NavigationEntryCommitted(
   OnNavigation(NavigationDetailsFromLoadCommittedDetails(load_details));
 }
 
-void InfoBarService::WebContentsDestroyed(content::WebContents* web_contents) {
+void InfoBarService::WebContentsDestroyed() {
   // The WebContents is going away; be aggressively paranoid and delete
   // ourselves lest other parts of the system attempt to add infobars or use
   // us otherwise during the destruction.
-  web_contents->RemoveUserData(UserDataKey());
+  web_contents()->RemoveUserData(UserDataKey());
   // That was the equivalent of "delete this". This object is now destroyed;
   // returning from this function is the only safe thing to do.
 }