Revert "Use NotificationDisplayService for ExtensionStorageMonitor notification."

This reverts commit 37bd8dbf.

Reason for revert: browser test failures
ExtensionStorageMonitorTest.ThrottleNotifications
ExtensionStorageMonitorTest.DisableForInstalledExtensions
ExtensionStorageMonitorTest.DoubleInitialThreshold
ExtensionStorageMonitorTest.UnderThreshold
ExtensionStorageMonitorTest.TwoHostedAppsInSameOrigin
ExtensionStorageMonitorTest.HostedAppTemporaryFilesystem
ExtensionStorageMonitorTest.UserDisabledNotifications
ExtensionStorageMonitorTest.ExceedInitialThreshold

first failed build https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20CFI/builds/4334

Original change's description:
> Use NotificationDisplayService for ExtensionStorageMonitor notification.
> 
> Bug: 783018
> Change-Id: If8fe05a25344ce61f5388e13b2d65a58aa24e5ae
> Reviewed-on: https://chromium-review.googlesource.com/816051
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Nick Carter <nick@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522872}

TBR=stevenjb@chromium.org,nick@chromium.org,rdevlin.cronin@chromium.org,estade@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 783018
Change-Id: Ia32a0edadd1858c024a67ef80f247edfc1a4d04e
Reviewed-on: https://chromium-review.googlesource.com/817297Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523063}

chrome/browser/extensions/extension_storage_monitor.cc

@@ -15,8 +15,6 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_storage_monitor_factory.h"
 #include "chrome/browser/extensions/extension_util.h"
-#include "chrome/browser/notifications/notification_display_service.h"
-#include "chrome/browser/notifications/notification_handler.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/grit/generated_resources.h"
@@ -37,7 +35,7 @@
 #include "storage/browser/quota/quota_manager.h"
 #include "storage/browser/quota/storage_observer.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/message_center/notification.h"
+#include "ui/message_center/message_center.h"
 #include "ui/message_center/notifier_id.h"
 #include "ui/message_center/views/constants.h"
 
@@ -274,24 +272,26 @@ void SingleExtensionStorageObserver::OnStorageEvent(const Event& event) {
 // ExtensionStorageMonitor
 
 // static
-ExtensionStorageMonitor* ExtensionStorageMonitor::Get(Profile* profile) {
-  return ExtensionStorageMonitorFactory::GetForBrowserContext(profile);
+ExtensionStorageMonitor* ExtensionStorageMonitor::Get(
+    content::BrowserContext* context) {
+  return ExtensionStorageMonitorFactory::GetForBrowserContext(context);
 }
 
-ExtensionStorageMonitor::ExtensionStorageMonitor(Profile* profile)
+ExtensionStorageMonitor::ExtensionStorageMonitor(
+    content::BrowserContext* context)
     : enable_for_all_extensions_(false),
       initial_extension_threshold_(kExtensionInitialThreshold),
       observer_rate_(kStorageEventRate),
-      profile_(profile),
-      extension_prefs_(ExtensionPrefs::Get(profile)),
+      context_(context),
+      extension_prefs_(ExtensionPrefs::Get(context)),
       extension_registry_observer_(this),
       weak_ptr_factory_(this) {
   DCHECK(extension_prefs_);
 
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
-                 content::Source<Profile>(profile_));
+                 content::Source<content::BrowserContext>(context_));
 
-  extension_registry_observer_.Add(ExtensionRegistry::Get(profile_));
+  extension_registry_observer_.Add(ExtensionRegistry::Get(context_));
 }
 
 ExtensionStorageMonitor::~ExtensionStorageMonitor() {}
