Detach the RulesRegistryWithCache::weak_ptr_factory_ on destruction.

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

 // 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.
 
 #include "chrome/browser/extensions/api/declarative/rules_registry_service.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/extensions/api/declarative/initializing_rules_registry.h"
@@ skipping 46 matching lines @@
   scoped_refptr<ContentRulesRegistry> content_rules_registry(
       new ContentRulesRegistry(profile_, &ui_part));
   ui_parts_of_registries_.push_back(ui_part.release());
 
   RegisterRulesRegistry(content_rules_registry);
   content_rules_registry_ = content_rules_registry.get();
 #endif  // defined(ENABLE_EXTENSIONS)
 }
 
 void RulesRegistryService::Shutdown() {
+  // Release the references to all registries. This would happen soon during
+  // destruction of |*this|, but we need the ExtensionWebRequestEventRouter to
+  // be the last to reference the WebRequestRulesRegistry objects, so that
+  // the posted task below causes their destruction on the IO thread, not on UI
+  // where the destruction of |*this| takes place.
+  // TODO(vabr): Remove once http://crbug.com/218451#c6 gets addressed.
+  rule_registries_.clear();
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&RegisterToExtensionWebRequestEventRouterOnIO,
           profile_, scoped_refptr<WebRequestRulesRegistry>(NULL)));

[battre, 2013/06/03 10:22:15]

Wouldn't the correct way of solving this be:

void HoldReferenceUntilCalled(scoped_refptr<WebRequestRulesRegistry>
rules_registry) {
  // Do nothing. The purpose of this function is just to hold the ref-counted
reference until
  // it is called. This allows us to destruct the object on the right thread.
}

void RulesRegistryService::Shutdown() {
  RulesRegistryMap temp_rule_registries;
  std::swap(&temp_rule_registries, &rule_registries_);
  content::BrowserThread::PostTask(
      content::BrowserThread::IO, FROM_HERE,
      base::Bind(&RegisterToExtensionWebRequestEventRouterOnIO,
      profile_, scoped_refptr<WebRequestRulesRegistry>(NULL)));
  for (rule registry : temp_rule_registries) {
     post task HoldReferenceUntilCalled on owner thread of rule registry
  }
}

[vabr (Chromium), 2013/06/03 10:45:45]

Could you please be more specific about why you prefer the suggested solution?
As far as I understood, the RegisterToExtensionWebRequestEventRouterOnIO is
already there to make sure WebRequestRulesRegistry objects are deleted on the
appropriate thread, only there was a bug in how it was used, which is what I'm
hoping to fix with the explicit clear() on |rules_registries_|.

Your solution introduces a couple of posted tasks, which means slowing down the
shutdown unnecessarily. It does not give any more guarantee that the
WebRequestRulesRegistry objects are deleted on the right thread, than the
solution from this patch set -- both depend on the fact that the registries are
not referenced anywhere else than in RulesRegistryService or
ExtensionWebRequestEventRouter.

[battre, 2013/06/03 11:42:42]

On second thought, your solution is equally good.

[Jeffrey Yasskin, 2013/06/03 16:12:16]

The above code is subtly incorrect because it's possible for any of those posted
tasks to be executed before Shutdown() finishes, or even before PostTask
returns, in which case the reference would be destroyed on the current thread
anyway.

To make this correct, if we had move semantics for Bind(), we could ensure the
reference was transferred over to the other thread instead of copied (we can
even do enough of this in C++98), or we could manually call AddRef in this
function and Release on the raw pointer in HoldReferenceUntilCalled. But I think
it's cleaner to move toward singly-owned RulesRegistries anyway instead of
refcounting them.

 }
 
 void RulesRegistryService::RegisterRulesRegistry(
     scoped_refptr<RulesRegistry> rule_registry) {
   const std::string event_name(rule_registry->event_name());
   DCHECK(rule_registries_.find(event_name) == rule_registries_.end());
   rule_registries_[event_name] =
       make_scoped_refptr(new InitializingRulesRegistry(rule_registry));
 }
 
@@ skipping 38 matching lines @@
       OnExtensionUnloaded(extension->id());
       break;
     }
     default:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace extensions