Remove InstallTracker's extension load, unload observer

chrome/browser/ui/app_list/search/app_result.cc

 // Copyright 2013 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/ui/app_list/search/app_result.h"
 
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
-#include "chrome/browser/extensions/install_tracker.h"
-#include "chrome/browser/extensions/install_tracker_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_context_menu.h"
 #include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
 #include "chrome/browser/ui/app_list/search/tokenized_string.h"
 #include "chrome/browser/ui/app_list/search/tokenized_string_match.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "chrome/browser/ui/webui/ntp/core_app_launcher_handler.h"
 #include "content/public/browser/user_metrics.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_system_provider.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_icon_set.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image_skia_operations.h"
 
 namespace app_list {
 
 AppResult::AppResult(Profile* profile,
                      const std::string& app_id,
                      AppListControllerDelegate* controller)
     : profile_(profile),
       app_id_(app_id),
       controller_(controller),
-      install_tracker_(NULL) {
+      extension_registry_(NULL) {
   set_id(extensions::Extension::GetBaseURLFromExtensionId(app_id_).spec());
 
   const extensions::Extension* extension =
       extensions::ExtensionSystem::Get(profile_)->extension_service()
           ->GetInstalledExtension(app_id_);
   DCHECK(extension);
 
   is_platform_app_ = extension->is_platform_app();
 
   icon_.reset(new extensions::IconImage(
       profile_,
       extension,
       extensions::IconsInfo::GetIcons(extension),
       extension_misc::EXTENSION_ICON_SMALL,
       extensions::util::GetDefaultAppIcon(),
       this));
   UpdateIcon();
-  StartObservingInstall();
+
+  StartObservingExtensionRegistry();
 }
 
 AppResult::~AppResult() {
-  StopObservingInstall();
+  StopObservingExtensionRegistry();
 }
 
 void AppResult::UpdateFromMatch(const TokenizedString& title,
                                 const TokenizedStringMatch& match) {
   const TokenizedStringMatch::Hits& hits = match.hits();
 
   Tags tags;
   tags.reserve(hits.size());
   for (size_t i = 0; i < hits.size(); ++i)
     tags.push_back(Tag(Tag::MATCH, hits[i].start(), hits[i].end()));
@@ skipping 44 matching lines @@
   if (!context_menu_) {
     context_menu_.reset(new AppContextMenu(
         this, profile_, app_id_, controller_));
     context_menu_->set_is_platform_app(is_platform_app_);
     context_menu_->set_is_search_result(true);
   }
 
   return context_menu_->GetMenuModel();
 }
 
-void AppResult::StartObservingInstall() {
-  DCHECK(!install_tracker_);
+void AppResult::StartObservingExtensionRegistry() {
+  DCHECK(!extension_registry_);
 
-  install_tracker_ = extensions::InstallTrackerFactory::GetForProfile(profile_);
-  install_tracker_->AddObserver(this);
+  extension_registry_ = extensions::ExtensionRegistry::Get(profile_);
+  extension_registry_->AddObserver(this);
 }
 
-void AppResult::StopObservingInstall() {
-  if (install_tracker_)
-    install_tracker_->RemoveObserver(this);
-
-  install_tracker_ = NULL;
+void AppResult::StopObservingExtensionRegistry() {
+  if (extension_registry_)
+    extension_registry_->RemoveObserver(this);

[tfarina, 2014/05/30 23:05:15]

right, so if ExtensionRegistry's |observers_| was declared as:

ObserverList<ExtensionRegistryObserver, true> observers_; 

Then, if |extension_registry_| is NULL here, it would mean we had registered (in
the ctor), but never unregistered.

And the DCHECK in ObserverList would have been fired:

https://chromium.googlesource.com/chromium/chromium/+/trunk/base/observer_lis...

Is it possible that |extension_registry_| will be NULL at the time AppResult is
destroyed?

[not at google - send to devlin, 2014/05/30 23:22:17]

I think it would mean that StopObservingExtensionRegistry was called twice, e.g.
first from OnShutdown (which gets called as part of Profile shutdown; it's not
clear to me that AppResult is tied to Profile shutdown) and then on destruction.

if it was called in the other order then we have problems.

(Of course, it's not declared as ObserverList<ExtensionRegistryObserver, true>
but rather than ObserverList<ExtensionRegistryObserver>, but including the
"true" there is probably a good idea.)

I'm not sure if that answered your question...

+  extension_registry_ = NULL;
 }
 
 bool AppResult::RunExtensionEnableFlow() {
   if (extensions::util::IsAppLaunchableWithoutEnabling(app_id_, profile_))
     return false;
 
   if (!extension_enable_flow_) {
     controller_->OnShowChildDialog();
 
     extension_enable_flow_.reset(new ExtensionEnableFlow(
@@ skipping 30 matching lines @@
 
   // Automatically open app after enabling.
   Open(ui::EF_NONE);
 }
 
 void AppResult::ExtensionEnableFlowAborted(bool user_initiated) {
   extension_enable_flow_.reset();
   controller_->OnCloseChildDialog();
 }
 
-void AppResult::OnExtensionLoaded(const extensions::Extension* extension) {
+void AppResult::OnExtensionLoaded(content::BrowserContext* browser_context,
+                                  const extensions::Extension* extension) {
   UpdateIcon();
 }
 
-void AppResult::OnExtensionUninstalled(const extensions::Extension* extension) {
+void AppResult::OnExtensionUninstalled(content::BrowserContext* browser_context,
+                                       const extensions::Extension* extension) {
   if (extension->id() != app_id_)
     return;
 
   NotifyItemUninstalled();
 }
 
-void AppResult::OnShutdown() { StopObservingInstall(); }
+void AppResult::OnShutdown() {
+  StopObservingExtensionRegistry();
+}
 
 }  // namespace app_list