Reland: Refactor ExtensionManifestTest to allow usage in src/extensions

Original CL broke the Win official bots because they report their default
version channel as "canary" instead of "trunk".

https://codereview.chromium.org/572813002

Original description:

This refactor will allow some manifest tests running in Chrome's unit_tests
suite to move to the extensions_unittests suite.

* Rename ExtensionManifestTest to extensions::ManifestTest and move it into
  src/extensions.
* Introduce ChromeManifestTest to load manifests from Chrome's test data dir.
* Eliminate some unnecessary use of chrome::VersionInfo::Channel.
* Move SharedModuleManifestTest to extensions_unittests as an example.

FileHandlerManifestTest and ExternallyConnectableManifestTest will be next,
but require more refactoring (e.g. of permissions).

BUG=397165
TEST=unit_tests, extensions_unittests
TBR=rockot@chromium.org

Review URL: https://codereview.chromium.org/574173002

Cr-Commit-Position: refs/heads/master@{#295225}

24 files changed, 606 insertions, 46 deletions

diff --git a/extensions/BUILD.gn b/extensions/BUILD.gn
index 52d5749..0b46037 100644
--- a/extensions/BUILD.gn
+++ b/extensions/BUILD.gn
@@ -75,6 +75,8 @@ source_set("test_support") {
     "browser/test_runtime_api_delegate.h",
     "common/extension_builder.cc",
     "common/extension_builder.h",
+    "common/manifest_test.cc",
+    "common/manifest_test.h",
     "common/test_util.cc",
     "common/test_util.h",
     "common/value_builder.cc",
@@ -206,6 +208,7 @@ test("unittests") {
     "common/extension_resource_unittest.cc",
     "common/extension_set_unittest.cc",
     "common/manifest_handler_unittest.cc",
+    "common/manifest_handlers/shared_module_manifest_unittest.cc",
     "common/message_bundle_unittest.cc",
     "common/one_shot_event_unittest.cc",
     "common/permissions/manifest_permission_set_unittest.cc",
diff --git a/extensions/DEPS b/extensions/DEPS
index 836d930..7cfd017 100644
--- a/extensions/DEPS
+++ b/extensions/DEPS
@@ -39,7 +39,7 @@ specific_include_rules = {
     "+chrome/browser/ui/browser.h",
     "+chrome/common/chrome_paths.h",
     "+chrome/common/extensions/features/feature_channel.h",
-    "+chrome/common/extensions/manifest_tests/extension_manifest_test.h",
+    "+chrome/common/extensions/manifest_tests/chrome_manifest_test.h",
     "+chrome/test/base/testing_profile.h",
     "+chrome/test/base/ui_test_utils.h",
   ],
diff --git a/extensions/common/manifest_handlers/externally_connectable_unittest.cc b/extensions/common/manifest_handlers/externally_connectable_unittest.cc
index 77acf35..87e29cc 100644
--- a/extensions/common/manifest_handlers/externally_connectable_unittest.cc
+++ b/extensions/common/manifest_handlers/externally_connectable_unittest.cc
@@ -4,8 +4,7 @@
 
 #include <algorithm>
 
-#include "chrome/common/extensions/features/feature_channel.h"
-#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/externally_connectable.h"
@@ -19,9 +18,11 @@ namespace extensions {
 
 namespace errors = externally_connectable_errors;
 
-class ExternallyConnectableTest : public ExtensionManifestTest {
+// TODO(jamescook): Convert from ChromeManifestTest to ManifestTest.
+class ExternallyConnectableTest : public ChromeManifestTest {
  public:
-  ExternallyConnectableTest() : channel_(chrome::VersionInfo::CHANNEL_DEV) {}
+  ExternallyConnectableTest() {}
+  virtual ~ExternallyConnectableTest() {}
 
  protected:
   ExternallyConnectableInfo* GetExternallyConnectableInfo(
@@ -29,9 +30,6 @@ class ExternallyConnectableTest : public ExtensionManifestTest {
     return static_cast<ExternallyConnectableInfo*>(
         extension->GetManifestData(manifest_keys::kExternallyConnectable));
   }
-
- private:
-  ScopedCurrentChannel channel_;
 };
 
 TEST_F(ExternallyConnectableTest, IDsAndMatches) {
diff --git a/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc b/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc
index 678dc6b..f45d922 100644
--- a/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc
+++ b/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
+#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/file_handler_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -11,7 +11,8 @@ namespace extensions {
 
 namespace errors = manifest_errors;
 
-class FileHandlersManifestTest : public ExtensionManifestTest {
+// TODO(jamescook): Convert from ChromeManifestTest to ManifestTest.
+class FileHandlersManifestTest : public ChromeManifestTest {
 };
 
 TEST_F(FileHandlersManifestTest, InvalidFileHandlers) {
diff --git a/extensions/common/manifest_handlers/shared_module_manifest_unittest.cc b/extensions/common/manifest_handlers/shared_module_manifest_unittest.cc
index 9fad618..ef90a1a 100644
--- a/extensions/common/manifest_handlers/shared_module_manifest_unittest.cc
+++ b/extensions/common/manifest_handlers/shared_module_manifest_unittest.cc
@@ -3,9 +3,9 @@
 // found in the LICENSE file.
 
 #include "base/version.h"
-#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_handlers/shared_module_info.h"
+#include "extensions/common/manifest_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -23,11 +23,11 @@ const char* kNoImport = "cccccccccccccccccccccccccccccccc";
 
 namespace extensions {
 
-class SharedModuleManifestTest : public ExtensionManifestTest {
+class SharedModuleManifestTest : public ManifestTest {
 };
 
 TEST_F(SharedModuleManifestTest, ExportsAll) {
-  Manifest manifest("shared_module_export.json");
+  ManifestData manifest("shared_module_export.json");
 
   scoped_refptr<Extension> extension = LoadAndExpectSuccess(manifest);
 
@@ -49,7 +49,7 @@ TEST_F(SharedModuleManifestTest, ExportsAll) {
 }
 
 TEST_F(SharedModuleManifestTest, ExportWhitelistAll) {
-  Manifest manifest("shared_module_export_no_whitelist.json");
+  ManifestData manifest("shared_module_export_no_whitelist.json");
 
   scoped_refptr<Extension> extension = LoadAndExpectSuccess(manifest);
 
@@ -62,7 +62,7 @@ TEST_F(SharedModuleManifestTest, ExportWhitelistAll) {
 }
 
 TEST_F(SharedModuleManifestTest, ExportFoo) {
-  Manifest manifest("shared_module_export_foo.json");
+  ManifestData manifest("shared_module_export_foo.json");
 
   scoped_refptr<Extension> extension = LoadAndExpectSuccess(manifest);
 
@@ -108,7 +108,7 @@ TEST_F(SharedModuleManifestTest, SharedModuleStaticFunctions) {
 }
 
 TEST_F(SharedModuleManifestTest, Import) {
-  Manifest manifest("shared_module_import.json");
+  ManifestData manifest("shared_module_import.json");
 
   scoped_refptr<Extension> extension = LoadAndExpectSuccess(manifest);
 
diff --git a/extensions/common/manifest_test.cc b/extensions/common/manifest_test.cc
new file mode 100644
index 0000000..b1ab115
--- /dev/null
+++ b/extensions/common/manifest_test.cc
@@ -0,0 +1,258 @@
+// 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 "extensions/common/manifest_test.h"
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/json/json_file_value_serializer.h"
+#include "base/path_service.h"
+#include "base/strings/string_util.h"
+#include "base/values.h"
+#include "extensions/common/extension_l10n_util.h"
+#include "extensions/common/extension_paths.h"
+#include "extensions/common/test_util.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace extensions {
+namespace {
+
+// |manifest_path| is an absolute path to a manifest file.
+base::DictionaryValue* LoadManifestFile(const base::FilePath& manifest_path,
+                                        std::string* error) {
+  base::FilePath extension_path = manifest_path.DirName();
+
+  EXPECT_TRUE(base::PathExists(manifest_path)) <<
+      "Couldn't find " << manifest_path.value();
+
+  JSONFileValueSerializer serializer(manifest_path);
+  base::DictionaryValue* manifest =
+      static_cast<base::DictionaryValue*>(serializer.Deserialize(NULL, error));
+
+  // Most unit tests don't need localization, and they'll fail if we try to
+  // localize them, since their manifests don't have a default_locale key.
+  // Only localize manifests that indicate they want to be localized.
+  // Calling LocalizeExtension at this point mirrors file_util::LoadExtension.
+  if (manifest &&
+      manifest_path.value().find(FILE_PATH_LITERAL("localized")) !=
+      std::string::npos)
+    extension_l10n_util::LocalizeExtension(extension_path, manifest, error);
+
+  return manifest;
+}
+
+}  // namespace
+
+ManifestTest::ManifestTest()
+    : enable_apps_(true) {
+}
+
+ManifestTest::~ManifestTest() {
+}
+
+// Helper class that simplifies creating methods that take either a filename
+// to a manifest or the manifest itself.
+ManifestTest::ManifestData::ManifestData(const char* name)
+    : name_(name), manifest_(NULL) {
+}
+
+ManifestTest::ManifestData::ManifestData(base::DictionaryValue* manifest,
+                                          const char* name)
+    : name_(name), manifest_(manifest) {
+  CHECK(manifest_) << "Manifest NULL";
+}
+
+ManifestTest::ManifestData::ManifestData(
+    scoped_ptr<base::DictionaryValue> manifest)
+    : manifest_(manifest.get()), manifest_holder_(manifest.Pass()) {
+  CHECK(manifest_) << "Manifest NULL";
+}
+
+ManifestTest::ManifestData::ManifestData(const ManifestData& m) {
+  NOTREACHED();
+}
+
+ManifestTest::ManifestData::~ManifestData() {
+}
+
+base::DictionaryValue* ManifestTest::ManifestData::GetManifest(
+    base::FilePath test_data_dir, std::string* error) const {
+  if (manifest_)
+    return manifest_;
+
+  base::FilePath manifest_path = test_data_dir.AppendASCII(name_);
+  manifest_ = LoadManifestFile(manifest_path, error);
+  manifest_holder_.reset(manifest_);
+  return manifest_;
+}
+
+base::FilePath ManifestTest::GetTestDataDir() {
+  base::FilePath path;
+  PathService::Get(DIR_TEST_DATA, &path);
+  return path.AppendASCII("manifest_tests");
+}
+
+scoped_ptr<base::DictionaryValue> ManifestTest::LoadManifest(
+    char const* manifest_name, std::string* error) {
+  base::FilePath manifest_path = GetTestDataDir().AppendASCII(manifest_name);
+  return make_scoped_ptr(LoadManifestFile(manifest_path, error));
+}
+
+scoped_refptr<Extension> ManifestTest::LoadExtension(
+    const ManifestData& manifest,
+    std::string* error,
+    extensions::Manifest::Location location,
+    int flags) {
+  base::FilePath test_data_dir = GetTestDataDir();
+  base::DictionaryValue* value = manifest.GetManifest(test_data_dir, error);
+  if (!value)
+    return NULL;
+  return Extension::Create(
+      test_data_dir.DirName(), location, *value, flags, error);
+}
+
+scoped_refptr<Extension> ManifestTest::LoadAndExpectSuccess(
+    const ManifestData& manifest,
+    extensions::Manifest::Location location,
+    int flags) {
+  std::string error;
+  scoped_refptr<Extension> extension =
+      LoadExtension(manifest, &error, location, flags);
+  EXPECT_TRUE(extension.get()) << manifest.name();
+  EXPECT_EQ("", error) << manifest.name();
+  return extension;
+}
+
+scoped_refptr<Extension> ManifestTest::LoadAndExpectSuccess(
+    char const* manifest_name,
+    extensions::Manifest::Location location,
+    int flags) {
+  return LoadAndExpectSuccess(ManifestData(manifest_name), location, flags);
+}
+
+scoped_refptr<Extension> ManifestTest::LoadAndExpectWarning(
+    const ManifestData& manifest,
+    const std::string& expected_warning,
+    extensions::Manifest::Location location,
+    int flags) {
+  std::string error;
+  scoped_refptr<Extension> extension =
+      LoadExtension(manifest, &error, location, flags);
+  EXPECT_TRUE(extension.get()) << manifest.name();
+  EXPECT_EQ("", error) << manifest.name();
+  EXPECT_EQ(1u, extension->install_warnings().size());
+  EXPECT_EQ(expected_warning, extension->install_warnings()[0].message);
+  return extension;
+}
+
+scoped_refptr<Extension> ManifestTest::LoadAndExpectWarning(
+    char const* manifest_name,
+    const std::string& expected_warning,
+    extensions::Manifest::Location location,
+    int flags) {
+  return LoadAndExpectWarning(
+      ManifestData(manifest_name), expected_warning, location, flags);
+}
+
+void ManifestTest::VerifyExpectedError(
+    Extension* extension,
+    const std::string& name,
+    const std::string& error,
+    const std::string& expected_error) {
+  EXPECT_FALSE(extension) <<
+      "Expected failure loading extension '" << name <<
+      "', but didn't get one.";
+  EXPECT_TRUE(MatchPattern(error, expected_error)) << name <<
+      " expected '" << expected_error << "' but got '" << error << "'";
+}
+
+void ManifestTest::LoadAndExpectError(
+    const ManifestData& manifest,
+    const std::string& expected_error,
+    extensions::Manifest::Location location,
+    int flags) {
+  std::string error;
+  scoped_refptr<Extension> extension(
+      LoadExtension(manifest, &error, location, flags));
+  VerifyExpectedError(extension.get(), manifest.name(), error,
+                      expected_error);
+}
+
+void ManifestTest::LoadAndExpectError(
+    char const* manifest_name,
+    const std::string& expected_error,
+    extensions::Manifest::Location location,
+    int flags) {
+  return LoadAndExpectError(
+      ManifestData(manifest_name), expected_error, location, flags);
+}
+
+void ManifestTest::AddPattern(extensions::URLPatternSet* extent,
+                                       const std::string& pattern) {
+  int schemes = URLPattern::SCHEME_ALL;
+  extent->AddPattern(URLPattern(schemes, pattern));
+}
+
+ManifestTest::Testcase::Testcase(
+    std::string manifest_filename,
+    std::string expected_error,
+    extensions::Manifest::Location location,
+    int flags)
+    : manifest_filename_(manifest_filename),
+      expected_error_(expected_error),
+      location_(location), flags_(flags) {
+}
+
+ManifestTest::Testcase::Testcase(std::string manifest_filename,
+                                          std::string expected_error)
+    : manifest_filename_(manifest_filename),
+      expected_error_(expected_error),
+      location_(extensions::Manifest::INTERNAL),
+      flags_(Extension::NO_FLAGS) {
+}
+
+ManifestTest::Testcase::Testcase(std::string manifest_filename)
+    : manifest_filename_(manifest_filename),
+      location_(extensions::Manifest::INTERNAL),
+      flags_(Extension::NO_FLAGS) {}
+
+ManifestTest::Testcase::Testcase(
+    std::string manifest_filename,
+    extensions::Manifest::Location location,
+    int flags)
+    : manifest_filename_(manifest_filename),
+      location_(location),
+      flags_(flags) {}
+
+void ManifestTest::RunTestcases(const Testcase* testcases,
+                                         size_t num_testcases,
+                                         ExpectType type) {
+  for (size_t i = 0; i < num_testcases; ++i)
+    RunTestcase(testcases[i], type);
+}
+
+void ManifestTest::RunTestcase(const Testcase& testcase,
+                                        ExpectType type) {
+  switch (type) {
+    case EXPECT_TYPE_ERROR:
+      LoadAndExpectError(testcase.manifest_filename_.c_str(),
+                         testcase.expected_error_,
+                         testcase.location_,
+                         testcase.flags_);
+      break;
+    case EXPECT_TYPE_WARNING:
+      LoadAndExpectWarning(testcase.manifest_filename_.c_str(),
+                           testcase.expected_error_,
+                           testcase.location_,
+                           testcase.flags_);
+      break;
+    case EXPECT_TYPE_SUCCESS:
+      LoadAndExpectSuccess(testcase.manifest_filename_.c_str(),
+                           testcase.location_,
+                           testcase.flags_);
+      break;
+   }
+}
+
+}  // namespace extensions
diff --git a/extensions/common/manifest_test.h b/extensions/common/manifest_test.h
new file mode 100644
index 0000000..d331a3b
--- /dev/null
+++ b/extensions/common/manifest_test.h
@@ -0,0 +1,160 @@
+// 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.
+
+#ifndef EXTENSIONS_COMMON_MANIFEST_TEST_H_
+#define EXTENSIONS_COMMON_MANIFEST_TEST_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/manifest.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace base {
+class FilePath;
+}
+
+namespace extensions {
+
+// Base class for tests that parse a manifest file.
+class ManifestTest : public testing::Test {
+ public:
+  ManifestTest();
+  virtual ~ManifestTest();
+
+ protected:
+  // Helper class that simplifies creating methods that take either a filename
+  // to a manifest or the manifest itself.
+  class ManifestData {
+   public:
+    explicit ManifestData(const char* name);
+    ManifestData(base::DictionaryValue* manifest, const char* name);
+    explicit ManifestData(scoped_ptr<base::DictionaryValue> manifest);
+    // C++98 requires the copy constructor for a type to be visible if you
+    // take a const-ref of a temporary for that type.  Since Manifest
+    // contains a scoped_ptr, its implicit copy constructor is declared
+    // Manifest(Manifest&) according to spec 12.8.5.  This breaks the first
+    // requirement and thus you cannot use it with LoadAndExpectError() or
+    // LoadAndExpectSuccess() easily.
+    //
+    // To get around this spec pedantry, we declare the copy constructor
+    // explicitly.  It will never get invoked.
+    ManifestData(const ManifestData& m);
+
+    ~ManifestData();
+
+    const std::string& name() const { return name_; };
+
+    base::DictionaryValue* GetManifest(base::FilePath manifest_path,
+                                       std::string* error) const;
+
+   private:
+    const std::string name_;
+    mutable base::DictionaryValue* manifest_;
+    mutable scoped_ptr<base::DictionaryValue> manifest_holder_;
+  };
+
+  // Returns the path in which to find test manifest data files, for example
+  // extensions/test/data/manifest_tests.
+  virtual base::FilePath GetTestDataDir();
+
+  scoped_ptr<base::DictionaryValue> LoadManifest(
+      char const* manifest_name,
+      std::string* error);
+
+  scoped_refptr<extensions::Extension> LoadExtension(
+      const ManifestData& manifest,
+      std::string* error,
+      extensions::Manifest::Location location =
+          extensions::Manifest::INTERNAL,
+      int flags = extensions::Extension::NO_FLAGS);
+
+  scoped_refptr<extensions::Extension> LoadAndExpectSuccess(
+      const ManifestData& manifest,
+      extensions::Manifest::Location location =
+          extensions::Manifest::INTERNAL,
+      int flags = extensions::Extension::NO_FLAGS);
+
+  scoped_refptr<extensions::Extension> LoadAndExpectSuccess(
+      char const* manifest_name,
+      extensions::Manifest::Location location =
+          extensions::Manifest::INTERNAL,
+      int flags = extensions::Extension::NO_FLAGS);
+
+  scoped_refptr<extensions::Extension> LoadAndExpectWarning(
+      const ManifestData& manifest,
+      const std::string& expected_error,
+      extensions::Manifest::Location location =
+          extensions::Manifest::INTERNAL,
+      int flags = extensions::Extension::NO_FLAGS);
+
+  scoped_refptr<extensions::Extension> LoadAndExpectWarning(
+      char const* manifest_name,
+      const std::string& expected_error,
+      extensions::Manifest::Location location =
+          extensions::Manifest::INTERNAL,
+      int flags = extensions::Extension::NO_FLAGS);
+
+  void VerifyExpectedError(extensions::Extension* extension,
+                           const std::string& name,
+                           const std::string& error,
+                           const std::string& expected_error);
+
+  void LoadAndExpectError(char const* manifest_name,
+                          const std::string& expected_error,
+                          extensions::Manifest::Location location =
+                              extensions::Manifest::INTERNAL,
+                          int flags = extensions::Extension::NO_FLAGS);
+
+  void LoadAndExpectError(const ManifestData& manifest,
+                          const std::string& expected_error,
+                          extensions::Manifest::Location location =
+                              extensions::Manifest::INTERNAL,
+                          int flags = extensions::Extension::NO_FLAGS);
+
+  void AddPattern(extensions::URLPatternSet* extent,
+                  const std::string& pattern);
+
+  // used to differentiate between calls to LoadAndExpectError,
+  // LoadAndExpectWarning and LoadAndExpectSuccess via function RunTestcases.
+  enum ExpectType {
+    EXPECT_TYPE_ERROR,
+    EXPECT_TYPE_WARNING,
+    EXPECT_TYPE_SUCCESS
+  };
+
+  struct Testcase {
+    std::string manifest_filename_;
+    std::string expected_error_; // only used for ExpectedError tests
+    extensions::Manifest::Location location_;
+    int flags_;
+
+    Testcase(std::string manifest_filename, std::string expected_error,
+        extensions::Manifest::Location location, int flags);
+
+    Testcase(std::string manifest_filename, std::string expected_error);
+
+    explicit Testcase(std::string manifest_filename);
+
+    Testcase(std::string manifest_filename,
+             extensions::Manifest::Location location,
+             int flags);
+  };
+
+  void RunTestcases(const Testcase* testcases,
+                    size_t num_testcases,
+                    ExpectType type);
+
+  void RunTestcase(const Testcase& testcase, ExpectType type);
+
+  bool enable_apps_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ManifestTest);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_COMMON_MANIFEST_TEST_H_
diff --git a/extensions/common/test_util.cc b/extensions/common/test_util.cc
index 03b35b8..4eacfa6 100644
--- a/extensions/common/test_util.cc
+++ b/extensions/common/test_util.cc
@@ -4,12 +4,9 @@
 
 #include "extensions/common/test_util.h"
 
-#include "base/json/json_reader.h"
-#include "base/values.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "extensions/common/value_builder.h"
-#include "testing/gtest/include/gtest/gtest.h"
 
 namespace extensions {
 namespace test_util {
@@ -39,21 +36,5 @@ scoped_refptr<Extension> CreateExtensionWithID(const std::string& id) {
       .Build();
 }
 
-scoped_ptr<base::DictionaryValue> ParseJsonDictionaryWithSingleQuotes(
-    std::string json) {
-  std::replace(json.begin(), json.end(), '\'', '"');
-  std::string error_msg;
-  scoped_ptr<base::Value> result(base::JSONReader::ReadAndReturnError(
-      json, base::JSON_ALLOW_TRAILING_COMMAS, NULL, &error_msg));
-  scoped_ptr<base::DictionaryValue> result_dict;
-  if (result && result->IsType(base::Value::TYPE_DICTIONARY)) {
-    result_dict.reset(static_cast<base::DictionaryValue*>(result.release()));
-  } else {
-    ADD_FAILURE() << "Failed to parse \"" << json << "\": " << error_msg;
-    result_dict.reset(new base::DictionaryValue());
-  }
-  return result_dict.Pass();
-}
-
 }  // namespace test_util
 }  // namespace extensions
diff --git a/extensions/common/test_util.h b/extensions/common/test_util.h
index 46ac5d08..862c469 100644
--- a/extensions/common/test_util.h
+++ b/extensions/common/test_util.h
@@ -8,11 +8,6 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
-
-namespace base {
-class DictionaryValue;
-}  // namespace base
 
 namespace extensions {
 class Extension;
@@ -30,12 +25,6 @@ scoped_refptr<Extension> CreateEmptyExtension();
 // Return a very simple extension with a given |id|.
 scoped_refptr<Extension> CreateExtensionWithID(const std::string& id);
 
-// Parses |json| allowing trailing commas and replacing single quotes with
-// double quotes for test readability. If the json fails to parse, calls gtest's
-// ADD_FAILURE and returns an empty dictionary.
-scoped_ptr<base::DictionaryValue> ParseJsonDictionaryWithSingleQuotes(
-    std::string json);
-
 }  // namespace test_util
 }  // namespace extensions
 
diff --git a/extensions/extensions.gyp b/extensions/extensions.gyp
index 364c2ba..c5a1e0c 100644
--- a/extensions/extensions.gyp
+++ b/extensions/extensions.gyp
@@ -934,6 +934,8 @@
         'browser/test_runtime_api_delegate.h',
         'common/extension_builder.cc',
         'common/extension_builder.h',
+        'common/manifest_test.cc',
+        'common/manifest_test.h',
         'common/test_util.cc',
         'common/test_util.h',
         'common/value_builder.cc',
@@ -1094,6 +1096,7 @@
         'common/features/complex_feature_unittest.cc',
         'common/features/simple_feature_unittest.cc',
         'common/manifest_handler_unittest.cc',
+        'common/manifest_handlers/shared_module_manifest_unittest.cc',
         'common/message_bundle_unittest.cc',
         'common/one_shot_event_unittest.cc',
         'common/permissions/api_permission_set_unittest.cc',
diff --git a/extensions/test/data/manifest_tests/shared_module_export.json b/extensions/test/data/manifest_tests/shared_module_export.json
new file mode 100644
index 0000000..49c0c11
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export.json
@@ -0,0 +1,15 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*"
+    ],
+    "whitelist": [
+      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+      "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+    ]
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_and_import.json b/extensions/test/data/manifest_tests/shared_module_export_and_import.json
new file mode 100644
index 0000000..3f59ac2
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_and_import.json
@@ -0,0 +1,14 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*"
+    ]
+  },
+  "import": [{
+    "id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+  }],
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_foo.json b/extensions/test/data/manifest_tests/shared_module_export_foo.json
new file mode 100644
index 0000000..e0b583f
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_foo.json
@@ -0,0 +1,11 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "foo"
+    ]
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_no_whitelist.json b/extensions/test/data/manifest_tests/shared_module_export_no_whitelist.json
new file mode 100644
index 0000000..aafb5f8
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_no_whitelist.json
@@ -0,0 +1,11 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*"
+    ]
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_not_dict.json b/extensions/test/data/manifest_tests/shared_module_export_not_dict.json
new file mode 100644
index 0000000..2500644
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_not_dict.json
@@ -0,0 +1,9 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": [
+    {"resources": "*"}
+  ],
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_resource_not_string.json b/extensions/test/data/manifest_tests/shared_module_export_resource_not_string.json
new file mode 100644
index 0000000..2021553
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_resource_not_string.json
@@ -0,0 +1,12 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*",
+      {}
+    ]
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_resources_not_list.json b/extensions/test/data/manifest_tests/shared_module_export_resources_not_list.json
new file mode 100644
index 0000000..76971072
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_resources_not_list.json
@@ -0,0 +1,9 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": "*"
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_whitelist_item_not_id.json b/extensions/test/data/manifest_tests/shared_module_export_whitelist_item_not_id.json
new file mode 100644
index 0000000..6c1bb1b
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_whitelist_item_not_id.json
@@ -0,0 +1,14 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*"
+    ],
+    "whitelist": [
+      "notavalidid"
+    ]
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_whitelist_item_not_string.json b/extensions/test/data/manifest_tests/shared_module_export_whitelist_item_not_string.json
new file mode 100644
index 0000000..2e75502
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_whitelist_item_not_string.json
@@ -0,0 +1,17 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*"
+    ],
+    "whitelist": [
+      {
+       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": "",
+       "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb": ""
+      }
+    ]
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_export_whitelist_not_list.json b/extensions/test/data/manifest_tests/shared_module_export_whitelist_not_list.json
new file mode 100644
index 0000000..06d3e18
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_export_whitelist_not_list.json
@@ -0,0 +1,12 @@
+{
+  "name": "shared_module_export unittest",
+  "manifest_version": 2,
+  "version": "1.0",
+  "export": {
+    "resources": [
+      "*"
+    ],
+    "whitelist": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+  },
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCq2ADgfXh2Yh6uLgpBrls/gtQEp5gLWZedwRV1Cyy4R1giRnGeYwZ8pgmtpkjdSfoNIrx/WfiXDF+K2TF2I9V+LM60FqqoY6uJNmXjByzKFjNtFETXthA+v/zv6uX1NJ3m+GN9tdDR53AC7ws28Zi0S4+n/a++uzDY+aDubrp+PwIDAQAB"
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_import.json b/extensions/test/data/manifest_tests/shared_module_import.json
new file mode 100644
index 0000000..ca9e170
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_import.json
@@ -0,0 +1,14 @@
+{
+  "name": "Import unit test",
+  "manifest_version": 2,
+  "version": "1.0",
+  "import": [
+    {
+      "id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+    },
+    {
+      "id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+      "minimum_version": "1.0.0.0"
+    }
+  ]
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_import_invalid_id.json b/extensions/test/data/manifest_tests/shared_module_import_invalid_id.json
new file mode 100644
index 0000000..aadd9ae
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_import_invalid_id.json
@@ -0,0 +1,10 @@
+{
+  "name": "Import unit test",
+  "manifest_version": 2,
+  "version": "1.0",
+  "import": [
+    {
+      "id": "xyz"
+    }
+  ]
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_import_invalid_version.json b/extensions/test/data/manifest_tests/shared_module_import_invalid_version.json
new file mode 100644
index 0000000..77a921c
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_import_invalid_version.json
@@ -0,0 +1,11 @@
+{
+  "name": "Import unit test",
+  "manifest_version": 2,
+  "version": "1.0",
+  "import": [
+    {
+      "id": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+      "minimum_version": "aaa"
+    }
+  ]
+}
diff --git a/extensions/test/data/manifest_tests/shared_module_import_not_list.json b/extensions/test/data/manifest_tests/shared_module_import_not_list.json
new file mode 100644
index 0000000..a989bd9
--- /dev/null
+++ b/extensions/test/data/manifest_tests/shared_module_import_not_list.json
@@ -0,0 +1,8 @@
+{
+  "name": "Import unit test",
+  "manifest_version": 2,
+  "version": "1.0",
+  "import": {
+    "id": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+  }
+}