Parse manifest file with app.service_worker.script.

chrome/common/extensions/manifest_tests/extension_manifests_service_worker_unittest.cc

Index: chrome/common/extensions/manifest_tests/extension_manifests_service_worker_unittest.cc
diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_service_worker_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_service_worker_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..e1c6c3f2a8c5a5485bfccc530b8c37806c7422c9
--- /dev/null
+++ b/chrome/common/extensions/manifest_tests/extension_manifests_service_worker_unittest.cc
@@ -0,0 +1,127 @@
+// Copyright (c) 2012 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 "base/command_line.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/test/values_test_util.h"
+#include "base/values.h"
+#include "chrome/common/extensions/features/feature_channel.h"
+#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "content/public/common/content_switches.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/error_utils.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/manifest_constants.h"
+#include "extensions/common/manifest_handlers/background_info.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+class ExtensionManifestServiceWorkerTest : public ExtensionManifestTest {};
+
+// Checks that a service_worker key is ignored without enable-service-worker
+// switch.
+TEST_F(ExtensionManifestServiceWorkerTest, ServiceWorkerCommandLineRequired) {
+  const char* manifest_string =
+      "{"
+      "  'name': '',"
+      "  'manifest_version': 2,"
+      "  'version': '1',"
+      "  'app': {"
+      "    'service_worker': {"
+      "      'script': 'service_worker.js'"
+      "    }"
+      "  }"
+      "}";
+  LoadAndExpectError(Manifest(base::test::ParseJsonDictionaryWithSingleQuotes(
+                                  manifest_string).Pass(),
+                              ""),
+                     manifest_errors::kServiceWorkerRequiresFlag);
+}
+
+// Checks that an app manifest with a service_worker key but no script fails.
+TEST_F(ExtensionManifestServiceWorkerTest, ServiceWorkerEmpty) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(

[Jeffrey Yasskin, 2014/03/04 01:41:48]

You have to remove the flag again at the end of the test, or all other unittests
will see it set. (Note what happens if you move ServiceWorkerCommandLineRequired
to be last in this file.) However, CommandLine provides no way to remove a
switch. :-P

We have FeatureSwitch for this purpose in extensions:
https://code.google.com/p/chromium/codesearch/#chromium/src/extensions/common...
We could define a FeatureSwitch for SWs with its default set by the CommandLine
switch, or try promote it to base for Alec to use.

[scheib, 2014/03/04 15:34:31]

Wonderfully this is no longer the case, base::test::TestSuite takes care of
this:
https://code.google.com/p/chromium/codesearch#chromium/src/base/test/test_sui...

I've added CHECKS so that others don't have the same concern when reading.

[Jeffrey Yasskin, 2014/03/04 17:29:15]

Oh, great! I will forget that pitfall then. :)

+      switches::kEnableServiceWorker);
+  const char* manifest_string =
+      "{"
+      "  'name': '',"
+      "  'manifest_version': 2,"
+      "  'version': '1',"
+      "  'app': {"
+      "    'service_worker': {}"  // No script specified.
+      "  }"
+      "}";
+  LoadAndExpectError(Manifest(base::test::ParseJsonDictionaryWithSingleQuotes(
+                                  manifest_string).Pass(),
+                              ""),
+                     manifest_errors::kBackgroundRequiredForPlatformApps);
+
+  const char* manifest_string2 =
+      "{"
+      "  'name': '',"
+      "  'manifest_version': 2,"
+      "  'version': '1',"
+      "  'app': {"
+      "    'service_worker': {"
+      "      'script': ''"  // Empty script.
+      "    }"
+      "  }"
+      "}";
+  LoadAndExpectError(Manifest(base::test::ParseJsonDictionaryWithSingleQuotes(
+                                  manifest_string2).Pass(),
+                              ""),
+                     manifest_errors::kBackgroundRequiredForPlatformApps);
+}
+
+// Checks that an app manifest with a single script is loaded.
+TEST_F(ExtensionManifestServiceWorkerTest, ServiceWorkerScript) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableServiceWorker);
+  const char* manifest_string =
+      "{"
+      "  'name': '',"
+      "  'manifest_version': 2,"
+      "  'version': '1',"
+      "  'app': {"
+      "    'service_worker': {"
+      "      'script': 'service_worker.js'"
+      "    }"
+      "  }"
+      "}";
+  scoped_refptr<Extension> extension(LoadAndExpectSuccess(Manifest(
+      base::test::ParseJsonDictionaryWithSingleQuotes(manifest_string).Pass(),
+      "")));
+  ASSERT_TRUE(extension.get());
+  EXPECT_EQ("service_worker.js",
+            BackgroundInfo::GetServiceWorkerScript(extension.get()));
+
+  EXPECT_TRUE(BackgroundInfo::HasServiceWorker(extension.get()));
+}
+
+// Checks that an app manifest with service worker and background script fails.
+TEST_F(ExtensionManifestServiceWorkerTest, ServiceWorkerWithBackgroundScript) {
+  CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kEnableServiceWorker);
+  const char* manifest_string =
+      "{"
+      "  'name': '',"
+      "  'manifest_version': 2,"
+      "  'version': '1',"
+      "  'app': {"
+      "    'service_worker': {"
+      "      'script': 'service_worker.js'"
+      "    },"

[not at google - send to devlin, 2014/03/03 19:56:07]

when do you plan on adding the parsing logic for just 'service_worker' i.e. not
nested inside 'app'?

[scheib, 2014/03/03 20:25:46]

After we have a hello world app working.

https://code.google.com/p/chromium/issues/detail?id=346885

[not at google - send to devlin, 2014/03/03 20:34:22]

Why? It seems like minimal effort to add it right now, and diverging the
behaviour for background scripts is a bit confusing.

[scheib, 2014/03/03 21:38:00]

Discussed offline - it's not lots of work, but it's enough that this patch
doesn't need the delay or added complexity for. Will follow up with
https://code.google.com/p/chromium/issues/detail?id=346885 when it's the right
time to do so (after we have a prototype of app working, or blocked in doing
that).

+      "    'background': {"
+      "      'scripts': [ 'background.js' ]"
+      "    }"
+      "  }"
+      "}";
+  LoadAndExpectError(Manifest(base::test::ParseJsonDictionaryWithSingleQuotes(
+                                  manifest_string).Pass(),
+                              ""),
+                     manifest_errors::kInvalidBackgroundCombination);
+}
+
+}  // namespace extensions