[Extensions] Update developerPrivate API to use base-64 data urls for icons

chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc

Index: chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc
diff --git a/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc b/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc
index eece9df0c2c088b0f476b0345f038f0b8624ee56..3204b7ef71f6ac2b13f9dff94838de45940a2f19 100644
--- a/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc
+++ b/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc
@@ -55,6 +55,30 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
     InitializeEmptyExtensionService();
   }
 
+  void OnInfosGenerated(const ExtensionInfoGenerator::ExtensionInfoList& list) {
+    EXPECT_EQ(1u, list.size());
+    if (!list.empty())
+      last_info_ = list[0];
+    quit_closure_.Run();
+    quit_closure_.Reset();
+  }
+
+  linked_ptr<developer::ExtensionInfo> GenerateExtensionInfo(
+      const std::string& extension_id) {
+    base::RunLoop run_loop;
+    quit_closure_ = run_loop.QuitClosure();
+    scoped_ptr<ExtensionInfoGenerator> generator(
+        new ExtensionInfoGenerator(browser_context()));
+    generator->CreateExtensionInfo(
+        extension_id,
+        base::Bind(&ExtensionInfoGeneratorUnitTest::OnInfosGenerated,
+                   base::Unretained(this)));
+    run_loop.Run();
+    linked_ptr<developer::ExtensionInfo> result = last_info_;

[not at google - send to devlin, 2015/04/22 21:38:37]

Rather than making last_info_ a member variable you can keep everything local by
passing a linked_ptr* to OnInfosGenerated. Assign list[0] to it. It would look
like:

linked_ptr<developer::ExtensionInfo> result;
generator->CreateExtensionInfo(..., base::Bind(..., &result));
return result;

[Devlin, 2015/04/23 19:17:21]

I like it. Done.

+    last_info_.reset();
+    return result;
+  }
+
   const scoped_refptr<const Extension> CreateExtension(
       const std::string& name,
       ListBuilder& permissions) {
@@ -76,7 +100,7 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
     return extension;
   }
 
-  scoped_ptr<developer::ExtensionInfo> CreateExtensionInfoFromPath(
+  linked_ptr<developer::ExtensionInfo> CreateExtensionInfoFromPath(

[not at google - send to devlin, 2015/04/22 21:38:37]

Why did you need to change this from scoped_ptr to linked_ptr?

[Devlin, 2015/04/23 19:17:21]

Originally thought because we couldn't be sure that it would be the only
linked_ptr reference (and if it isn't, everything explodes) - but looking
closer, I think we can make it work.  Done.

       const base::FilePath& extension_path,
       Manifest::Location location) {
     std::string error;
@@ -90,11 +114,10 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
         extension_path, location, *extension_data, Extension::REQUIRE_KEY,
         &error));
     CHECK(extension.get());
+    service()->AddExtension(extension.get());
     EXPECT_EQ(std::string(), error);
 
-    return ExtensionInfoGenerator(browser_context()).CreateExtensionInfo(
-        *extension,
-        api::developer_private::EXTENSION_STATE_ENABLED);
+    return GenerateExtensionInfo(extension->id());
   }
 
   void CompareExpectedAndActualOutput(
@@ -107,7 +130,7 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
     EXPECT_EQ(std::string(), error);
 
     // Produce test output.
-    scoped_ptr<developer::ExtensionInfo> info =
+    linked_ptr<developer::ExtensionInfo> info =
         CreateExtensionInfoFromPath(extension_path, Manifest::INVALID_LOCATION);
     info->views = views;
     scoped_ptr<base::DictionaryValue> actual_output_data = info->ToValue();
@@ -136,6 +159,12 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
       }
     }
   }
+
+ private:
+  linked_ptr<developer::ExtensionInfo> last_info_;
+  base::Closure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionInfoGeneratorUnitTest);
 };
 
 // Test some of the basic fields.
