Declarative rules should be removed on uninstalling, not unloading

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/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 28 matching lines @@
 }
 
 }  // namespace
 
 RulesRegistryService::RulesRegistryService(Profile* profile)
     : content_rules_registry_(NULL),
       profile_(profile) {
   if (profile) {
     registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
         content::Source<Profile>(profile->GetOriginalProfile()));
+    registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
+        content::Source<Profile>(profile->GetOriginalProfile()));
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_EXTENSION_LOADED,
+                   content::Source<Profile>(profile_->GetOriginalProfile()));
     registrar_.Add(
         this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
         content::NotificationService::AllBrowserContextsAndSources());
     EnsureDefaultRulesRegistriesRegistered(WebViewKey(0, 0));
   }
 }
 
 RulesRegistryService::~RulesRegistryService() {}
 
 void RulesRegistryService::EnsureDefaultRulesRegistriesRegistered(
@@ skipping 109 matching lines @@
     // Modifying the container while iterating is bad so we'll save the keys we
     // wish to delete in another container, and delete them in another loop.
     registries_to_delete.insert(key);
   }
   for (std::set<RulesRegistryKey>::iterator it = registries_to_delete.begin();
        it != registries_to_delete.end(); ++it) {
     rule_registries_.erase(*it);
   }
 }
 
-void RulesRegistryService::SimulateExtensionUnloaded(
+void RulesRegistryService::SimulateExtensionUninstalled(
     const std::string& extension_id) {
-  OnExtensionUnloaded(extension_id);
+  OnExtensionUninstalled(extension_id);
 }
 
-void RulesRegistryService::OnExtensionUnloaded(
+void RulesRegistryService::NotifyRegistriesHelper(
+    void (RulesRegistry::*notification_callback)(const std::string&),
     const std::string& extension_id) {
   RulesRegistryMap::iterator i;
   for (i = rule_registries_.begin(); i != rule_registries_.end(); ++i) {
     scoped_refptr<RulesRegistry> registry = i->second;
     if (content::BrowserThread::CurrentlyOn(registry->owner_thread())) {
-      registry->OnExtensionUnloaded(extension_id);
+      (registry->*notification_callback)(extension_id);
     } else {
       content::BrowserThread::PostTask(
           registry->owner_thread(),
           FROM_HERE,
-          base::Bind(
-              &RulesRegistry::OnExtensionUnloaded, registry, extension_id));
+          base::Bind(notification_callback, registry, extension_id));
     }
   }
 }
 
+void RulesRegistryService::OnExtensionUninstalled(

[Jeffrey Yasskin, 2013/11/20 05:55:28]

I would consider inlining these 1-line functions that are each called from 1
place. The only exception is OnExtensionUninstalled, which is called from 2, but
I think that's few enough to make inlining it worthwhile anyway.

+    const std::string& extension_id) {
+  NotifyRegistriesHelper(&RulesRegistry::OnExtensionUninstalled, extension_id);
+}
+
+void RulesRegistryService::OnExtensionUnloaded(
+    const std::string& extension_id) {
+  NotifyRegistriesHelper(&RulesRegistry::OnExtensionUnloaded, extension_id);
+}
+
+void RulesRegistryService::OnExtensionLoaded(const std::string& extension_id) {
+  NotifyRegistriesHelper(&RulesRegistry::OnExtensionLoaded, extension_id);
+}
+
 void RulesRegistryService::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
       const Extension* extension =
           content::Details<UnloadedExtensionInfo>(details)->extension;
       OnExtensionUnloaded(extension->id());
       break;
     }
+    case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
+      const Extension* extension =
+          content::Details<const Extension>(details).ptr();
+      OnExtensionUninstalled(extension->id());
+      break;
+    }
+    case chrome::NOTIFICATION_EXTENSION_LOADED: {
+      const Extension* extension =
+          content::Details<const Extension>(details).ptr();
+      OnExtensionLoaded(extension->id());
+      break;
+    }
     case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: {
       content::RenderProcessHost* process =
           content::Source<content::RenderProcessHost>(source).ptr();
       RemoveWebViewRulesRegistries(process->GetID());
       break;
     }
     default:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace extensions