Refactor ExtensionManifestTest to allow usage in src/extensions

extensions/common/manifest_test.h

Index: extensions/common/manifest_test.h
diff --git a/chrome/common/extensions/manifest_tests/extension_manifest_test.h b/extensions/common/manifest_test.h
similarity index 71%
rename from chrome/common/extensions/manifest_tests/extension_manifest_test.h
rename to extensions/common/manifest_test.h
index 6d68cf9d5b7c157b3a03bb4f55c263fa9faa037c..d331a3bff368de2011fafde1ed0b5aafa1357fc1 100644
--- a/chrome/common/extensions/manifest_tests/extension_manifest_test.h
+++ b/extensions/common/manifest_test.h
@@ -2,29 +2,36 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_COMMON_EXTENSIONS_MANIFEST_TESTS_EXTENSION_MANIFEST_TEST_H_
-#define CHROME_COMMON_EXTENSIONS_MANIFEST_TESTS_EXTENSION_MANIFEST_TEST_H_
+#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 "chrome/common/extensions/features/feature_channel.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class ExtensionManifestTest : public testing::Test {
+namespace base {
+class FilePath;
+}
+
+namespace extensions {
+
+// Base class for tests that parse a manifest file.
+class ManifestTest : public testing::Test {
  public:
-  ExtensionManifestTest();
+  ManifestTest();
+  virtual ~ManifestTest();
 
  protected:
   // Helper class that simplifies creating methods that take either a filename
   // to a manifest or the manifest itself.
-  class Manifest {
+  class ManifestData {
    public:
-    explicit Manifest(const char* name);
-    Manifest(base::DictionaryValue* manifest, const char* name);
-    explicit Manifest(scoped_ptr<base::DictionaryValue> manifest);
+    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
@@ -34,13 +41,13 @@ class ExtensionManifestTest : public testing::Test {
     //
     // To get around this spec pedantry, we declare the copy constructor
     // explicitly.  It will never get invoked.
-    Manifest(const Manifest& m);
+    ManifestData(const ManifestData& m);
 
-    ~Manifest();
+    ~ManifestData();
 
     const std::string& name() const { return name_; };
 
-    base::DictionaryValue* GetManifest(char const* test_data_dir,
+    base::DictionaryValue* GetManifest(base::FilePath manifest_path,
                                        std::string* error) const;
 
    private:
@@ -49,22 +56,23 @@ class ExtensionManifestTest : public testing::Test {
     mutable scoped_ptr<base::DictionaryValue> manifest_holder_;
   };
 
-  // The subdirectory in which to find test data files.
-  virtual char const* test_data_dir();
+  // 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 Manifest& manifest,
+      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 Manifest& manifest,
+      const ManifestData& manifest,
       extensions::Manifest::Location location =
           extensions::Manifest::INTERNAL,
       int flags = extensions::Extension::NO_FLAGS);
@@ -75,13 +83,8 @@ class ExtensionManifestTest : public testing::Test {
           extensions::Manifest::INTERNAL,
       int flags = extensions::Extension::NO_FLAGS);
 
-  // Load and expect success from a manifest provided as a json string. Single
-  // quotes will be replaced with double quotes for test readability.
-  scoped_refptr<extensions::Extension> LoadFromStringAndExpectSuccess(
-      char const* manifest_json);

[James Cook, 2014/09/16 00:51:50]

Unused, deleted it and associated dead code.

[Ken Rockot(use gerrit already), 2014/09/16 16:20:20]

Woot! Kill ALL the dead code!

-
   scoped_refptr<extensions::Extension> LoadAndExpectWarning(
-      const Manifest& manifest,
+      const ManifestData& manifest,
       const std::string& expected_error,
       extensions::Manifest::Location location =
           extensions::Manifest::INTERNAL,
@@ -105,17 +108,12 @@ class ExtensionManifestTest : public testing::Test {
                               extensions::Manifest::INTERNAL,
                           int flags = extensions::Extension::NO_FLAGS);
 
-  void LoadAndExpectError(const Manifest& manifest,
+  void LoadAndExpectError(const ManifestData& manifest,
                           const std::string& expected_error,
                           extensions::Manifest::Location location =
                               extensions::Manifest::INTERNAL,
                           int flags = extensions::Extension::NO_FLAGS);
 
-  // Load and expect an error from a manifest provided as a json string. Single
-  // quotes will be replaced with double quotes for test readability.
-  void LoadFromStringAndExpectError(char const* manifest_json,
-                                    const std::string& expected_error);
-
   void AddPattern(extensions::URLPatternSet* extent,
                   const std::string& pattern);
 
@@ -153,13 +151,10 @@ class ExtensionManifestTest : public testing::Test {
 
   bool enable_apps_;
 
-  // Force the manifest tests to run as though they are on trunk, since several
-  // tests rely on manifest features being available that aren't on
-  // stable/beta.
-  //
-  // These objects nest, so if a test wants to explicitly test the behaviour
-  // on stable or beta, declare it inside that test.
-  extensions::ScopedCurrentChannel current_channel_;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ManifestTest);
 };
 
-#endif  // CHROME_COMMON_EXTENSIONS_MANIFEST_TESTS_EXTENSION_MANIFEST_TEST_H_
+}  // namespace extensions
+
+#endif  // EXTENSIONS_COMMON_MANIFEST_TEST_H_