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

chrome/browser/dom_ui/chrome_url_data_manager.cc

 // Copyright (c) 2011 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.
 
 #include "chrome/browser/dom_ui/chrome_url_data_manager.h"
 
 #include "app/l10n_util.h"
 #include "base/file_util.h"
 #include "base/i18n/rtl.h"
 #include "base/message_loop.h"
 #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"
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #endif
 #include "chrome/browser/appcache/view_appcache_internals_job_factory.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/dom_ui/shared_resources_data_source.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
@@ skipping 145 matching lines @@
   if (p.IsAbsolute())
     return false;
 
   *file_path = i->second.AppendASCII(relative_path);
 
   return true;
 }
 
 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() {
   return Singleton<ChromeURLDataManager>::get();
 }
 
 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;
 }
 
 void ChromeURLDataManager::AddFileSource(const std::string& source_name,
                                          const FilePath& file_path) {
   DCHECK(file_sources_.count(source_name) == 0);
   file_sources_[source_name] = file_path;
 }
@@ skipping 235 matching lines @@
                                            net::ERR_INVALID_URL));
   }
 }
 
 URLRequestChromeFileJob::URLRequestChromeFileJob(net::URLRequest* request,
                                                  const FilePath& path)
     : net::URLRequestFileJob(request, path) {
 }
 
 URLRequestChromeFileJob::~URLRequestChromeFileJob() { }