[Extensions] Remove some test-only manifest v1 switches

Remove Extension::ScopedAllowLegacyExtensions and
ExtensionBrowserTest::ShouldAllowLegacyExtensionManifests(). These are
both unused now that all the affected tests have been updated.

Leave in the commandline flag kAllowLegacyExtensionManifests. This can
still be useful as a last resort for loading a v1 extension, and
there's no harm in leaving it in since we've removed all manifest
v1-specific handling (that is, all extensions loaded will effectively
be treated as v2).

Bug: 816679
Change-Id: I673bee01d00eaf7b82e1996da84f9b44e090aaf9
Reviewed-on: https://chromium-review.googlesource.com/1029000Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553860}

chrome/browser/extensions/extension_browsertest.cc

@@ -146,10 +146,6 @@ Profile* ExtensionBrowserTest::profile() {
   return profile_;
 }
 
-bool ExtensionBrowserTest::ShouldAllowLegacyExtensionManifests() {
-  return false;
-}
-
 bool ExtensionBrowserTest::ShouldEnableContentVerification() {
   return false;
 }
@@ -186,12 +182,6 @@ void ExtensionBrowserTest::SetUpCommandLine(base::CommandLine* command_line) {
   ExtensionMessageBubbleFactory::set_override_for_tests(
       ExtensionMessageBubbleFactory::OVERRIDE_DISABLED);
 
-  // TODO(devlin): Remove this. See https://crbug.com/816679.
-  if (ShouldAllowLegacyExtensionManifests()) {
-    command_line->AppendSwitch(
-        extensions::switches::kAllowLegacyExtensionManifests);
-  }
-
   if (!ShouldEnableContentVerification()) {
     ignore_content_verification_.reset(
         new extensions::ScopedIgnoreContentVerifierForTest());

chrome/browser/extensions/extension_browsertest.h

@@ -76,11 +76,6 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest {
   // Get the profile to use.
   virtual Profile* profile();
 
-  // Returns true if extensions with antiquated manifest versions (i.e., version
-  // 1) should be allowed to load in the test. This is only used for migration
-  // while the remainder of these tests are updated; do not add more usages.
-  virtual bool ShouldAllowLegacyExtensionManifests();
-
   // Extensions used in tests are typically not from the web store and will have
   // missing content verification hashes. The default implementation disables
   // content verification; this should be overridden by derived tests which care

extensions/common/extension.cc

@@ -64,8 +64,6 @@ const char kKeyInfoEndMarker[] = "KEY-----";
 const char kPublic[] = "PUBLIC";
 const char kPrivate[] = "PRIVATE";
 
-bool g_allow_legacy_extensions = false;
-
 bool ContainsReservedCharacters(const base::FilePath& path) {
   // We should disallow backslash '\\' as file path separator even on Windows,
   // because the backslash is not regarded as file path separator on Linux/Mac.
@@ -92,7 +90,6 @@ bool IsManifestSupported(int manifest_version,
   // handling has been removed, which means they will effectively be treated as
   // v2s.
   bool allow_legacy_extensions =
-      g_allow_legacy_extensions ||
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kAllowLegacyExtensionManifests);
   if (type == Manifest::TYPE_EXTENSION && allow_legacy_extensions)
@@ -476,14 +473,6 @@ void Extension::AddWebExtentPattern(const URLPattern& pattern) {
   extent_.AddPattern(pattern);
 }
 
-Extension::ScopedAllowLegacyExtensions
-Extension::allow_legacy_extensions_for_testing() {
-  DCHECK(!g_allow_legacy_extensions)
-      << "Multiple ScopedAllowLegacyExtensions objects are not allowed.";
-  return std::make_unique<base::AutoReset<bool>>(&g_allow_legacy_extensions,
-                                                 true);
-}
-
 // static
 bool Extension::InitExtensionID(extensions::Manifest* manifest,
                                 const base::FilePath& path,

extensions/common/extension.h

@@ -319,9 +319,6 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   void AddWebExtentPattern(const URLPattern& pattern);
   const URLPatternSet& web_extent() const { return extent_; }
 
-  using ScopedAllowLegacyExtensions = std::unique_ptr<base::AutoReset<bool>>;
-  static ScopedAllowLegacyExtensions allow_legacy_extensions_for_testing();
-
  private:
   friend class base::RefCountedThreadSafe<Extension>;
 

extensions/common/extension_unittest.cc

@@ -82,7 +82,7 @@ TEST(ExtensionTest, ExtensionManifestVersions) {
   EXPECT_TRUE(RunManifestVersionFailure(get_manifest(-1)));
 
   {
-    // Manifest v1 should only load if a command line switch is used....
+    // Manifest v1 should only load if a command line switch is used.
     base::test::ScopedCommandLine command_line;
     command_line.GetProcessCommandLine()->AppendSwitch(
         switches::kAllowLegacyExtensionManifests);
@@ -90,15 +90,6 @@ TEST(ExtensionTest, ExtensionManifestVersions) {
     EXPECT_TRUE(
         RunManifestVersionSuccess(get_manifest(base::nullopt), kType, 1));
   }
-
-  {
-    // ...or a runtime flag is set.
-    Extension::ScopedAllowLegacyExtensions allow_legacy_extensions =
-        Extension::allow_legacy_extensions_for_testing();
-    EXPECT_TRUE(RunManifestVersionSuccess(get_manifest(1), kType, 1));
-    EXPECT_TRUE(
-        RunManifestVersionSuccess(get_manifest(base::nullopt), kType, 1));
-  }
 }
 
 TEST(ExtensionTest, PlatformAppManifestVersions) {