Remove closed notifications from expected map

The CL in crrev.com/c/2159911 introduced an in-memory map of
notifications that are expected to be displayed. This map is then used
to detect when the OS or user closes a notification. If however Chrome
closes the notification before that we need to remove the entry from the
map so we don't try to close it twice.

Bug: None
Change-Id: I0bae9af5b6ac01e26c1969a77b149e375c1115a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2450070Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814193}

chrome/browser/notifications/notification_platform_bridge_win.cc

@@ -431,6 +431,9 @@ class NotificationPlatformBridgeWinImpl
       DLOG(ERROR) << "Failed to remove notification with id "
                   << notification_id.c_str() << " " << std::hex << hr;
     } else {
+      // We expect the notification to be removed from the action center now.
+      displayed_notifications_.erase({profile_id, notification_id});
+
       LogCloseHistogram(CloseStatus::SUCCESS);
     }
   }

chrome/browser/notifications/notification_platform_bridge_win.h

@@ -76,6 +76,8 @@ class NotificationPlatformBridgeWin : public NotificationPlatformBridge {
                            DisplayWithFakeAC);
   FRIEND_TEST_ALL_PREFIXES(NotificationPlatformBridgeWinUITest,
                            SynchronizeNotifications);
+  FRIEND_TEST_ALL_PREFIXES(NotificationPlatformBridgeWinUITest,
+                           SynchronizeNotificationsAfterClose);
 
   void SynchronizeNotificationsForTesting();
 

chrome/browser/notifications/notification_platform_bridge_win_interactive_uitest.cc

@@ -13,11 +13,13 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/win/scoped_hstring.h"
 #include "base/win/windows_version.h"
@@ -467,16 +469,12 @@ IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest,
       notifications;
   bridge->SetDisplayedNotificationsForTesting(&notifications);
 
-  bool incognito = true;
-
   notifications.push_back(Microsoft::WRL::Make<FakeIToastNotification>(
       GetToastString(L"P1i", L"Default", true), L"tag"));
   expected_displayed_notifications[{/*profile_id=*/"Default",
                                     /*notification_id=*/"P1i"}] =
       GetNotificationLaunchId(notifications.back().Get());
 
-  incognito = false;
-
   expected_displayed_notifications[{/*profile_id=*/"Default",
                                     /*notification_id=*/"P2i"}] =
       GetNotificationLaunchId(
@@ -515,6 +513,53 @@ IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest,
   bridge->SetExpectedDisplayedNotificationsForTesting(nullptr);
 }
 
+IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest,
+                       SynchronizeNotificationsAfterClose) {
+  if (base::win::GetVersion() < kMinimumWindowsVersion)
+    return;
+
+  NotificationPlatformBridgeWin* bridge = GetBridge();
+  ASSERT_TRUE(bridge);
+  FakeIToastNotifier notifier;
+  bridge->SetNotifierForTesting(&notifier);
+
+  // Show a new notification.
+  message_center::Notification notification(
+      message_center::NOTIFICATION_TYPE_SIMPLE, "notification_id", L"Text1",
+      L"Text2", gfx::Image(), base::string16(), GURL("https://example.com/"),
+      message_center::NotifierId(), message_center::RichNotificationData(),
+      nullptr);
+  base::RunLoop display_run_loop;
+  notifier.SetNotificationShownCallback(base::BindLambdaForTesting(
+      [&display_run_loop](const NotificationLaunchId& launch_id) {
+        display_run_loop.Quit();
+      }));
+  bridge->Display(NotificationHandler::Type::WEB_PERSISTENT,
+                  browser()->profile(), notification, /*metadata=*/nullptr);
+  display_run_loop.Run();
+
+  // The notification should now be in the expected map.
+  EXPECT_EQ(1u, bridge->GetExpectedDisplayedNotificationForTesting().size());
+
+  // Close the notification
+  base::RunLoop close_run_loop;
+  EXPECT_TRUE(base::StatisticsRecorder::SetCallback(
+      "Notifications.Windows.CloseStatus",
+      base::BindLambdaForTesting(
+          [&close_run_loop](const char* histogram_name, uint64_t name_hash,
+                            base::HistogramBase::Sample sample) {
+            close_run_loop.Quit();
+          })));
+  bridge->Close(browser()->profile(), notification.id());
+  close_run_loop.Run();
+
+  // Closing a notification should remove it from the expected map.
+  EXPECT_EQ(0u, bridge->GetExpectedDisplayedNotificationForTesting().size());
+
+  base::StatisticsRecorder::ClearCallback("Notifications.Windows.CloseStatus");
+  bridge->SetNotifierForTesting(nullptr);
+}
+
 // Test calling Display with a fake implementation of the Action Center
 // and validate it gets the values expected.
 IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest, DisplayWithFakeAC) {