We have a problem in the process on destroying WebContentsImpl because

content/public/browser/web_contents_observer.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 #define CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_
 
 #include "base/process/kill.h"
 #include "base/process/process_handle.h"
 #include "content/common/content_export.h"
@@ skipping 274 matching lines @@
   // that this ID is supplied by the renderer, so should be validated before
   // it's used for anything in case there's an exploited renderer.
   virtual void PluginHungStatusChanged(int plugin_child_id,
                                        const base::FilePath& plugin_path,
                                        bool is_hung) {}
 
   // Invoked when WebContents::Clone() was used to clone a WebContents.
   virtual void DidCloneToNewWebContents(WebContents* old_web_contents,
                                         WebContents* new_web_contents) {}
 
-  // Invoked when the WebContents is being destroyed. Gives subclasses a chance
-  // to cleanup. At the time this is invoked |web_contents()| returns NULL.
-  // It is safe to delete 'this' from here.
-  virtual void WebContentsDestroyed(WebContents* web_contents) {}
+  // When WebContents is being destroyed it notifies its observers and this
+  // process is split in two phases (untrusted and trusted).

[jam, 2014/05/02 19:34:11]

this trusted and untrusted terminology is a bit confusing and not standard, so
i'd leave this out.

i would stay with something like 

// Invoked when the WebContents is being destroyed. Gives subclasses a chance
// to cleanup. After this method returns, web_contents() returns NULL.

[zverre, 2014/05/05 16:40:25]

Done.

+  // First WebContents loops through its observers and calls
+  // WebContentsDestroyed to let them run cleanup code. At the time this is
+  // invoked |web_contents()| is guaranteed to be non NULL for all
+  // WebContentsObservers so they can directly or indirectly call each other.
+  // It is safe to delete 'this' from here. When this loop has been finished
+  // trusted phase starts, WebContents loops its WebContentsObservers again
+  // and NULLs its |web_contents_| pointers. This is done by calling
+  // WebContentsDestroyed and this behavior can't be modified since it is
+  // private.
+  virtual void WebContentsDestroyed() {}
 
   // Called when the user agent override for a WebContents has been changed.
   virtual void UserAgentOverrideSet(const std::string& user_agent) {}
 
   // Invoked when new FaviconURL candidates are received from the renderer.
   virtual void DidUpdateFaviconURL(int32 page_id,
                                    const std::vector<FaviconURL>& candidates) {}
 
   // Invoked when a pepper plugin creates and shows or destroys a fullscreen
   // render widget.
@@ skipping 43 matching lines @@
   virtual ~WebContentsObserver();
 
   // Start observing a different WebContents; used with the default constructor.
   void Observe(WebContents* web_contents);
 
   WebContents* web_contents() const;
 
  private:
   friend class WebContentsImpl;
 
-  // Invoked from WebContentsImpl. Invokes WebContentsDestroyed and NULL out
-  // |web_contents_|.
-  void WebContentsImplDestroyed();
+  // Resets |web_contents_| pointer to NULL as a final step in the process
+  // of destroying the WebContents. See comments near WebContentsDestroyed.
+  void WebContentsDestroyedPrivate() { web_contents_ = NULL; }

[jam, 2014/05/02 19:34:11]

nit: since it's inline and it's a simple method, it should just be
reset_web_contents() { web_contents_ = NULL; }

or really since it's private and the calling class is friends, i don't see a
need to have a method at all

[zverre, 2014/05/05 12:40:43]

Jam, thank you for your comments. I don't really want to write loop manually
because actual list of observers is private and only ObserverListBase::Iterator
can get actual observers' pointers. Also this approach seems not standard and
looks a bit weird in Chromium code. It's much more easy to find observers'
invocation in the code if it is always done with FOR_EACH_OBSERVER. So I'd
better stay with reset_web_contents as a better alternative to
WebContentsDestroyedPrivate.

[jam, 2014/05/05 15:05:03]

ok sure. i don't feel strongly about it (btw for the record, we have some places
that iterate manually over the observer list)

[zverre, 2014/05/05 16:40:25]

Done.

 
   WebContentsImpl* web_contents_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsObserver);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_OBSERVER_H_