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

Unified Diff: chrome/browser/services/gcm/push_messaging_browsertest.cc

Issue 883743002: Push API: Grace - allow one in ten pushes to show no notification. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@userdata
Patch Set: Fix typo (invesed needed but not shown logic) Created 5 years, 10 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
« no previous file with comments | « no previous file | chrome/browser/services/gcm/push_messaging_service_impl.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/services/gcm/push_messaging_browsertest.cc
diff --git a/chrome/browser/services/gcm/push_messaging_browsertest.cc b/chrome/browser/services/gcm/push_messaging_browsertest.cc
index b244392dc3b3d24e16cb9beb546721195c32e912..42a3221f6ae3aabaead9a909bacc4b692b7b1c6a 100644
--- a/chrome/browser/services/gcm/push_messaging_browsertest.cc
+++ b/chrome/browser/services/gcm/push_messaging_browsertest.cc
@@ -499,39 +499,34 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
browser()->tab_strip_model()->GetActiveWebContents();
// If the site is visible in an active tab, we should not force a notification
- // to be shown.
+ // to be shown. Try it twice, since we allow one mistake per 10 push events.
GCMClient::IncomingMessage message;
- message.data["data"] = "testdata";
- push_service()->OnMessage(app_id.ToString(), message);
- ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result));
- EXPECT_EQ("testdata", script_result);
- ASSERT_EQ(0u, notification_manager()->GetNotificationCount());
+ for (int n = 0; n < 2; n++) {
+ message.data["data"] = "testdata";
+ push_service()->OnMessage(app_id.ToString(), message);
+ ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result));
+ EXPECT_EQ("testdata", script_result);
+ EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
+ }
// Open a blank foreground tab so site is no longer visible.
ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL("about:blank"), NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
- // If the Service Worker push event handler shows a notification, we should
- // not show a forced one.
- message.data["data"] = "shownotification";
+ // 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.data["data"] = "testdata";
push_service()->OnMessage(app_id.ToString(), message);
ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
- EXPECT_EQ("shownotification", script_result);
- ASSERT_EQ(1u, notification_manager()->GetNotificationCount());
- EXPECT_EQ(base::ASCIIToUTF16("push_test_tag"),
- notification_manager()->GetNotificationAt(0).replace_id());
-
- notification_manager()->CancelAll();
- ASSERT_EQ(0u, notification_manager()->GetNotificationCount());
-
- // However if the Service Worker push event handler does not show a
- // notification, we should show a forced one.
+ EXPECT_EQ("testdata", script_result);
+ EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
message.data["data"] = "testdata";
push_service()->OnMessage(app_id.ToString(), message);
ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
EXPECT_EQ("testdata", script_result);
- ASSERT_EQ(1u, notification_manager()->GetNotificationCount());
+ EXPECT_EQ(1u, notification_manager()->GetNotificationCount());
EXPECT_EQ(base::ASCIIToUTF16(kPushMessagingForcedNotificationTag),
notification_manager()->GetNotificationAt(0).replace_id());
@@ -541,7 +536,31 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
push_service()->OnMessage(app_id.ToString(), message);
ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
EXPECT_EQ("shownotification", script_result);
- ASSERT_EQ(2u, notification_manager()->GetNotificationCount());
+ EXPECT_EQ(2u, notification_manager()->GetNotificationCount());
+
+ 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.data["data"] = "shownotification";
+ for (int n = 0; n < 9; n++) {
+ push_service()->OnMessage(app_id.ToString(), message);
+ ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
+ EXPECT_EQ("shownotification", script_result);
+ EXPECT_EQ(1u, notification_manager()->GetNotificationCount());
+ EXPECT_EQ(base::ASCIIToUTF16("push_test_tag"),
+ notification_manager()->GetNotificationAt(0).replace_id());
+ 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.data["data"] = "testdata";
+ push_service()->OnMessage(app_id.ToString(), message);
+ ASSERT_TRUE(RunScript("resultQueue.pop()", &script_result, web_contents));
+ EXPECT_EQ("testdata", script_result);
+ EXPECT_EQ(0u, notification_manager()->GetNotificationCount());
}
#endif
« no previous file with comments | « no previous file | chrome/browser/services/gcm/push_messaging_service_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698