Fix for crash when deleting DataSource's in IO thread.

chrome/browser/dom_ui/chrome_url_data_manager.cc

Index: chrome/browser/dom_ui/chrome_url_data_manager.cc
diff --git a/chrome/browser/dom_ui/chrome_url_data_manager.cc b/chrome/browser/dom_ui/chrome_url_data_manager.cc
index 8acd19f10ac6c8a36ce32ce108c7ca14409fb621..c17d58b610497f907acf3e63997e9253c41e8c7e 100644
--- a/chrome/browser/dom_ui/chrome_url_data_manager.cc
+++ b/chrome/browser/dom_ui/chrome_url_data_manager.cc
@@ -11,6 +11,7 @@
 #include "base/path_service.h"
 #include "base/ref_counted_memory.h"
 #include "base/singleton.h"
+#include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/threading/thread.h"
 #include "base/values.h"
@@ -176,7 +177,15 @@ bool ChromeURLDataManager::URLToFilePath(const GURL& url,
 
 ChromeURLDataManager::ChromeURLDataManager() : next_request_id_(0) { }
 
-ChromeURLDataManager::~ChromeURLDataManager() { }
+ChromeURLDataManager::~ChromeURLDataManager() {
+  // This is used as a Singleton, so it is only called at exit cleanup time.
+  // This means it is called on the main (UI) thread.
+  //
+  // It will break if it is called at shutdown time on a different thread, as
+  // it will attempt to call the destructors for its |data_source_|s on the
+  // UI thread, but the UI thread's message loop will be not be running
+  // -- so the destructor calls will be dropped and we will leak the objects.
+}
 
 // static
 ChromeURLDataManager* ChromeURLDataManager::GetInstance() {
@@ -184,6 +193,15 @@ ChromeURLDataManager* ChromeURLDataManager::GetInstance() {
 }
 
 void ChromeURLDataManager::AddDataSource(scoped_refptr<DataSource> source) {
+  // Some |DataSource|-derived classes, notably |FileIconSource| and
+  // |DOMUIFavIconSource|, have members that will DCHECK if they are not
+  // destructed in the same thread as they are constructed (the UI thread).
+  //
+  // If |AddDataSource| is called more than once, it will destruct the object
+  // that it had before, as it is the only thing still holding a reference to
+  // that object.  In order for the destructor to be called in the UI thread,
+  // |DataSource| now uses the |DeleteOnUIThread| trait.

[eroman, 2011/01/24 18:26:39]

I don't like this comment, in particular how it says "now uses".

Comments written about how things used to be don't make sense in the future.

Please remove or reword this.

[ahendrickson, 2011/01/24 19:58:20]

Done.

+  //
   // TODO(jackson): A new data source with same name should not clobber the
   // existing one.
   data_sources_[source->source_name()] = source;