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

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

Issue 2697793004: Push API: Validate storage before returning cached subscriptions (Closed)
Patch Set: Comment out PUSH_GETREGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE Created 3 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
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 024accdb7bb1a361dfd5708353fbac0ca04492bf..8dcc74eca05c071b2f1c956e1bf9fef186e8af0c 100644
--- a/chrome/browser/push_messaging/push_messaging_browsertest.cc
+++ b/chrome/browser/push_messaging/push_messaging_browsertest.cc
@@ -26,6 +26,8 @@
#include "chrome/browser/engagement/site_engagement_service.h"
#include "chrome/browser/gcm/fake_gcm_profile_service.h"
#include "chrome/browser/gcm/gcm_profile_service_factory.h"
+#include "chrome/browser/gcm/instance_id/instance_id_profile_service.h"
+#include "chrome/browser/gcm/instance_id/instance_id_profile_service_factory.h"
#include "chrome/browser/lifetime/keep_alive_registry.h"
#include "chrome/browser/lifetime/keep_alive_types.h"
#include "chrome/browser/notifications/message_center_display_service.h"
@@ -49,6 +51,7 @@
#include "components/gcm_driver/common/gcm_messages.h"
#include "components/gcm_driver/gcm_client.h"
#include "components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h"
+#include "components/gcm_driver/instance_id/instance_id_driver.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/push_subscription_options.h"
@@ -108,6 +111,14 @@ void DidRegister(base::Closure done_callback,
done_callback.Run();
}
+void InstanceIDResultCallback(base::Closure done_callback,
+ instance_id::InstanceID::Result* out_result,
+ instance_id::InstanceID::Result result) {
+ if (out_result)
Peter Beverloo 2017/03/20 23:50:13 if -> DCHECK, it's always given
johnme 2017/03/30 18:36:38 Done.
+ *out_result = result;
+ done_callback.Run();
+}
+
} // namespace
class PushMessagingBrowserTest : public InProcessBrowserTest {
@@ -241,6 +252,11 @@ class PushMessagingBrowserTest : public InProcessBrowserTest {
bool standard_protocol = true,
std::string* out_token = nullptr);
+ // Deletes an Instance ID from the GCM Store but keeps the push subscription
+ // stored in the PushMessagingAppIdentifier map and Service Worker DB.
+ // Calls should be wrapped in the ASSERT_NO_FATAL_FAILURE() macro.
+ void DeleteInstanceIDAsIfGCMStoreReset(const std::string& app_id);
+
PushMessagingAppIdentifier GetAppIdentifierForServiceWorkerRegistration(
int64_t service_worker_registration_id);
@@ -378,8 +394,8 @@ void PushMessagingBrowserTest::LegacySubscribeSuccessfully(
GURL requesting_origin = https_server()->GetURL("/").GetOrigin();
int64_t service_worker_registration_id = 0LL;
PushMessagingAppIdentifier app_identifier =
- PushMessagingAppIdentifier::Generate(requesting_origin,
- service_worker_registration_id);
+ PushMessagingAppIdentifier::LegacyGenerateForTesting(
+ requesting_origin, service_worker_registration_id);
push_service_->IncreasePushSubscriptionCount(1, true /* is_pending */);
std::string subscription_id;
@@ -436,6 +452,31 @@ PushMessagingBrowserTest::GetAppIdentifierForServiceWorkerRegistration(
return app_identifier;
}
+void PushMessagingBrowserTest::DeleteInstanceIDAsIfGCMStoreReset(
+ const std::string& app_id) {
+ // Delete the Instance ID directly, keeping the push subscription stored in
+ // the PushMessagingAppIdentifier map and the Service Worker database. This
+ // simulates the GCM Store getting reset but failing to clear push
+ // subscriptions, either because the store got reset before
+ // 93ec793ac69a542b2213297737178a55d069fd0d (Chrome 56), or because a race
+ // condition (e.g. shutdown) prevents PushMessagingServiceImpl::OnStoreReset
+ // from clearing all subscriptions.
+ instance_id::InstanceIDProfileService* instance_id_profile_service =
+ instance_id::InstanceIDProfileServiceFactory::GetForProfile(
+ GetBrowser()->profile());
+ DCHECK(instance_id_profile_service);
+ instance_id::InstanceIDDriver* instance_id_driver =
+ instance_id_profile_service->driver();
+ DCHECK(instance_id_driver);
+ instance_id::InstanceID::Result delete_result =
+ instance_id::InstanceID::UNKNOWN_ERROR;
+ base::RunLoop run_loop;
+ instance_id_driver->GetInstanceID(app_id)->DeleteID(base::Bind(
+ &InstanceIDResultCallback, run_loop.QuitClosure(), &delete_result));
+ run_loop.Run();
+ ASSERT_EQ(instance_id::InstanceID::SUCCESS, delete_result);
+}
+
void PushMessagingBrowserTest::SendMessageAndWaitUntilHandled(
const PushMessagingAppIdentifier& app_identifier,
const gcm::IncomingMessage& message) {
@@ -1752,6 +1793,93 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
}
IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
+ InvalidGetSubscriptionUnsubscribes) {
+ std::string script_result;
+
+ ASSERT_NO_FATAL_FAILURE(SubscribeSuccessfully());
+
+ GURL origin = https_server()->GetURL("/").GetOrigin();
+ PushMessagingAppIdentifier app_identifier1 =
+ PushMessagingAppIdentifier::FindByServiceWorker(
+ GetBrowser()->profile(), origin,
+ 0LL /* service_worker_registration_id */);
+ ASSERT_FALSE(app_identifier1.is_null());
+
+ ASSERT_NO_FATAL_FAILURE(
+ DeleteInstanceIDAsIfGCMStoreReset(app_identifier1.app_id()));
+
+ // Push messaging should not yet be aware of the InstanceID being deleted.
+ histogram_tester_.ExpectTotalCount("PushMessaging.UnregistrationReason", 0);
+ // We should still be able to look up the app id.
+ PushMessagingAppIdentifier app_identifier2 =
+ PushMessagingAppIdentifier::FindByServiceWorker(
+ GetBrowser()->profile(), origin,
+ 0LL /* service_worker_registration_id */);
+ EXPECT_FALSE(app_identifier2.is_null());
+ EXPECT_EQ(app_identifier1.app_id(), app_identifier2.app_id());
+
+ // Now call PushManager.getSubscription(). It should return null.
+ ASSERT_TRUE(RunScript("hasSubscription()", &script_result));
+ EXPECT_EQ("false - not subscribed", script_result);
+
+ // This should have unsubscribed the push subscription.
+ histogram_tester_.ExpectUniqueSample(
+ "PushMessaging.UnregistrationReason",
+ content::PUSH_UNREGISTRATION_REASON_GET_SUBSCRIPTION_STORAGE_CORRUPT, 1);
+ // We should no longer be able to look up the app id.
+ PushMessagingAppIdentifier app_identifier3 =
+ PushMessagingAppIdentifier::FindByServiceWorker(
+ GetBrowser()->profile(), origin,
+ 0LL /* service_worker_registration_id */);
+ EXPECT_TRUE(app_identifier3.is_null());
+}
+
+IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, InvalidSubscribeUnsubscribes) {
+ std::string script_result;
+
+ std::string token1;
+ ASSERT_NO_FATAL_FAILURE(SubscribeSuccessfully(true /* use_key */, &token1));
+
+ GURL origin = https_server()->GetURL("/").GetOrigin();
+ PushMessagingAppIdentifier app_identifier1 =
+ PushMessagingAppIdentifier::FindByServiceWorker(
+ GetBrowser()->profile(), origin,
+ 0LL /* service_worker_registration_id */);
+ ASSERT_FALSE(app_identifier1.is_null());
+
+ ASSERT_NO_FATAL_FAILURE(
+ DeleteInstanceIDAsIfGCMStoreReset(app_identifier1.app_id()));
+
+ // Push messaging should not yet be aware of the InstanceID being deleted.
+ histogram_tester_.ExpectTotalCount("PushMessaging.UnregistrationReason", 0);
+ // We should still be able to look up the app id.
+ PushMessagingAppIdentifier app_identifier2 =
+ PushMessagingAppIdentifier::FindByServiceWorker(
+ GetBrowser()->profile(), origin,
+ 0LL /* service_worker_registration_id */);
+ EXPECT_FALSE(app_identifier2.is_null());
+ EXPECT_EQ(app_identifier1.app_id(), app_identifier2.app_id());
+
+ // Now call PushManager.subscribe() again. It should succeed, but with a
+ // *different* token, indicating that it unsubscribed and re-subscribed.
+ std::string token2;
+ ASSERT_NO_FATAL_FAILURE(SubscribeSuccessfully(true /* use_key */, &token2));
+ EXPECT_NE(token1, token2);
+
+ // This should have unsubscribed the original push subscription.
+ histogram_tester_.ExpectUniqueSample(
+ "PushMessaging.UnregistrationReason",
+ content::PUSH_UNREGISTRATION_REASON_SUBSCRIBE_STORAGE_CORRUPT, 1);
+ // Looking up the app id should return a different id.
+ PushMessagingAppIdentifier app_identifier3 =
+ PushMessagingAppIdentifier::FindByServiceWorker(
+ GetBrowser()->profile(), origin,
+ 0LL /* service_worker_registration_id */);
+ EXPECT_FALSE(app_identifier3.is_null());
+ EXPECT_NE(app_identifier2.app_id(), app_identifier3.app_id());
+}
+
+IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
GlobalResetPushPermissionUnsubscribes) {
std::string script_result;

Powered by Google App Engine
This is Rietveld 408576698