Fix a memory leak in RulesRegistryStorageDelegate.

chrome/browser/extensions/api/declarative/rules_registry_storage_delegate.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_storage_delegate.h"
 
 #include "base/bind.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/state_store.h"
 #include "chrome/common/chrome_notification_types.h"
@@ skipping 34 matching lines @@
   return rules;
 }
 
 }  // namespace
 
 // This class coordinates information between the UI and RulesRegistry threads.
 // It may outlive the RulesRegistry, which owns the delegate. Methods/variables
 // should be used on the UI thread unless otherwise noted.
 class RulesRegistryStorageDelegate::Inner
     : public content::NotificationObserver,
-      public base::RefCountedThreadSafe<
-          Inner, content::BrowserThread::DeleteOnUIThread> {
+      public base::RefCountedThreadSafe<Inner> {
  public:
   Inner(Profile* profile,
         RulesRegistryWithCache* rules_registry,
         const std::string& storage_key);
 
  private:
+  friend class base::RefCountedThreadSafe<Inner>;
   friend class RulesRegistryStorageDelegate;
-  friend struct content::BrowserThread::DeleteOnThread<
-      content::BrowserThread::UI>;
-  friend class base::DeleteHelper<Inner>;
 
   ~Inner();
 
   // NotificationObserver
   virtual void Observe(
       int type,
       const content::NotificationSource& source,
       const content::NotificationDetails& details) OVERRIDE;
 
   // Read/write a list of rules serialized to Values.
   void ReadFromStorage(const std::string& extension_id);
   void ReadFromStorageCallback(const std::string& extension_id,
                        scoped_ptr<base::Value> value);
   void WriteToStorage(const std::string& extension_id,
                       scoped_ptr<base::Value> value);
 
   // Check if we are done reading all data from storage on startup, and notify
   // the RulesRegistry on its thread if so. The notification is delivered
   // exactly once.
   void CheckIfReady();
 
   // Deserialize the rules from the given Value object and add them to the
   // RulesRegistry.
   void ReadFromStorageOnRegistryThread(const std::string& extension_id,
                                        scoped_ptr<base::Value> value);
 
   // Notify the RulesRegistry that we are now ready.
   void NotifyReadyOnRegistryThread();
 
-  content::NotificationRegistrar registrar_;
+  scoped_ptr<content::NotificationRegistrar> registrar_;
   Profile* profile_;
 
   // The key under which rules are stored.
   const std::string storage_key_;
 
   // A set of extension IDs that have rules we are reading from storage.
   std::set<std::string> waiting_for_extensions_;
 
   // The thread that our RulesRegistry lives on.
   content::BrowserThread::ID rules_registry_thread_;
 
   // The following are only accessible on rules_registry_thread_.
 
   // The RulesRegistry whose delegate we are.
   RulesRegistryWithCache* rules_registry_;
 
   // True when we have finished reading from storage for all extensions that
   // are loaded on startup.
   bool ready_;
 };
 
 RulesRegistryStorageDelegate::RulesRegistryStorageDelegate() {
 }
 
 RulesRegistryStorageDelegate::~RulesRegistryStorageDelegate() {
   // RulesRegistry owns us, which means it has been deleted.
   inner_->rules_registry_ = NULL;
 }
 
-void RulesRegistryStorageDelegate::Init(Profile* profile,
-                                        RulesRegistryWithCache* rules_registry,
-                                        const std::string& storage_key) {
+void RulesRegistryStorageDelegate::InitOnUIThread(
+    Profile* profile,
+    RulesRegistryWithCache* rules_registry,
+    const std::string& storage_key) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   extensions::StateStore* store = ExtensionSystem::Get(profile)->state_store();
   if (store)
     store->RegisterKey(storage_key);
   inner_ = new Inner(profile, rules_registry, storage_key);
 }
 
+void RulesRegistryStorageDelegate::CleanupOnUIThread() {
+  // The registrar must be deleted on the UI thread.
+  inner_->registrar_.reset();
+}
+
 bool RulesRegistryStorageDelegate::IsReady() {
   DCHECK(content::BrowserThread::CurrentlyOn(inner_->rules_registry_thread_));
   return inner_->ready_;
 }
 
 void RulesRegistryStorageDelegate::OnRulesChanged(
     RulesRegistryWithCache* rules_registry,
     const std::string& extension_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(inner_->rules_registry_thread_));
   std::vector<linked_ptr<RulesRegistry::Rule> > new_rules;
   std::string error = rules_registry->GetAllRules(extension_id, &new_rules);
   DCHECK_EQ("", error);
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&Inner::WriteToStorage, inner_.get(), extension_id,
                  base::Passed(RulesToValue(new_rules))));
 }
 
 RulesRegistryStorageDelegate::Inner::Inner(
     Profile* profile,
     RulesRegistryWithCache* rules_registry,
     const std::string& storage_key)
-    : profile_(profile),
+    : registrar_(new content::NotificationRegistrar()),
+      profile_(profile),
       storage_key_(storage_key),
       rules_registry_thread_(rules_registry->GetOwnerThread()),
       rules_registry_(rules_registry),
       ready_(false) {
-  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
-                 content::Source<Profile>(profile));
-  registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
-                 content::Source<Profile>(profile));
+  registrar_->Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
+                  content::Source<Profile>(profile));
+  registrar_->Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
+                  content::Source<Profile>(profile));
 }
 
-RulesRegistryStorageDelegate::Inner::~Inner() {}
+RulesRegistryStorageDelegate::Inner::~Inner() {
+  DCHECK(!registrar_.get());
+}
 
 void RulesRegistryStorageDelegate::Inner::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (type == chrome::NOTIFICATION_EXTENSION_LOADED) {
     const extensions::Extension* extension =
         content::Details<const extensions::Extension>(details).ptr();
     // TODO(mpcomplete): This API check should generalize to any use of
     // declarative rules, not just webRequest.
     if (extension->HasAPIPermission(
             APIPermission::kDeclarativeWebRequest)) {
       ReadFromStorage(extension->id());
     }
   } else if (type == chrome::NOTIFICATION_EXTENSIONS_READY) {
     CheckIfReady();
   }
 }
 
 void RulesRegistryStorageDelegate::Inner::ReadFromStorage(
     const std::string& extension_id) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   extensions::StateStore* store = ExtensionSystem::Get(profile_)->state_store();
   if (store) {
     waiting_for_extensions_.insert(extension_id);
     store->GetExtensionValue(extension_id, storage_key_,
         base::Bind(&Inner::ReadFromStorageCallback, this, extension_id));
   }
 }
 
 void RulesRegistryStorageDelegate::Inner::ReadFromStorageCallback(
     const std::string& extension_id, scoped_ptr<base::Value> value) {
@@ skipping 37 matching lines @@
 void RulesRegistryStorageDelegate::Inner::NotifyReadyOnRegistryThread() {
   DCHECK(content::BrowserThread::CurrentlyOn(rules_registry_thread_));
   if (ready_)
     return;  // we've already notified our readiness
 
   ready_ = true;
   rules_registry_->OnReady();
 }
 
 }  // namespace extensions