Detach the RulesRegistryWithCache::weak_ptr_factory_ on destruction.

chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc

Index: chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
diff --git a/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc b/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
index ab0003bfcc8c39a777954122575676430741e548..db79f17dddea803bc2e6dd3e39688b931498733a 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
@@ -80,7 +80,7 @@ RulesRegistryWithCache::RulesRegistryWithCache(
     bool log_storage_init_delay,
     scoped_ptr<RuleStorageOnUI>* ui_part)
     : RulesRegistry(owner_thread, event_name),
-      weak_ptr_factory_((profile) ? this : NULL),
+      weak_ptr_factory_(this),
       storage_on_ui_((profile
                           ? (new RuleStorageOnUI(profile,
                                                  GetDeclarativeRuleStorageKey(
@@ -217,6 +217,7 @@ void RulesRegistryWithCache::OnExtensionUnloaded(
 }
 
 RulesRegistryWithCache::~RulesRegistryWithCache() {
+  weak_ptr_factory_.DetachFromThread();

[Jeffrey Yasskin, 2013/05/28 22:37:10]

This isn't safe, according to the thread-safety rules at
https://code.google.com/p/chromium/codesearch/#chromium/src/base/memory/weak_....
weak_ptr_factory_.Invalidate() has to be called on the thread that uses
WeakPtr::get(), and that's the IO thread for the WebRequestRulesRegistry.

The underlying problem looks to be that RulesRegistry is refcounted, and so
occasionally the last reference will happen to be on the UI thread instead of
the rules_registry_thread? Now that RulesRegistryWithCache has a
weak_ptr_factory, it might be easy to un-ref-count RulesRegistry.

It would be good to know where this last reference is held. Could you add a
DCHECK(CurrentlyOn(owner_thread())) here and run your test again to get a stack
trace for the culprit?

[Jeffrey Yasskin, 2013/05/28 22:48:49]

One way to work around this would be to add a method that calls
weak_ptr_factory_.Invalidate(), post a call to that onto the owner thread from
whatever point on the UI thread intends to release the RulesRegistry.

 }
 
 void RulesRegistryWithCache::MarkReady(base::Time storage_init_time) {