Remove deprecated extension notifications from APIs

chrome/browser/extensions/api/management/management_api.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/management/management_api.h"
 
-#include <map>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/management/management_api_constants.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_ui_util.h"
 #include "chrome/browser/extensions/extension_uninstall_dialog.h"
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_utility_messages.h"
 #include "chrome/common/extensions/api/management.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/extensions/manifest_url_handler.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
 #include "content/public/browser/utility_process_host.h"
 #include "content/public/browser/utility_process_host_client.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/management_policy.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
@@ skipping 799 matching lines @@
   return true;
 }
 
 ManagementGenerateAppForLinkFunction::ManagementGenerateAppForLinkFunction() {
 }
 
 ManagementGenerateAppForLinkFunction::~ManagementGenerateAppForLinkFunction() {
 }
 
 void ManagementGenerateAppForLinkFunction::FinishCreateBookmarkApp(
-    const extensions::Extension* extension,
+    const Extension* extension,
     const WebApplicationInfo& web_app_info) {
   if (extension) {
     scoped_ptr<management::ExtensionInfo> info =
         CreateExtensionInfo(*extension, ExtensionSystem::Get(GetProfile()));
     results_ = management::GenerateAppForLink::Results::Create(*info);
 
     SendResponse(true);
     Release();
   } else {
     error_ = keys::kGenerateAppForLinkInstallError;
@@ skipping 57 matching lines @@
       base::Bind(&ManagementGenerateAppForLinkFunction::OnFaviconForApp, this),
       &cancelable_task_tracker_);
 
   // Matched with a Release() in OnExtensionLoaded().
   AddRef();
 
   // Response is sent async in OnExtensionLoaded().
   return true;
 }
 
