Remove profile destruction notification observation from

ExtensionStorageMonitor.

Bug: 793887
Change-Id: I877a8734b419054da94b307bab5389bada8aab62
Reviewed-on: https://chromium-review.googlesource.com/1000114
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549670}

chrome/browser/extensions/extension_storage_monitor.cc

@@ -12,7 +12,6 @@
 #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/extension_service.h"
 #include "chrome/browser/extensions/extension_storage_monitor_factory.h"
 #include "chrome/browser/extensions/extension_util.h"
@@ -23,8 +22,6 @@
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_source.h"
 #include "content/public/browser/storage_partition.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
@@ -290,22 +287,10 @@ ExtensionStorageMonitor::ExtensionStorageMonitor(Profile* profile)
       weak_ptr_factory_(this) {
   DCHECK(extension_prefs_);
 
-  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
-                 content::Source<Profile>(profile_));
-
   extension_registry_observer_.Add(ExtensionRegistry::Get(profile_));
 }
 
-ExtensionStorageMonitor::~ExtensionStorageMonitor() {}
-
-void ExtensionStorageMonitor::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK_EQ(chrome::NOTIFICATION_PROFILE_DESTROYED, type);
-
-  StopMonitoringAll();
-}
+ExtensionStorageMonitor::~ExtensionStorageMonitor() = default;
 
 void ExtensionStorageMonitor::OnExtensionLoaded(
     content::BrowserContext* browser_context,
@@ -539,13 +524,6 @@ void ExtensionStorageMonitor::StopMonitoringStorage(
           io_helper_, extension_id));
 }
 
-void ExtensionStorageMonitor::StopMonitoringAll() {
-  extension_registry_observer_.RemoveAll();
-
-  io_helper_ = nullptr;
-  weak_ptr_factory_.InvalidateWeakPtrs();
-}
-
 void ExtensionStorageMonitor::RemoveNotificationForExtension(
     const std::string& extension_id) {
   std::set<std::string>::iterator ext_id =

chrome/browser/extensions/extension_storage_monitor.h

@@ -16,8 +16,6 @@
 #include "base/scoped_observer.h"
 #include "chrome/browser/extensions/extension_uninstall_dialog.h"
 #include "components/keyed_service/core/keyed_service.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/extension_registry_observer.h"
 
 namespace gfx {
@@ -37,7 +35,6 @@ class ExtensionStorageMonitorIOHelper;
 // that are granted unlimited storage and displays notifications when high
 // usage is detected.
 class ExtensionStorageMonitor : public KeyedService,
-                                public content::NotificationObserver,
                                 public ExtensionRegistryObserver,
                                 public ExtensionUninstallDialog::Delegate {
  public:
@@ -53,11 +50,6 @@ class ExtensionStorageMonitor : public KeyedService,
   ~ExtensionStorageMonitor() override;
 
  private:
-  // content::NotificationObserver overrides:
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
-
   // ExtensionRegistryObserver overrides:
   void OnExtensionLoaded(content::BrowserContext* browser_context,
                          const Extension* extension) override;
@@ -90,7 +82,6 @@ class ExtensionStorageMonitor : public KeyedService,
   void DisableStorageMonitoring(const std::string& extension_id);
   void StartMonitoringStorage(const Extension* extension);
   void StopMonitoringStorage(const std::string& extension_id);
-  void StopMonitoringAll();
 
   void RemoveNotificationForExtension(const std::string& extension_id);
 
@@ -132,7 +123,6 @@ class ExtensionStorageMonitor : public KeyedService,
   Profile* profile_;
   extensions::ExtensionPrefs* extension_prefs_;
 
-  content::NotificationRegistrar registrar_;
   ScopedObserver<extensions::ExtensionRegistry,
                  extensions::ExtensionRegistryObserver>
       extension_registry_observer_;

chrome/browser/extensions/extension_storage_monitor_browsertest.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_storage_monitor.h"
+#include "chrome/browser/extensions/extension_storage_monitor_factory.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/ui/extensions/app_launch_params.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
@@ -41,6 +42,12 @@ const int kInitialUsageThreshold = 500;
 
 const char kWriteDataApp[] = "storage_monitor/write_data";
 
+std::unique_ptr<KeyedService> CreateExtensionStorageMonitorInstance(
+    content::BrowserContext* context) {
+  return std::make_unique<ExtensionStorageMonitor>(
+      Profile::FromBrowserContext(context));
+}
+
 class NotificationObserver {
  public:
   NotificationObserver(NotificationDisplayServiceTester* display_service,
@@ -89,7 +96,7 @@ class NotificationObserver {
 
 class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
  public:
-  ExtensionStorageMonitorTest() : storage_monitor_(NULL) {}
+  ExtensionStorageMonitorTest() = default;
 
  protected:
   // ExtensionBrowserTest overrides:
@@ -106,7 +113,7 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
 
   ExtensionStorageMonitor* monitor() {
     CHECK(storage_monitor_);
-    return storage_monitor_;
+    return storage_monitor_.get();
   }
 
   int64_t GetInitialExtensionThreshold() {
@@ -187,10 +194,18 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
     WriteBytes(extension, num_bytes, filesystem, false);
   }
 
-  void SimulateProfileShutdown() { storage_monitor_->StopMonitoringAll(); }
+  void SimulateProfileShutdown() {
+    // Setting a testing factory function deletes the current
+    // ExtensionStorageMonitor; see KeyedServiceFactory::SetTestingFactory().
+    ExtensionStorageMonitorFactory::GetInstance()->SetTestingFactoryAndUse(
+        profile(), &CreateExtensionStorageMonitorInstance);
+    InitStorageMonitor();
+  }
 
   void InitStorageMonitor() {
-    storage_monitor_ = ExtensionStorageMonitor::Get(profile());
+    EXPECT_FALSE(storage_monitor_);
+    storage_monitor_ =
+        ExtensionStorageMonitor::Get(profile())->weak_ptr_factory_.GetWeakPtr();
     ASSERT_TRUE(storage_monitor_);
 
     // Override thresholds so that we don't have to write a huge amount of data
@@ -265,7 +280,7 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
     }
   }
 
-  ExtensionStorageMonitor* storage_monitor_;
+  base::WeakPtr<ExtensionStorageMonitor> storage_monitor_;
   std::unique_ptr<NotificationDisplayServiceTester> display_service_;
   std::vector<std::unique_ptr<TestExtensionDir>> temp_dirs_;
 };