Modify shutdown sequence of NotificationDisplayServiceTester.

This is a more general fix for the CFI issues seen while updating tests
to use NotificationDisplayServiceTester. While referencing |profile_|
after shutdown would typically be sorta safe in this case, the CFI bot
objects because it detects a cast on a destroyed object.

The old solution, administered somewhat inconsistently, was to destroy
the NotificationDisplayServiceTester before the profile. However this
also has issues, because some code references the
NotificationDisplayService during profile shutdown. Hence it is not
safe for NDSTester to outlive its profile, and not safe not to outlive
its profile.

The solution is to allow it to outlive its profile but not reference
the profile after it's been destroyed.

TBR=rdevlin.cronin@chromium.org,atwilson@chromium.org

Bug: 804231
Change-Id: I17f29befb18f0667b2f15ca2bb7554af1854b6d1
Reviewed-on: https://chromium-review.googlesource.com/883224Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531950}

chrome/browser/background/background_contents_service_unittest.cc

@@ -127,11 +127,6 @@ class BackgroundContentsServiceNotificationTest
         std::make_unique<NotificationDisplayServiceTester>(profile());
   }
 
-  void TearDown() override {
-    display_service_.reset();
-    BrowserWithTestWindowTest::TearDown();
-  }
-
  protected:
   // Creates crash notification for the specified extension and returns
   // the created one.

chrome/browser/extensions/extension_storage_monitor_browsertest.cc

@@ -104,11 +104,6 @@ class ExtensionStorageMonitorTest : public ExtensionBrowserTest {
     InitStorageMonitor();
   }
 
-  void TearDownOnMainThread() override {
-    display_service_.reset();
-    ExtensionBrowserTest::TearDownOnMainThread();
-  }
-
   ExtensionStorageMonitor* monitor() {
     CHECK(storage_monitor_);
     return storage_monitor_;

chrome/browser/notifications/notification_display_service_tester.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/notifications/stub_notification_display_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/test/base/testing_browser_process.h"
+#include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "ui/base/ui_features.h"
 #include "ui/message_center/notification.h"
@@ -58,6 +59,28 @@ class MockNotificationPlatformBridge : public NotificationPlatformBridge {
 
 #endif  // !BUILDFLAG(ENABLE_MESSAGE_CENTER)
 
+class NotificationDisplayServiceShutdownNotifierFactory
+    : public BrowserContextKeyedServiceShutdownNotifierFactory {
+ public:
+  static NotificationDisplayServiceShutdownNotifierFactory* GetInstance() {
+    return base::Singleton<
+        NotificationDisplayServiceShutdownNotifierFactory>::get();
+  }
+
+ private:
+  friend struct base::DefaultSingletonTraits<
+      NotificationDisplayServiceShutdownNotifierFactory>;
+
+  NotificationDisplayServiceShutdownNotifierFactory()
+      : BrowserContextKeyedServiceShutdownNotifierFactory(
+            "NotificationDisplayService") {
+    DependsOn(NotificationDisplayServiceFactory::GetInstance());
+  }
+  ~NotificationDisplayServiceShutdownNotifierFactory() override {}
+
+  DISALLOW_COPY_AND_ASSIGN(NotificationDisplayServiceShutdownNotifierFactory);
+};
+
 }  // namespace
 
 NotificationDisplayServiceTester::NotificationDisplayServiceTester(
@@ -78,13 +101,23 @@ NotificationDisplayServiceTester::NotificationDisplayServiceTester(
   display_service_ = static_cast<StubNotificationDisplayService*>(
       NotificationDisplayServiceFactory::GetInstance()->SetTestingFactoryAndUse(
           profile_, &StubNotificationDisplayService::FactoryForTests));
+
+  profile_shutdown_subscription_ =
+      NotificationDisplayServiceShutdownNotifierFactory::GetInstance()
+          ->Get(profile)
+          ->Subscribe(base::BindRepeating(
+              &NotificationDisplayServiceTester::OnProfileShutdown,
+              base::Unretained(this)));
+
   g_tester = this;
 }
 
 NotificationDisplayServiceTester::~NotificationDisplayServiceTester() {
   g_tester = nullptr;
-  NotificationDisplayServiceFactory::GetInstance()->SetTestingFactory(profile_,
-                                                                      nullptr);
+  if (profile_) {
+    NotificationDisplayServiceFactory::GetInstance()->SetTestingFactory(
+        profile_, nullptr);
+  }
 }
 
 // static
@@ -149,3 +182,8 @@ void NotificationDisplayServiceTester::SetProcessNotificationOperationDelegate(
         delegate) {
   display_service_->SetProcessNotificationOperationDelegate(delegate);
 }
+
+void NotificationDisplayServiceTester::OnProfileShutdown() {
+  profile_ = nullptr;
+  profile_shutdown_subscription_.reset();
+}

chrome/browser/notifications/notification_display_service_tester.h

@@ -13,6 +13,7 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/notifications/notification_common.h"
 #include "chrome/browser/notifications/stub_notification_display_service.h"
+#include "components/keyed_service/core/keyed_service_shutdown_notifier.h"
 
 class Profile;
 
@@ -78,8 +79,12 @@ class NotificationDisplayServiceTester {
           ProcessNotificationOperationCallback& delegate);
 
  private:
+  void OnProfileShutdown();
+
   Profile* profile_;
   StubNotificationDisplayService* display_service_;
+  std::unique_ptr<KeyedServiceShutdownNotifier::Subscription>
+      profile_shutdown_subscription_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationDisplayServiceTester);
 };