-ManagementEventRouter::ManagementEventRouter(Profile* profile)
-    : profile_(profile) {
-  int types[] = {chrome::NOTIFICATION_EXTENSION_INSTALLED_DEPRECATED,
-                 chrome::NOTIFICATION_EXTENSION_UNINSTALLED_DEPRECATED,
-                 chrome::NOTIFICATION_EXTENSION_LOADED_DEPRECATED,
-                 chrome::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED};
-
-  CHECK(registrar_.IsEmpty());
-  for (size_t i = 0; i < arraysize(types); i++) {
-    registrar_.Add(this,
-                   types[i],
-                   content::Source<Profile>(profile_));
-  }
+ManagementEventRouter::ManagementEventRouter(content::BrowserContext* context)
+    : browser_context_(context), extension_registry_observer_(this) {
+  extension_registry_observer_.Add(ExtensionRegistry::Get(browser_context_));
 }
 
 ManagementEventRouter::~ManagementEventRouter() {}
 
-void ManagementEventRouter::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  const char* event_name = NULL;
-  const Extension* extension = NULL;
-  Profile* profile = content::Source<Profile>(source).ptr();
-  CHECK(profile);
-  CHECK(profile_->IsSameProfile(profile));
+void ManagementEventRouter::OnExtensionLoaded(
+    content::BrowserContext* browser_context,
+    const Extension* extension) {
+  BroadcastEvent(extension, browser_context, management::OnEnabled::kEventName);
+}
 
-  switch (type) {
-    case chrome::NOTIFICATION_EXTENSION_INSTALLED_DEPRECATED:
-      event_name = management::OnInstalled::kEventName;
-      extension =
-          content::Details<const InstalledExtensionInfo>(details)->extension;
-      break;
-    case chrome::NOTIFICATION_EXTENSION_UNINSTALLED_DEPRECATED:
-      event_name = management::OnUninstalled::kEventName;
-      extension = content::Details<const Extension>(details).ptr();
-      break;
-    case chrome::NOTIFICATION_EXTENSION_LOADED_DEPRECATED:
-      event_name = management::OnEnabled::kEventName;
-      extension = content::Details<const Extension>(details).ptr();
-      break;
-    case chrome::NOTIFICATION_EXTENSION_UNLOADED_DEPRECATED:
-      event_name = management::OnDisabled::kEventName;
-      extension =
-          content::Details<const UnloadedExtensionInfo>(details)->extension;
-      break;
-    default:
-      NOTREACHED();
-      return;
-  }
-  DCHECK(event_name);
-  DCHECK(extension);
+void ManagementEventRouter::OnExtensionUnloaded(
+    content::BrowserContext* browser_context,
+    const Extension* extension,
+    UnloadedExtensionInfo::Reason reason) {
+  BroadcastEvent(
+      extension, browser_context, management::OnDisabled::kEventName);
+}
 
-  if (ui_util::ShouldNotBeVisible(extension, profile_))
-    return; // Don't dispatch events for built-in extensions.
+void ManagementEventRouter::OnExtensionInstalled(
+    content::BrowserContext* browser_context,
+    const Extension* extension) {
+  BroadcastEvent(
+      extension, browser_context, management::OnInstalled::kEventName);
+}
 
+void ManagementEventRouter::OnExtensionUninstalled(
+    content::BrowserContext* browser_context,
+    const Extension* extension) {
+  if (ui_util::ShouldNotBeVisible(extension, browser_context_))
+    return;
   scoped_ptr<base::ListValue> args(new base::ListValue());
-  if (event_name == management::OnUninstalled::kEventName) {
-    args->Append(new base::StringValue(extension->id()));
-  } else {
-    scoped_ptr<management::ExtensionInfo> info = CreateExtensionInfo(
-        *extension, ExtensionSystem::Get(profile));
-    args->Append(info->ToValue().release());
-  }
+  args->Append(new base::StringValue(extension->id()));
+
+  scoped_ptr<Event> event(
+      new Event(management::OnUninstalled::kEventName, args.Pass()));
+  EventRouter::Get(browser_context)->BroadcastEvent(event.Pass());
+}
+
+void ManagementEventRouter::BroadcastEvent(
+    const Extension* extension,
+    content::BrowserContext* browser_context,
+    const char* event_name) {
+  if (ui_util::ShouldNotBeVisible(extension, browser_context_))
+    return;  // Don't dispatch events for built-in extensions.
+  scoped_ptr<base::ListValue> args(new base::ListValue());
+  scoped_ptr<management::ExtensionInfo> info =

[Devlin, 2014/06/18 16:04:34]

A few small things:
- You actually don't need to pass in the browser context - it should always be
from this objects |browser_context_|.
- You can inline the event into the EventRouter broadcast.
- I think I'd like to keep all the logic for dispatching in one place, and just
put in the if statement again.  The reason is it makes it easier to modify the
dispatch logic later on.  For instance, if we decided to take out the
early-return on ShouldNotBeVisible(), someone might overlook the dispatch in
OnExtensionUninstalled().

So,

void ManagementEventRouter::BroadcastEvent(const Extension* extension,
                                           const char* event_name) {
  if (ui_util::ShouldNotBeVisible(extension, browser_context_))
    return;  // Don't dispatch events for built-in extenions.
  scoped_ptr<base::ListValue> args(new base::ListValue());
  if (event_name == management::OnUninstalled::kEventName) {
    args->Append(new base::StringValue(extension->id());
  } else {
    scoped_ptr<management::ExtensionInfo> info = CreateExtensionInfo(
        *extension, ExtensionSystem::Get(browser_context_));
    args->Append(info->ToValue().release());
  }

  EventRouter::Get(browser_context_)->BroadcastEvent(
      scoped_ptr<Event>(new Event(event_name, args.Pass())));
}

[limasdf, 2014/06/19 01:48:42]

Done.
Nice. I have learned good point :)

+      CreateExtensionInfo(*extension, ExtensionSystem::Get(browser_context));
+  args->Append(info->ToValue().release());
 
   scoped_ptr<Event> event(new Event(event_name, args.Pass()));
-  EventRouter::Get(profile)->BroadcastEvent(event.Pass());
+  EventRouter::Get(browser_context)->BroadcastEvent(event.Pass());
 }
 
 ManagementAPI::ManagementAPI(content::BrowserContext* context)
     : browser_context_(context) {
   EventRouter* event_router = EventRouter::Get(browser_context_);
   event_router->RegisterObserver(this, management::OnInstalled::kEventName);
   event_router->RegisterObserver(this, management::OnUninstalled::kEventName);
   event_router->RegisterObserver(this, management::OnEnabled::kEventName);
   event_router->RegisterObserver(this, management::OnDisabled::kEventName);
 }
 
 ManagementAPI::~ManagementAPI() {
 }
 
 void ManagementAPI::Shutdown() {
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
 static base::LazyInstance<BrowserContextKeyedAPIFactory<ManagementAPI> >
     g_factory = LAZY_INSTANCE_INITIALIZER;
 
 // static
 BrowserContextKeyedAPIFactory<ManagementAPI>*
 ManagementAPI::GetFactoryInstance() {
   return g_factory.Pointer();
 }
 
 void ManagementAPI::OnListenerAdded(const EventListenerInfo& details) {
-  management_event_router_.reset(
-      new ManagementEventRouter(Profile::FromBrowserContext(browser_context_)));
+  management_event_router_.reset(new ManagementEventRouter(browser_context_));
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
 }  // namespace extensions