Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(40)

Unified Diff: chrome/browser/push_messaging/push_messaging_browsertest.cc

Issue 1887623002: Replace the 1 in 10 grace period with an accumulating budget based on SES. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Using Remove* instead of Set* Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/push_messaging/push_messaging_browsertest.cc
diff --git a/chrome/browser/push_messaging/push_messaging_browsertest.cc b/chrome/browser/push_messaging/push_messaging_browsertest.cc
index eade560989f097adb3b81c5f1c3681fe4b0b6eae..d619fa9939351a04c50d29f966d725e6cb4f1477 100644
--- a/chrome/browser/push_messaging/push_messaging_browsertest.cc
+++ b/chrome/browser/push_messaging/push_messaging_browsertest.cc
@@ -22,6 +22,7 @@
#include "chrome/browser/browsing_data/browsing_data_remover_factory.h"
#include "chrome/browser/browsing_data/browsing_data_remover_test_util.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
#include "chrome/browser/notifications/message_center_display_service.h"
#include "chrome/browser/notifications/notification_test_util.h"
#include "chrome/browser/notifications/platform_notification_service_impl.h"
@@ -239,6 +240,12 @@ class PushMessagingBrowserTest : public InProcessBrowserTest {
PushMessagingServiceImpl* push_service() const { return push_service_; }
+ void SetSiteEngagementScore(const GURL& url, double score) {
+ SiteEngagementService* service =
+ SiteEngagementService::Get(GetBrowser()->profile());
+ service->ResetScoreForURL(url, score);
+ }
+
protected:
virtual std::string GetTestURL() { return "/push_messaging/test.html"; }
@@ -750,14 +757,34 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
GetBrowser(), GURL("about:blank"), NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
- // If the Service Worker push event handler does not show a notification, we
- // should show a forced one, but only on the 2nd occurrence since we allow one
- // mistake per 10 push events.
- message.raw_data = "testdata";
+ // Set the site engagement score for the site. Setting it to 2 means we should
+ // have enough budget for two non-shown notification.
+ SetSiteEngagementScore(web_contents->GetURL(), 2.0);
+
+ // If the Service Worker push event handler shows a notification, we
+ // should not show a forced one.
+ message.raw_data = "shownotification";
SendMessageAndWaitUntilHandled(app_identifier, message);
ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
- EXPECT_EQ("testdata", script_result);
- EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
+ EXPECT_EQ("shownotification", script_result);
+ EXPECT_EQ(1u, notification_manager()->GetNotificationCount());
+ EXPECT_EQ("push_test_tag",
+ notification_manager()->GetNotificationAt(0).tag());
+ notification_manager()->CancelAll();
+
+ // If the Service Worker push event handler does not show a notification, we
+ // should show a forced one, but only once the origin is out of budget.
+ message.raw_data = "testdata";
+ for (int n = 0; n < 2; n++) {
+ // First two missed notifications shouldn't force a default one.
+ SendMessageAndWaitUntilHandled(app_identifier, message);
+ ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
+ EXPECT_EQ("testdata", script_result);
+ EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
+ }
+
+ // Third missed notification should trigger a default notification, since the
+ // origin will be out of budget.
message.raw_data = "testdata";
SendMessageAndWaitUntilHandled(app_identifier, message);
ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
@@ -786,30 +813,6 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
EXPECT_NE(kPushMessagingForcedNotificationTag, first_notification.tag());
}
-
- notification_manager()->CancelAll();
- EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
-
- // However if the Service Worker push event handler shows a notification, we
- // should not show a forced one.
- message.raw_data = "shownotification";
- for (int n = 0; n < 9; n++) {
- SendMessageAndWaitUntilHandled(app_identifier, message);
- ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
- EXPECT_EQ("shownotification", script_result);
- EXPECT_EQ(1u, notification_manager()->GetNotificationCount());
- EXPECT_EQ("push_test_tag",
- notification_manager()->GetNotificationAt(0).tag());
- notification_manager()->CancelAll();
- }
-
- // Now that 10 push messages in a row have shown notifications, we should
- // allow the next one to mistakenly not show a notification.
- message.raw_data = "testdata";
- SendMessageAndWaitUntilHandled(app_identifier, message);
- ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
- EXPECT_EQ("testdata", script_result);
- EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
}
IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,

Powered by Google App Engine
This is Rietveld 408576698