Handle push resubscribes with no sender info (avoids DCHECK)

chrome/browser/push_messaging/push_messaging_browsertest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
 #include <string>
 
@@ skipping 169 matching lines @@
 
     InProcessBrowserTest::TearDown();
   }
 
   void LoadTestPage(const std::string& path) {
     ui_test_utils::NavigateToURL(GetBrowser(), https_server_->GetURL(path));
   }
 
   void LoadTestPage() { LoadTestPage(GetTestURL()); }
 
+  void LoadTestPageWithoutManifest() { LoadTestPage(GetNoManifestTestURL()); }
+
   bool RunScript(const std::string& script, std::string* result) {
     return RunScript(script, result, nullptr);
   }
 
   bool RunScript(const std::string& script, std::string* result,
                  content::WebContents* web_contents) {
     if (!web_contents)
       web_contents = GetBrowser()->tab_strip_model()->GetActiveWebContents();
     return content::ExecuteScriptAndExtractString(web_contents->GetMainFrame(),
                                                   script, result);
@@ skipping 64 matching lines @@
 
   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"; }
 
+  virtual std::string GetNoManifestTestURL() {
+    return "/push_messaging/test_no_manifest.html";
+  }
+
   virtual Browser* GetBrowser() const { return browser(); }
 
   gcm::FakeGCMProfileService* gcm_service_;
   instance_id::FakeGCMDriverForInstanceID* gcm_driver_;
   base::HistogramTester histogram_tester_;
 
  private:
   std::unique_ptr<net::EmbeddedTestServer> https_server_;
   PushMessagingServiceImpl* push_service_;
 
@@ skipping 233 matching lines @@
 
   ASSERT_TRUE(RunScript("isControlled()", &script_result));
   ASSERT_EQ("true - is controlled", script_result);
 
   // Try to subscribe from a worker without a key. This should fail.
   ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
   EXPECT_EQ(
       "AbortError - Registration failed - missing applicationServerKey, and "
       "gcm_sender_id not found in manifest",
       script_result);
-  // Now run the subscribe from the service worker with a key. This
-  // should succeed, and write the key to the datastore.
+
+  // Now run the subscribe with a key. This should succeed.
   ASSERT_TRUE(RunScript("workerSubscribePush()", &script_result));
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, true /* standard_protocol */));
+
+  ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
+  EXPECT_EQ("unsubscribe result: true", script_result);
+  EXPECT_NE(push_service(), GetAppHandler());
+}
+
+IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
+                       ResubscribeWithoutKeyAfterSubscribingWithKeyInManifest) {
+  std::string script_result;
+
+  ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
+  ASSERT_EQ("ok - service worker registered", script_result);
+
+  ASSERT_NO_FATAL_FAILURE(RequestAndAcceptPermission());
+
+  LoadTestPage();  // Reload to become controlled.
+
+  ASSERT_TRUE(RunScript("isControlled()", &script_result));
+  ASSERT_EQ("true - is controlled", script_result);
+
+  // Run the subscription from the document without a key, this will trigger
+  // the code to read sender id from the manifest and will write it to the
+  // datastore.
+  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
   std::string token1;
   ASSERT_NO_FATAL_FAILURE(
-      EndpointToToken(script_result, true /* standard_protocol */, &token1));
+      EndpointToToken(script_result, false /* standard_protocol */, &token1));
+
+  ASSERT_TRUE(RunScript("removeManifest()", &script_result));
+  ASSERT_EQ("manifest removed", script_result);
+
+  // Try to resubscribe from the document without a key or manifest.
+  // This should fail.
+  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - missing applicationServerKey, "
+      "and manifest empty or missing",
+      script_result);
+
+  // Now run the subscribe from the service worker without a key.
+  // In this case, the sender id should be read from the datastore.
+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  std::string token2;
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, false /* standard_protocol */, &token2));
+  EXPECT_EQ(token1, token2);
 
   ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
   EXPECT_EQ("unsubscribe result: true", script_result);
   EXPECT_NE(push_service(), GetAppHandler());
 
-  // Now run the subscribe from the service worker without a key.
-  // In this case, the key will be read from the datastore.
+  // After unsubscribing, subscribe again from the worker with no key.
+  // The sender id should again be read from the datastore, so the
+  // subcribe should succeed, and we should get a new subscription token.
   ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
-  std::string token2;
   ASSERT_NO_FATAL_FAILURE(
-      EndpointToToken(script_result, true /* standard_protocol */, &token2));
+      EndpointToToken(script_result, false /* standard_protocol */, &token2));

[johnme, 2016/11/07 18:11:59]

