Add testing for subscription from service workers

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 83 matching lines @@
 #if defined(ENABLE_NOTIFICATIONS)
     notification_manager_.reset(new StubNotificationUIManager);
     notification_service()->SetNotificationUIManagerForTesting(
         notification_manager());
 #endif
 
     InProcessBrowserTest::SetUp();
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
-    // Enable experiemntal features for subscription restrictions.
+    // Enable experimental features for subscription restrictions.
     command_line->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
     InProcessBrowserTest::SetUpCommandLine(command_line);
   }
 
   // InProcessBrowserTest:
   void SetUpOnMainThread() override {
     gcm_service_ = static_cast<gcm::FakeGCMProfileService*>(
         gcm::GCMProfileServiceFactory::GetInstance()->SetTestingFactoryAndUse(
             GetBrowser()->profile(), &gcm::FakeGCMProfileService::Build));
@@ skipping 273 matching lines @@
 
   ASSERT_TRUE(RunScript("swapManifestNoSenderId()", &script_result));
   ASSERT_EQ("sender id removed from manifest", script_result);
 
   ASSERT_TRUE(RunScript("subscribePush()", &script_result));
   EXPECT_EQ(
       "AbortError - Registration failed - gcm_sender_id not found in manifest",
       script_result);
 }
 
-// TODO(johnme): Test subscribing from a worker - see https://crbug.com/437298.
-
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTestEmptySubscriptionOptions,
                        RegisterFailureEmptyPushSubscriptionOptions) {
   std::string script_result;
 
   ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
   ASSERT_EQ("ok - service worker registered", script_result);
 
   RequestAndAcceptPermission();
 
   ASSERT_TRUE(RunScript("subscribePush()", &script_result));
   EXPECT_EQ("PermissionDeniedError - Registration failed - permission denied",
             script_result);
 }
 
+IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, WorkerSubscribeSuccessful) {

[Michael van Ouwerkerk, 2016/03/22 11:31:37]

Not all attempts in this test are successful. Maybe name it something like
SubscribeFromWorker?

[harkness, 2016/03/24 16:56:23]

Done.

+  std::string script_result;
+
+  ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
+  ASSERT_EQ("ok - service worker registered", script_result);
+
+  RequestAndAcceptPermission();
+
+  LoadTestPage();  // 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.
+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  EXPECT_EQ(
+      "AbortError - Registration failed - gcm_sender_id not found in manifest",
+      script_result);
+
+  // Run the subscription from the document without a key, this will
+  // trigger the code to read the manifest.
+  ASSERT_TRUE(RunScript("subscribePushWithoutKey()", &script_result));

[Michael van Ouwerkerk, 2016/03/22 11:31:37]

For clarity it might help to rename subscribePushWithoutKey to
documentSubscribePushWithoutKey (and similar in other places if we have a worker
and a document version of the function).

[harkness, 2016/03/24 16:56:23]

Done.

+  EXPECT_EQ(GetEndpointForSubscriptionId("1-0"), script_result);
+
+  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.

[Michael van Ouwerkerk, 2016/03/22 11:31:37]

I think no application server key has been provided yet at this point, only a
sender_id through the manifest. What is read from the datastore? Let's be more
explicit, and replace "key" with "application server key" in a few places, and
distinguish it from a sender id even though it is stored in the same place.

[harkness, 2016/03/24 16:56:23]

Good point! In fact, this was hiding an actual code issue, which was also missed
in the unit tests. Tests have been expanded appropriately.

+  ASSERT_TRUE(RunScript("workerSubscribePushNoKey()", &script_result));
+  EXPECT_EQ(GetEndpointForSubscriptionId("1-1"), script_result);
+
+  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 with a key.
+  ASSERT_TRUE(RunScript("workerSubscribePush()", &script_result));
+  EXPECT_EQ(GetEndpointForSubscriptionId("1-2"), script_result);
+
+  ASSERT_TRUE(RunScript("unsubscribePush()", &script_result));
+  EXPECT_EQ("unsubscribe result: true", script_result);
+  EXPECT_NE(push_service(), GetAppHandler());
+}
+
 // Disabled on Windows and Linux due to flakiness (http://crbug.com/554003).
 #if defined(OS_WIN) || defined(OS_LINUX)
 #define MAYBE_SubscribePersisted DISABLED_SubscribePersisted
 #else
 #define MAYBE_SubscribePersisted SubscribePersisted
 #endif
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, MAYBE_SubscribePersisted) {
   std::string script_result;
 
   // First, test that Service Worker registration IDs are assigned in order of
@@ skipping 911 matching lines @@
   ASSERT_FALSE(background_mode_manager->IsBackgroundModeActive());
 
   // After dropping the last subscription background mode is still inactive.
   std::string script_result;
   gcm_service()->AddExpectedUnregisterResponse(gcm::GCMClient::SUCCESS);
   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)