Add testing for subscription from service workers

chrome/browser/push_messaging/push_messaging_browsertest.cc

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 2543d79dd4824a2ae26f2cd7165fd98a6125ee35..75a1ec0fdb1bfcba9abd78e484e33a32b7172c46 100644
--- a/chrome/browser/push_messaging/push_messaging_browsertest.cc
+++ b/chrome/browser/push_messaging/push_messaging_browsertest.cc
@@ -101,7 +101,7 @@ class PushMessagingBrowserTest : public InProcessBrowserTest {
   }
 
   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);
@@ -395,8 +395,6 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, SubscribeFailureNoSenderId) {
       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;
@@ -411,6 +409,52 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTestEmptySubscriptionOptions,
             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