Nit: seems more consistent to call this token3 (ditto several times below), even
though technically we know that token1==token2.

[awdf, 2016/11/07 19:13:23]

Done.

   EXPECT_NE(token1, token2);
 
   ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
   EXPECT_EQ("unsubscribe result: true", script_result);
   EXPECT_NE(push_service(), GetAppHandler());
 }
 
-IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
-                       SubscribeWorkerUsingManifest) {
+IN_PROC_BROWSER_TEST_F(
+    PushMessagingBrowserTest,
+    ResubscribeFailureWithoutKeyAfterSubscribingFromDocumentWithP256Key) {
   std::string script_result;
 
   ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
   ASSERT_EQ("ok - service worker registered", script_result);
 
   ASSERT_NO_FATAL_FAILURE(RequestAndAcceptPermission());
 
-  LoadTestPage();  // Reload to become controlled.
+  LoadTestPageWithoutManifest();  // Reload to become controlled.
 
   ASSERT_TRUE(RunScript("isControlled()", &script_result));
   ASSERT_EQ("true - is controlled", script_result);
 
-  // Try to subscribe from a worker without a key. This should fail.
+  // Run the subscription from the document with a key.
+  ASSERT_TRUE(RunScript("documentSubscribePush()", &script_result));
+  ASSERT_NO_FATAL_FAILURE(EndpointToToken(script_result));
+
+  // Try to resubscribe from the document without a key - should fail.
+  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - missing applicationServerKey, "
+      "and manifest empty or missing",
+      script_result);
+
+  // Now try to resubscribe from the service worker without a key.
+  // This should also fail as the original key was not numeric.
+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - missing applicationServerKey, "
+      "and gcm_sender_id not found in manifest",
+      script_result);
+
+  ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
+  EXPECT_EQ("unsubscribe result: true", script_result);
+  EXPECT_NE(push_service(), GetAppHandler());
+}
+
+IN_PROC_BROWSER_TEST_F(
+    PushMessagingBrowserTest,
+    ResubscribeFailureWithoutKeyAfterSubscribingFromWorkerWithP256Key) {
+  std::string script_result;
+
+  ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
+  ASSERT_EQ("ok - service worker registered", script_result);
+
+  ASSERT_NO_FATAL_FAILURE(RequestAndAcceptPermission());
+
+  LoadTestPageWithoutManifest();  // Reload to become controlled.
+
+  ASSERT_TRUE(RunScript("isControlled()", &script_result));
+  ASSERT_EQ("true - is controlled", script_result);
+
+  // Run the subscribe from the service worker with a key.
+  // This should succeed.
+  ASSERT_TRUE(RunScript("workerSubscribePush()", &script_result));
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, true /* standard_protocol */));
+
+  // Try to resubscribe from the document without a key - should fail.
+  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - missing applicationServerKey, "
+      "and manifest empty or missing",
+      script_result);
+
+  // Now try to resubscribe from the service worker without a key.
+  // This should also fail as the original key was not numeric.
   ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
   EXPECT_EQ(
       "AbortError - Registration failed - missing applicationServerKey, and "
       "gcm_sender_id not found in manifest",
       script_result);
+
+  ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
+  EXPECT_EQ("unsubscribe result: true", script_result);
   EXPECT_NE(push_service(), GetAppHandler());
+}
 
-  // Run the subscription from the document without a key, this will trigger
-  // the code to read sender id from the manifest and will write it to the
-  // datastore.
-  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
+IN_PROC_BROWSER_TEST_F(
+    PushMessagingBrowserTest,
+    ResubscribeFailureWithoutKeyAfterSubscribingFromDocumentWithNumber) {
+  std::string script_result;
+
+  ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
+  ASSERT_EQ("ok - service worker registered", script_result);
+
+  ASSERT_NO_FATAL_FAILURE(RequestAndAcceptPermission());
+
+  LoadTestPageWithoutManifest();  // Reload to become controlled.
+
+  ASSERT_TRUE(RunScript("isControlled()", &script_result));
+  ASSERT_EQ("true - is controlled", script_result);
+
+  // Run the subscribe from the service worker with a key.

[johnme, 2016/11/07 18:11:59]

s/key/numeric key/ or similar

[awdf, 2016/11/07 19:13:22]

Done. It should also say document not service worker - fixed.

+  // This should succeed.
+  ASSERT_TRUE(
+      RunScript("documentSubscribePushWithNumericKey()", &script_result));
   std::string token1;
   ASSERT_NO_FATAL_FAILURE(
       EndpointToToken(script_result, false /* standard_protocol */, &token1));
 
+  // Try to resubscribe from the document without a key - should fail.
+  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - missing applicationServerKey, "
+      "and manifest empty or missing",
+      script_result);
+
+  // Now run the subscribe from the service worker without a key.
+  // In this case, the sender id should be read from the datastore.
+  // Note, we would rather this failed as we only really want to support
+  // no-key subscribes after subscribing with a numeric gcm sender id in the
+  // manifest, not a numeric applicationServerKey, but for code simplicity
+  // this case is allowed.
+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  std::string token2;
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, false /* standard_protocol */, &token2));
+  EXPECT_EQ(token1, token2);
+
   ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
   EXPECT_EQ("unsubscribe result: true", script_result);
   EXPECT_NE(push_service(), GetAppHandler());
 
