[Push] Register: require user-visible to be present in the manifest

chrome/browser/services/gcm/push_messaging_browsertest.cc

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 6dfd3b1ac50d09073524549acca3712bdec53ab3..1fb79b9a205651369ff1c433d249d3a9ac92655f 100644
--- a/chrome/browser/services/gcm/push_messaging_browsertest.cc
+++ b/chrome/browser/services/gcm/push_messaging_browsertest.cc
@@ -141,7 +141,7 @@ class PushMessagingBrowserTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUpOnMainThread();
   }
 
-  void loadTestPage() {
+  virtual void loadTestPage() {
     ui_test_utils::NavigateToURL(
         browser(), https_server_->GetURL("files/push_messaging/test.html"));
   }
@@ -162,14 +162,34 @@ class PushMessagingBrowserTest : public InProcessBrowserTest {
         gcm_service_->push_messaging_service());
   }
 
- private:
+ protected:

[Michael van Ouwerkerk, 2014/12/10 18:36:33]

This should still be private. We have public getters for the server and service.

[Miguel Garcia, 2014/12/11 11:37:08]

Done.

   scoped_ptr<net::SpawnedTestServer> https_server_;
   FakeGCMProfileService* gcm_service_;
 
   DISALLOW_COPY_AND_ASSIGN(PushMessagingBrowserTest);
 };
 
-IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, RegisterSuccess) {
+class PushMessagingBadManifestBrowserTest : public PushMessagingBrowserTest {
+  void loadTestPage() override {

[Michael van Ouwerkerk, 2014/12/10 18:36:33]

Instead of subclassing, you could add a loadTestPage method that accepts a url
for the page to load. Just call it at the start of your test method to load a
different page. I don't think it's overly expensive to load an extra page, but
if you do, we can talk about other approaches.

Calling loadTestPage with a specific url at the start of the test also seems
easier to read than using a different test class, I tend to gloss over those.

[Miguel Garcia, 2014/12/11 11:37:08]

mm not sure I agree here, the way this is structured we now have a page per test
which seems sensible and removes a lot of boiler plate code (having to load the
page on every test case.


On 2014/12/10 18:36:33, Michael van Ouwerkerk wrote:

+    ui_test_utils::NavigateToURL(
+        browser(),
+        https_server_->GetURL("files/push_messaging/test_bad_manifest.html"));
+  };
+};
+
+IN_PROC_BROWSER_TEST_F(PushMessagingBadManifestBrowserTest,
+                       RegisterFailsNotVisibleMessages) {
+  std::string script_result;
+
+  ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
+  ASSERT_EQ("ok - service worker registered", script_result);
+  ASSERT_TRUE(RunScript("registerPush()", &script_result));
+  EXPECT_EQ("AbortError - Registration failed - permission denied",
+            script_result);
+}
+
+IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
+                       RegisterSuccessNotificationsGranted) {
   std::string script_result;
 
   ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
@@ -188,15 +208,19 @@ IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest, RegisterSuccess) {
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,
-                       RegisterFailureNoPushPermission) {
+                       RegisterSuccessNotificationsPrompt) {
   std::string script_result;
 
   ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
   ASSERT_EQ("ok - service worker registered", script_result);
 
+  InfoBarResponder accepting_responder(browser(), true);
   ASSERT_TRUE(RunScript("registerPush()", &script_result));
-  EXPECT_EQ("AbortError - Registration failed - permission denied",
-            script_result);
+  EXPECT_EQ(std::string(kPushMessagingEndpoint) + " - 1", script_result);
+
+  PushMessagingApplicationId app_id(https_server()->GetURL(""), 0L);
+  EXPECT_EQ(app_id.ToString(), gcm_service()->last_registered_app_id());
+  EXPECT_EQ("1234567890", gcm_service()->last_registered_sender_ids()[0]);
 }
 
 IN_PROC_BROWSER_TEST_F(PushMessagingBrowserTest,