@@ -370,7 +370,7 @@ void ExtensionStorageMonitor::OnExtensionUninstallDialogClosed(
 std::string ExtensionStorageMonitor::GetNotificationId(
     const std::string& extension_id) {
   std::vector<std::string> placeholders;
-  placeholders.push_back(profile_->GetPath().BaseName().MaybeAsASCII());
+  placeholders.push_back(context_->GetPath().BaseName().MaybeAsASCII());
   placeholders.push_back(extension_id);
 
   return base::ReplaceStringPlaceholders(
@@ -383,7 +383,7 @@ void ExtensionStorageMonitor::OnStorageThresholdExceeded(
     int64_t current_usage) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  const Extension* extension = GetExtensionById(profile_, extension_id);
+  const Extension* extension = GetExtensionById(context_, extension_id);
   if (!extension)
     return;
 
@@ -393,23 +393,25 @@ void ExtensionStorageMonitor::OnStorageThresholdExceeded(
   const int kIconSize = message_center::kNotificationIconSize;
   ExtensionResource resource =  IconsInfo::GetIconResource(
       extension, kIconSize, ExtensionIconSet::MATCH_BIGGER);
-  ImageLoader::Get(profile_)->LoadImageAsync(
+  ImageLoader::Get(context_)->LoadImageAsync(
       extension, resource, gfx::Size(kIconSize, kIconSize),
       base::Bind(&ExtensionStorageMonitor::OnImageLoaded,
-                 weak_ptr_factory_.GetWeakPtr(), extension_id, current_usage));
+                 weak_ptr_factory_.GetWeakPtr(),
+                 extension_id,
+                 current_usage));
 }
 
 void ExtensionStorageMonitor::OnImageLoaded(const std::string& extension_id,
                                             int64_t current_usage,
                                             const gfx::Image& image) {
-  const Extension* extension = GetExtensionById(profile_, extension_id);
+  const Extension* extension = GetExtensionById(context_, extension_id);
   if (!extension)
     return;
 
   // Remove any existing notifications to force a new notification to pop up.
   std::string notification_id(GetNotificationId(extension_id));
-  NotificationDisplayService::GetForProfile(profile_)->Close(
-      NotificationHandler::Type::TRANSIENT, notification_id);
+  message_center::MessageCenter::Get()->RemoveNotification(
+      notification_id, false);
 
   message_center::RichNotificationData notification_data;
   notification_data.buttons.push_back(message_center::ButtonInfo(
@@ -428,7 +430,8 @@ void ExtensionStorageMonitor::OnImageLoaded(const std::string& extension_id,
                             : gfx::Image(util::GetDefaultExtensionIcon());
   }
 
-  message_center::Notification notification(
+  std::unique_ptr<message_center::Notification> notification;
+  notification.reset(new message_center::Notification(
       message_center::NOTIFICATION_TYPE_SIMPLE, notification_id,
       l10n_util::GetStringUTF16(IDS_EXTENSION_STORAGE_MONITOR_TITLE),
       l10n_util::GetStringFUTF16(
@@ -441,10 +444,10 @@ void ExtensionStorageMonitor::OnImageLoaded(const std::string& extension_id,
       notification_data,
       new message_center::HandleNotificationClickDelegate(
           base::Bind(&ExtensionStorageMonitor::OnNotificationButtonClick,
-                     weak_ptr_factory_.GetWeakPtr(), extension_id)));
-  notification.SetSystemPriority();
-  NotificationDisplayService::GetForProfile(profile_)->Display(
-      NotificationHandler::Type::TRANSIENT, notification);
+                     weak_ptr_factory_.GetWeakPtr(), extension_id))));
+  notification->SetSystemPriority();
+  message_center::MessageCenter::Get()->AddNotification(
+      std::move(notification));
 
   notified_extension_ids_.insert(extension_id);
 }
@@ -472,15 +475,15 @@ void ExtensionStorageMonitor::OnNotificationButtonClick(
 void ExtensionStorageMonitor::DisableStorageMonitoring(
     const std::string& extension_id) {
   scoped_refptr<const Extension> extension =
-      ExtensionRegistry::Get(profile_)->enabled_extensions().GetByID(
+      ExtensionRegistry::Get(context_)->enabled_extensions().GetByID(
           extension_id);
   if (!extension.get() || !ShouldGatherMetricsFor(extension.get()))
     StopMonitoringStorage(extension_id);
 
   SetStorageNotificationEnabled(extension_id, false);
 
-  NotificationDisplayService::GetForProfile(profile_)->Close(
-      NotificationHandler::Type::TRANSIENT, GetNotificationId(extension_id));
+  message_center::MessageCenter::Get()->RemoveNotification(
+      GetNotificationId(extension_id), false);
 }
 
 void ExtensionStorageMonitor::StartMonitoringStorage(
@@ -502,9 +505,9 @@ void ExtensionStorageMonitor::StartMonitoringStorage(
         weak_ptr_factory_.GetWeakPtr());
   }
 
-  GURL site_url = util::GetSiteForExtensionId(extension->id(), profile_);
+  GURL site_url = util::GetSiteForExtensionId(extension->id(), context_);
   content::StoragePartition* storage_partition =
-      content::BrowserContext::GetStoragePartitionForSite(profile_, site_url);
+      content::BrowserContext::GetStoragePartitionForSite(context_, site_url);
   DCHECK(storage_partition);
   scoped_refptr<storage::QuotaManager> quota_manager(
       storage_partition->GetQuotaManager());
@@ -554,30 +557,31 @@ void ExtensionStorageMonitor::RemoveNotificationForExtension(
     return;
 
   notified_extension_ids_.erase(ext_id);
-  NotificationDisplayService::GetForProfile(profile_)->Close(
-      NotificationHandler::Type::TRANSIENT, GetNotificationId(extension_id));
+  message_center::MessageCenter::Get()->RemoveNotification(
+      GetNotificationId(extension_id), false);
 }
 
 void ExtensionStorageMonitor::RemoveAllNotifications() {
   if (notified_extension_ids_.empty())
     return;
 
-  auto* display_service = NotificationDisplayService::GetForProfile(profile_);
-  for (const std::string& extension_id : notified_extension_ids_) {
-    display_service->Close(NotificationHandler::Type::TRANSIENT,
-                           GetNotificationId(extension_id));
+  message_center::MessageCenter* center = message_center::MessageCenter::Get();
+  DCHECK(center);
+  for (std::set<std::string>::iterator it = notified_extension_ids_.begin();
+       it != notified_extension_ids_.end(); ++it) {
+    center->RemoveNotification(GetNotificationId(*it), false);
   }
   notified_extension_ids_.clear();
 }
 
 void ExtensionStorageMonitor::ShowUninstallPrompt(
     const std::string& extension_id) {
-  const Extension* extension = GetExtensionById(profile_, extension_id);
+  const Extension* extension = GetExtensionById(context_, extension_id);
   if (!extension)
     return;
 
-  uninstall_dialog_.reset(
-      ExtensionUninstallDialog::Create(profile_, nullptr, this));
+  uninstall_dialog_.reset(ExtensionUninstallDialog::Create(
+      Profile::FromBrowserContext(context_), NULL, this));
 
   uninstall_extension_id_ = extension->id();
   uninstall_dialog_->ConfirmUninstall(

chrome/browser/extensions/extension_storage_monitor.h

@@ -20,12 +20,14 @@
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/extension_registry_observer.h"
 
+namespace content {
+class BrowserContext;
+}
+
 namespace gfx {
 class Image;
 }
 
-class Profile;
-
 namespace extensions {
 
 class Extension;
@@ -41,7 +43,7 @@ class ExtensionStorageMonitor : public KeyedService,
                                 public ExtensionRegistryObserver,
                                 public ExtensionUninstallDialog::Delegate {
  public:
-  static ExtensionStorageMonitor* Get(Profile* profile);
+  static ExtensionStorageMonitor* Get(content::BrowserContext* context);
 
   // Indices of buttons in the notification. Exposed for testing.
   enum ButtonIndex {
@@ -49,7 +51,7 @@ class ExtensionStorageMonitor : public KeyedService,
     BUTTON_UNINSTALL
   };
 
-  explicit ExtensionStorageMonitor(Profile* profile);
+  explicit ExtensionStorageMonitor(content::BrowserContext* context);
   ~ExtensionStorageMonitor() override;
 
  private:
@@ -130,7 +132,7 @@ class ExtensionStorageMonitor : public KeyedService,
   // IDs of extensions that notifications were shown for.
   std::set<std::string> notified_extension_ids_;
 
-  Profile* profile_;
+  content::BrowserContext* context_;
   extensions::ExtensionPrefs* extension_prefs_;
 
   content::NotificationRegistrar registrar_;

chrome/browser/extensions/extension_storage_monitor_browsertest.cc

@@ -9,14 +9,12 @@
 #include <vector>
 
 #include "base/message_loop/message_loop.h"
-#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #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/test_extension_dir.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"
 #include "content/public/test/browser_test_utils.h"
@@ -30,7 +28,8 @@
 #include "extensions/common/value_builder.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/dns/mock_host_resolver.h"
-#include "ui/message_center/notification.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/message_center_observer.h"
 
 namespace extensions {
 
@@ -40,23 +39,20 @@ const int kInitialUsageThreshold = 500;
 
 const char kWriteDataApp[] = "storage_monitor/write_data";
 
-class NotificationObserver {
+class NotificationObserver : public message_center::MessageCenterObserver {
  public:
-  NotificationObserver(NotificationDisplayServiceTester* display_service,
-                       const std::string& target_notification)
-      : display_service_(display_service),
-        target_notification_id_(target_notification) {
-    // Don't count old notifications.
-    display_service_->RemoveAllNotifications(
-        NotificationHandler::Type::TRANSIENT, false);
+  explicit NotificationObserver(const std::string& target_notification)
+      : message_center_(message_center::MessageCenter::Get()),
+        target_notification_id_(target_notification),
+        waiting_(false) {
+    message_center_->AddObserver(this);
   }
 
-  ~NotificationObserver() {
-    display_service_->SetNotificationAddedClosure(base::RepeatingClosure());
-  }
+  ~NotificationObserver() override { message_center_->RemoveObserver(this); }
 
   bool HasReceivedNotification() const {
-    return !!display_service_->GetNotification(target_notification_id_);
+    return received_notifications_.find(target_notification_id_) !=
+      received_notifications_.end();
   }
 
   // Runs the message loop and returns true if a notification is received.
@@ -66,22 +62,24 @@ class NotificationObserver {
       return true;
 
     waiting_ = true;
-    display_service_->SetNotificationAddedClosure(base::BindRepeating(
-        &NotificationObserver::OnNotificationAdded, base::Unretained(this)));
     content::RunMessageLoop();
     waiting_ = false;
     return HasReceivedNotification();
   }
 
  private:
-  void OnNotificationAdded() {
+  // MessageCenterObserver implementation:
+  void OnNotificationAdded(const std::string& notification_id) override {
+    received_notifications_.insert(notification_id);
+
     if (waiting_ && HasReceivedNotification())
       base::RunLoop::QuitCurrentWhenIdleDeprecated();
   }
 
-  NotificationDisplayServiceTester* display_service_;
+  message_center::MessageCenter* message_center_;
+  std::set<std::string> received_notifications_;
   std::string target_notification_id_;
-  bool waiting_ = false;
+  bool waiting_;
 };
 
 }  // namespace
@@ -95,9 +93,6 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
   void SetUpOnMainThread() override {
     ExtensionBrowserTest::SetUpOnMainThread();
 
-    display_service_ =
-        std::make_unique<NotificationDisplayServiceTester>(profile());
-
     host_resolver()->AddRule("*", "127.0.0.1");
 
     InitStorageMonitor();
@@ -188,6 +183,7 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
 
   void SimulateProfileShutdown() { storage_monitor_->StopMonitoringAll(); }
 
+ private:
   void InitStorageMonitor() {
     storage_monitor_ = ExtensionStorageMonitor::Get(profile());
     ASSERT_TRUE(storage_monitor_);
@@ -247,7 +243,7 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
                   const std::string& filesystem,
                   bool expected_notification) {
     NotificationObserver notification_observer(
-        display_service_.get(), GetNotificationId(extension->id()));
+        GetNotificationId(extension->id()));
 
     if (extension->is_hosted_app()) {
       WriteBytesForHostedApp(extension, num_bytes, filesystem);
@@ -265,7 +261,6 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
   }
 
   ExtensionStorageMonitor* storage_monitor_;
-  std::unique_ptr<NotificationDisplayServiceTester> display_service_;
   std::vector<std::unique_ptr<TestExtensionDir>> temp_dirs_;
 };
 
@@ -316,9 +311,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionStorageMonitorTest, UserDisabledNotifications) {
   EXPECT_TRUE(IsStorageNotificationEnabled(extension->id()));
 
   // Fake clicking the notification button to disable notifications.
-  display_service_->GetNotification(GetNotificationId(extension->id()))
-      ->delegate()
-      ->ButtonClick(ExtensionStorageMonitor::BUTTON_DISABLE_NOTIFICATION);
+  message_center::MessageCenter::Get()->ClickOnNotificationButton(
+      GetNotificationId(extension->id()),
+      ExtensionStorageMonitor::BUTTON_DISABLE_NOTIFICATION);
 
   EXPECT_FALSE(IsStorageNotificationEnabled(extension->id()));
 
@@ -419,9 +414,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionStorageMonitorTest,
       ScopedTestDialogAutoConfirm::ACCEPT);
   TestExtensionRegistryObserver observer(ExtensionRegistry::Get(profile()),
                                          extension->id());
-  display_service_->GetNotification(GetNotificationId(extension->id()))
-      ->delegate()
-      ->ButtonClick(ExtensionStorageMonitor::BUTTON_UNINSTALL);
+  message_center::MessageCenter::Get()->ClickOnNotificationButton(
+      GetNotificationId(extension->id()),
+      ExtensionStorageMonitor::BUTTON_UNINSTALL);
   observer.WaitForExtensionUninstalled();
 }
 

chrome/browser/extensions/extension_storage_monitor_factory.cc

@@ -6,7 +6,6 @@
 
 #include "chrome/browser/extensions/extension_storage_monitor.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "extensions/browser/extension_prefs_factory.h"
 #include "extensions/browser/extensions_browser_client.h"
@@ -39,7 +38,7 @@ ExtensionStorageMonitorFactory::~ExtensionStorageMonitorFactory() {
 
 KeyedService* ExtensionStorageMonitorFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  return new ExtensionStorageMonitor(Profile::FromBrowserContext(context));
+  return new ExtensionStorageMonitor(context);
 }
 
 content::BrowserContext* ExtensionStorageMonitorFactory::GetBrowserContextToUse(

chrome/browser/notifications/notification_display_service_tester.cc

@@ -51,7 +51,7 @@ NotificationDisplayServiceTester::GetDisplayedNotificationsForType(
 
 base::Optional<message_center::Notification>
 NotificationDisplayServiceTester::GetNotification(
-    const std::string& notification_id) const {
+    const std::string& notification_id) {
   return display_service_->GetNotification(notification_id);
 }
 

chrome/browser/notifications/notification_display_service_tester.h

@@ -46,7 +46,7 @@ class NotificationDisplayServiceTester {
       const message_center::Notification& notification);
 
   base::Optional<message_center::Notification> GetNotification(
-      const std::string& notification_id) const;
+      const std::string& notification_id);
 
   // Simulates the notification identified by |notification_id| being clicked
   // on, optionally with the given |action_index| and |reply|.

chrome/browser/notifications/stub_notification_display_service.cc

@@ -182,12 +182,11 @@ void StubNotificationDisplayService::Display(
     DCHECK(!handler);
   else
     handler->OnShow(profile_, notification.id());
+  if (notification_added_closure_)
+    notification_added_closure_.Run();
 
   notifications_.emplace_back(notification_type, notification,
                               std::move(metadata));
-
-  if (notification_added_closure_)
-    notification_added_closure_.Run();
 }
 
 void StubNotificationDisplayService::Close(