-  // Now run the subscribe from the service worker without a key.
-  // In this case, the sender id will be read from the datastore.
+  // After unsubscribing, subscribe again from the worker with no key.
+  // The sender id should again be read from the datastore, so the
+  // subcribe should succeed, and we should get a new subscription token.
   ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
-  std::string token2;
   ASSERT_NO_FATAL_FAILURE(
       EndpointToToken(script_result, false /* standard_protocol */, &token2));
   EXPECT_NE(token1, token2);
 
   ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
+  EXPECT_EQ("unsubscribe result: true", script_result);
+  EXPECT_NE(push_service(), GetAppHandler());
+}
+
+IN_PROC_BROWSER_TEST_F(
+    PushMessagingBrowserTest,
+    ResubscribeFailureWithoutKeyAfterSubscribingFromWorkerWithNumber) {
+  std::string script_result;
+
+  ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
+  ASSERT_EQ("ok - service worker registered", script_result);
+
+  ASSERT_NO_FATAL_FAILURE(RequestAndAcceptPermission());
+
+  LoadTestPageWithoutManifest();  // Reload to become controlled.
+
+  ASSERT_TRUE(RunScript("isControlled()", &script_result));
+  ASSERT_EQ("true - is controlled", script_result);
+
+  // Run the subscribe from the service worker with a key.

[johnme, 2016/11/07 18:11:59]

s/key/numeric key/ or similar

[awdf, 2016/11/07 19:13:22]

Done.

+  // This should succeed.
+  ASSERT_TRUE(RunScript("workerSubscribePushWithNumericKey()", &script_result));
+  std::string token1;
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, false /* standard_protocol */, &token1));
+
+  // Try to resubscribe from the document without a key - should fail.
+  ASSERT_TRUE(RunScript("documentSubscribePushWithoutKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - missing applicationServerKey, "
+      "and manifest empty or missing",
+      script_result);
+
+  // Now run the subscribe from the service worker without a key.
+  // In this case, the sender id should be read from the datastore.
+  // Note, we would rather this failed as we only really want to support
+  // no-key subscribes after subscribing with a numeric gcm sender id in the
+  // manifest, not a numeric applicationServerKey, but for code simplicity
+  // this case is allowed.
+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  std::string token2;
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, false /* standard_protocol */, &token2));
+  EXPECT_EQ(token1, token2);
+
+  ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
+  EXPECT_EQ("unsubscribe result: true", script_result);
+  EXPECT_NE(push_service(), GetAppHandler());
+
+  // After unsubscribing, subscribe again from the worker with no key.
+  // The sender id should again be read from the datastore, so the
+  // subcribe should succeed, and we should get a new subscription token.
+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  ASSERT_NO_FATAL_FAILURE(
+      EndpointToToken(script_result, false /* standard_protocol */, &token2));
+  EXPECT_NE(token1, token2);
+
+  ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
   EXPECT_EQ("unsubscribe result: true", script_result);
   EXPECT_NE(push_service(), GetAppHandler());
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, SubscribePersisted) {
   std::string script_result;
 
   // First, test that Service Worker registration IDs are assigned in order of
   // registering the Service Workers, and the (fake) push subscription ids are
   // assigned in order of push subscription (even when these orders are
@@ skipping 1254 matching lines @@
   ASSERT_NO_FATAL_FAILURE(SubscribeSuccessfully());
   ASSERT_FALSE(background_mode_manager->IsBackgroundModeActive());
 
   // After dropping the last subscription background mode is still inactive.
   std::string script_result;
   ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
   EXPECT_EQ("unsubscribe result: true", script_result);
   ASSERT_FALSE(background_mode_manager->IsBackgroundModeActive());
 }
 #endif  // BUILDFLAG(ENABLE_BACKGROUND) && !defined(OS_CHROMEOS)