Unrevert again r63919: "Part 2 of immutable Extension refactor."

(Last landing broke chromeos and chromium linux builder.)

I made Extension a refcounted object, and privitized the existing
con/destructor and InitFromValue. The only way to get an Extension is to call
a factory method.

In the next CL, I plan to make the factory method return a const Extension,
to guarantee that no one can modify the Extension object after creation.

Note: There was a tricky part of this CL because of the difference in
semantics between scoped_ptr and scoped_refptr. I had to be careful not to use
ptr.release(), since that would result in leaks (an un-Released AddRef).

BUG=56558
TEST=no functional change

Original Review URL: http://codereview.chromium.org/3982001

TBR=aa

Review URL: http://codereview.chromium.org/4119010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64139 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 60 insertions, 50 deletions

diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index d2a7049..3bad1dc 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -77,7 +77,9 @@ TEST(ExtensionTest, InitFromValueInvalid) {
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
-  Extension extension(path);
+  scoped_refptr<Extension> extension_ptr(new Extension(path,
+                                                       Extension::INVALID));
+  Extension& extension = *extension_ptr;
   int error_code = 0;
   std::string error;
 
@@ -303,7 +305,9 @@ TEST(ExtensionTest, InitFromValueValid) {
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
-  Extension extension(path);
+  scoped_refptr<Extension> extension_ptr(new Extension(path,
+                                                       Extension::INVALID));
+  Extension& extension = *extension_ptr;
   std::string error;
   DictionaryValue input_value;
 
@@ -366,7 +370,9 @@ TEST(ExtensionTest, InitFromValueValidNameInRTL) {
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
-  Extension extension(path);
+  scoped_refptr<Extension> extension_ptr(new Extension(path,
+                                                       Extension::INVALID));
+  Extension& extension = *extension_ptr;
   std::string error;
   DictionaryValue input_value;
 
@@ -403,18 +409,20 @@ TEST(ExtensionTest, GetResourceURLAndPath) {
 #elif defined(OS_POSIX)
   FilePath path(FILE_PATH_LITERAL("/foo"));
 #endif
-  Extension extension(path);
   DictionaryValue input_value;
   input_value.SetString(keys::kVersion, "1.0.0.0");
   input_value.SetString(keys::kName, "my extension");
-  EXPECT_TRUE(extension.InitFromValue(input_value, false, NULL));
-
-  EXPECT_EQ(extension.url().spec() + "bar/baz.js",
-            Extension::GetResourceURL(extension.url(), "bar/baz.js").spec());
-  EXPECT_EQ(extension.url().spec() + "baz.js",
-            Extension::GetResourceURL(extension.url(), "bar/../baz.js").spec());
-  EXPECT_EQ(extension.url().spec() + "baz.js",
-            Extension::GetResourceURL(extension.url(), "../baz.js").spec());
+  scoped_refptr<Extension> extension(Extension::Create(
+      path, Extension::INVALID, input_value, false, NULL));
+  EXPECT_TRUE(extension.get());
+
+  EXPECT_EQ(extension->url().spec() + "bar/baz.js",
+            Extension::GetResourceURL(extension->url(), "bar/baz.js").spec());
+  EXPECT_EQ(extension->url().spec() + "baz.js",
+            Extension::GetResourceURL(extension->url(),
+                                      "bar/../baz.js").spec());
+  EXPECT_EQ(extension->url().spec() + "baz.js",
+            Extension::GetResourceURL(extension->url(), "../baz.js").spec());
 }
 
 TEST(ExtensionTest, LoadPageActionHelper) {
@@ -423,7 +431,9 @@ TEST(ExtensionTest, LoadPageActionHelper) {
 #else
     FilePath path(StringPrintf("/extension"));
 #endif
-  Extension extension(path);
+  scoped_refptr<Extension> extension_ptr(new Extension(path,
+                                                       Extension::INVALID));
+  Extension& extension = *extension_ptr;
   std::string error_msg;
   scoped_ptr<ExtensionAction> action;
   DictionaryValue input;
@@ -655,14 +665,15 @@ TEST(ExtensionTest, UpdateUrls) {
 #else
     FilePath path(StringPrintf("/extension%" PRIuS, i));
 #endif
-    Extension extension(path);
     std::string error;
 
     input_value.SetString(keys::kVersion, "1.0");
     input_value.SetString(keys::kName, "Test");
     input_value.SetString(keys::kUpdateURL, url.spec());
 
-    EXPECT_TRUE(extension.InitFromValue(input_value, false, &error));
+    scoped_refptr<Extension> extension(Extension::Create(
+        path, Extension::INVALID, input_value, false, &error));
+    EXPECT_TRUE(extension.get()) << error;
   }
 
   // Test some invalid update urls
@@ -679,13 +690,14 @@ TEST(ExtensionTest, UpdateUrls) {
 #else
     FilePath path(StringPrintf("/extension%" PRIuS, i));
 #endif
-    Extension extension(path);
     std::string error;
     input_value.SetString(keys::kVersion, "1.0");
     input_value.SetString(keys::kName, "Test");
     input_value.SetString(keys::kUpdateURL, invalid[i]);
 
-    EXPECT_FALSE(extension.InitFromValue(input_value, false, &error));
+    scoped_refptr<Extension> extension(Extension::Create(
+        path, Extension::INVALID, input_value, false, &error));
+    EXPECT_FALSE(extension.get());
     EXPECT_TRUE(MatchPattern(error, errors::kInvalidUpdateURL));
   }
 }
@@ -714,8 +726,8 @@ TEST(ExtensionTest, MimeTypeSniffing) {
   EXPECT_EQ("application/octet-stream", result);
 }
 
-static Extension* LoadManifest(const std::string& dir,
-                               const std::string& test_file) {
+static scoped_refptr<Extension> LoadManifest(const std::string& dir,
+                                             const std::string& test_file) {
   FilePath path;
   PathService::Get(chrome::DIR_TEST_DATA, &path);
   path = path.AppendASCII("extensions")
@@ -730,74 +742,70 @@ static Extension* LoadManifest(const std::string& dir,
     return NULL;
   }
 
-  scoped_ptr<Extension> extension(new Extension(path.DirName()));
-  EXPECT_TRUE(extension->InitFromValue(
-      *static_cast<DictionaryValue*>(result.get()), false, &error)) << error;
-
-  return extension.release();
+  scoped_refptr<Extension> extension = Extension::Create(
+      path.DirName(), Extension::INVALID,
+      *static_cast<DictionaryValue*>(result.get()), false, &error);
+  EXPECT_TRUE(extension) << error;
+  return extension;
 }
 
 TEST(ExtensionTest, EffectiveHostPermissions) {
-  scoped_ptr<Extension> extension;
+  scoped_refptr<Extension> extension;
   ExtensionExtent hosts;
 
-  extension.reset(LoadManifest("effective_host_permissions", "empty.json"));
+  extension = LoadManifest("effective_host_permissions", "empty.json");
   EXPECT_EQ(0u, extension->GetEffectiveHostPermissions().patterns().size());
   EXPECT_FALSE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions", "one_host.json"));
+  extension = LoadManifest("effective_host_permissions", "one_host.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_FALSE(hosts.ContainsURL(GURL("https://www.google.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "one_host_wildcard.json"));
+  extension = LoadManifest("effective_host_permissions",
+                           "one_host_wildcard.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://foo.google.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "two_hosts.json"));
+  extension = LoadManifest("effective_host_permissions", "two_hosts.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "https_not_considered.json"));
+  extension = LoadManifest("effective_host_permissions",
+                           "https_not_considered.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("https://google.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "two_content_scripts.json"));
+  extension = LoadManifest("effective_host_permissions",
+                           "two_content_scripts.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://google.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.reddit.com")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://news.ycombinator.com")));
   EXPECT_FALSE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "all_hosts.json"));
+  extension = LoadManifest("effective_host_permissions", "all_hosts.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://test/")));
   EXPECT_FALSE(hosts.ContainsURL(GURL("https://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "all_hosts2.json"));
+  extension = LoadManifest("effective_host_permissions", "all_hosts2.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("http://www.google.com")));
   EXPECT_TRUE(extension->HasEffectiveAccessToAllHosts());
 
-  extension.reset(LoadManifest("effective_host_permissions",
-                               "all_hosts3.json"));
+  extension = LoadManifest("effective_host_permissions", "all_hosts3.json");
   hosts = extension->GetEffectiveHostPermissions();
   EXPECT_FALSE(hosts.ContainsURL(GURL("http://test/")));
   EXPECT_TRUE(hosts.ContainsURL(GURL("https://test/")));
@@ -834,10 +842,10 @@ TEST(ExtensionTest, IsPrivilegeIncrease) {
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) {
-    scoped_ptr<Extension> old_extension(
+    scoped_refptr<Extension> old_extension(
         LoadManifest("allow_silent_upgrade",
                      std::string(kTests[i].base_name) + "_old.json"));
-    scoped_ptr<Extension> new_extension(
+    scoped_refptr<Extension> new_extension(
         LoadManifest("allow_silent_upgrade",
                      std::string(kTests[i].base_name) + "_new.json"));
 
@@ -912,11 +920,12 @@ TEST(ExtensionTest, ImageCaching) {
 
   // Initialize the Extension.
   std::string errors;
-  scoped_ptr<Extension> extension(new Extension(path));
   DictionaryValue values;
   values.SetString(keys::kName, "test");
   values.SetString(keys::kVersion, "0.1");
-  ASSERT_TRUE(extension->InitFromValue(values, false, &errors));
+  scoped_refptr<Extension> extension(Extension::Create(
+      path, Extension::INVALID, values, false, &errors));
+  ASSERT_TRUE(extension.get());
 
   // Create an ExtensionResource pointing at an icon.
   FilePath icon_relative_path(FILE_PATH_LITERAL("icon3.png"));
@@ -997,10 +1006,11 @@ TEST(ExtensionTest, OldUnlimitedStoragePermission) {
 
   // Initialize the extension and make sure the permission for unlimited storage
   // is present.
-  Extension extension(extension_path);
   std::string errors;
-  EXPECT_TRUE(extension.InitFromValue(dictionary, false, &errors));
-  EXPECT_TRUE(extension.HasApiPermission(
+  scoped_refptr<Extension> extension(Extension::Create(
+      extension_path, Extension::INVALID, dictionary, false, &errors));
+  EXPECT_TRUE(extension.get());
+  EXPECT_TRUE(extension->HasApiPermission(
       Extension::kUnlimitedStoragePermission));
 }
 
@@ -1036,8 +1046,8 @@ TEST(ExtensionTest, ApiPermissions) {
     { "tabs.getSelected", false},
   };
 
-  scoped_ptr<Extension> extension;
-  extension.reset(LoadManifest("empty_manifest", "empty.json"));
+  scoped_refptr<Extension> extension;
+  extension = LoadManifest("empty_manifest", "empty.json");
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) {
     EXPECT_EQ(kTests[i].expect_success,