@@ -148,7 +177,7 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) {
 
   const char kName[] = "extension name";
   const char kVersion[] = "1.0.0.1";
-  std::string id = crx_file::id_util::GenerateId(kName);
+  std::string id = crx_file::id_util::GenerateId("alpha");
   scoped_ptr<base::DictionaryValue> manifest =
       DictionaryBuilder().Set("name", kName)
                          .Set("version", kVersion)
@@ -202,10 +231,9 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) {
   // It's not feasible to validate every field here, because that would be
   // a duplication of the logic in the method itself. Instead, test a handful
   // of fields for sanity.
-  scoped_ptr<api::developer_private::ExtensionInfo> info =
-      ExtensionInfoGenerator(browser_context()).CreateExtensionInfo(
-          *extension, developer::EXTENSION_STATE_ENABLED);
-  ASSERT_TRUE(info);
+  linked_ptr<api::developer_private::ExtensionInfo> info =
+      GenerateExtensionInfo(extension->id());
+  ASSERT_TRUE(info.get());
   EXPECT_EQ(kName, info->name);
   EXPECT_EQ(id, info->id);
   EXPECT_EQ(kVersion, info->version);
@@ -238,12 +266,13 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) {
   // Test an extension that isn't unpacked.
   manifest_copy->SetString("update_url",
                            "https://clients2.google.com/service/update2/crx");
+  id = crx_file::id_util::GenerateId("beta");
   extension = ExtensionBuilder().SetManifest(manifest_copy.Pass())
                                 .SetLocation(Manifest::EXTERNAL_PREF)
                                 .SetID(id)
                                 .Build();
-  info = ExtensionInfoGenerator(browser_context()).CreateExtensionInfo(
-             *extension, developer::EXTENSION_STATE_ENABLED);
+  service()->AddExtension(extension.get());
+  info = GenerateExtensionInfo(extension->id());
   EXPECT_EQ(developer::LOCATION_THIRD_PARTY, info->location);
   EXPECT_FALSE(info->path);
 }
@@ -322,11 +351,8 @@ TEST_F(ExtensionInfoGeneratorUnitTest, ExtensionInfoRunOnAllUrls) {
   scoped_refptr<const Extension> no_urls_extension =
       CreateExtension("no urls", ListBuilder().Pass());
 
-  ExtensionInfoGenerator generator(browser_context());
-  scoped_ptr<developer::ExtensionInfo> info =
-      generator.CreateExtensionInfo(
-          *all_urls_extension,
-          api::developer_private::EXTENSION_STATE_ENABLED);
+  linked_ptr<developer::ExtensionInfo> info =
+      GenerateExtensionInfo(all_urls_extension->id());
 
   // The extension should want all urls, but not currently have it.
   EXPECT_TRUE(info->run_on_all_urls.is_enabled);
@@ -336,14 +362,12 @@ TEST_F(ExtensionInfoGeneratorUnitTest, ExtensionInfoRunOnAllUrls) {
   util::SetAllowedScriptingOnAllUrls(all_urls_extension->id(), profile(), true);
 
   // Now the extension should both want and have all urls.
-  info = generator.CreateExtensionInfo(*all_urls_extension,
-                                       developer::EXTENSION_STATE_ENABLED);
+  info = GenerateExtensionInfo(all_urls_extension->id());
   EXPECT_TRUE(info->run_on_all_urls.is_enabled);
   EXPECT_TRUE(info->run_on_all_urls.is_active);
 
   // The other extension should neither want nor have all urls.
-  info = generator.CreateExtensionInfo(*no_urls_extension,
-                                       developer::EXTENSION_STATE_ENABLED);
+  info = GenerateExtensionInfo(no_urls_extension->id());
   EXPECT_FALSE(info->run_on_all_urls.is_enabled);
   EXPECT_FALSE(info->run_on_all_urls.is_active);
 
@@ -357,16 +381,14 @@ TEST_F(ExtensionInfoGeneratorUnitTest, ExtensionInfoRunOnAllUrls) {
 
   // Since the extension doesn't have access to all urls (but normally would),
   // the extension should have the "want" flag even with the switch off.
-  info = generator.CreateExtensionInfo(*all_urls_extension,
-                                       developer::EXTENSION_STATE_ENABLED);
+  info = GenerateExtensionInfo(all_urls_extension->id());
   EXPECT_TRUE(info->run_on_all_urls.is_enabled);
   EXPECT_FALSE(info->run_on_all_urls.is_active);
 
   // If we grant the extension all urls, then the checkbox should still be
   // there, since it has an explicitly-set user preference.
   util::SetAllowedScriptingOnAllUrls(all_urls_extension->id(), profile(), true);
-  info = generator.CreateExtensionInfo(*all_urls_extension,
-                                       developer::EXTENSION_STATE_ENABLED);
+  info = GenerateExtensionInfo(all_urls_extension->id());
   EXPECT_TRUE(info->run_on_all_urls.is_enabled);
   EXPECT_TRUE(info->run_on_all_urls.is_active);
 
@@ -376,8 +398,7 @@ TEST_F(ExtensionInfoGeneratorUnitTest, ExtensionInfoRunOnAllUrls) {
 
   // Even though the extension has all_urls permission, the checkbox shouldn't
   // show up without the switch.
-  info = generator.CreateExtensionInfo(*all_urls_extension,
-                                       developer::EXTENSION_STATE_ENABLED);
+  info = GenerateExtensionInfo(all_urls_extension->id());
   EXPECT_FALSE(info->run_on_all_urls.is_enabled);
   EXPECT_TRUE(info->run_on_all_urls.is_active);
 }