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

Change the developerPrivate API to uniformly use base-64 data urls for icons,
rather than relying on the custom url handler.

BUG=461039

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

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

chrome/browser/extensions/api/developer_private/developer_private_api.h

@@ -8,6 +8,7 @@
 #include <set>
 
 #include "base/files/file.h"
+#include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/extensions/api/developer_private/entry_picker.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/extensions/error_console/error_console.h"
 #include "chrome/browser/extensions/extension_uninstall_dialog.h"
 #include "chrome/browser/extensions/pack_extension_job.h"
+#include "chrome/common/extensions/api/developer_private.h"
 #include "chrome/common/extensions/webstore_install_result.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/browser_context_keyed_api_factory.h"
@@ -30,7 +32,9 @@ class Profile;
 
 namespace extensions {
 
+class EventRouter;
 class ExtensionError;
+class ExtensionInfoGenerator;
 class ExtensionRegistry;
 class ExtensionSystem;
 class ManagementPolicy;
@@ -42,27 +46,8 @@ namespace api {
 class EntryPicker;
 class EntryPickerClient;
 
-namespace developer_private {
-
-struct ItemInfo;
-struct ItemInspectView;
-struct ProjectInfo;
-
-namespace RequestFileSource {
-struct Params;
-}
-
-}  // namespace developer_private
-
 }  // namespace api
 
-namespace developer = api::developer_private;
-
-typedef std::vector<linked_ptr<developer::ItemInfo> > ItemInfoList;
-typedef std::vector<linked_ptr<developer::ProjectInfo> > ProjectInfoList;
-typedef std::vector<linked_ptr<developer::ItemInspectView> >
-    ItemInspectViewList;
-
 class DeveloperPrivateEventRouter : public ExtensionRegistryObserver,
                                     public ErrorConsole::Observer,
                                     public ProcessManagerObserver,
@@ -111,6 +96,16 @@ class DeveloperPrivateEventRouter : public ExtensionRegistryObserver,
   void OnExtensionActionVisibilityChanged(const std::string& extension_id,
                                           bool is_now_visible) override;
 
+  // Broadcasts an event to all listeners.
+  void BroadcastItemStateChanged(api::developer_private::EventType event_type,
+                                 const std::string& id);
+  void BroadcastItemStateChangedHelper(
+      api::developer_private::EventType event_type,
+      const std::string& extension_id,
+      scoped_ptr<ExtensionInfoGenerator> info_generator,
+      const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>&
+          infos);
+
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>
       extension_registry_observer_;
   ScopedObserver<ErrorConsole, ErrorConsole::Observer> error_console_observer_;
@@ -123,6 +118,8 @@ class DeveloperPrivateEventRouter : public ExtensionRegistryObserver,
 
   Profile* profile_;
 
+  EventRouter* event_router_;
+
   // The set of IDs of the Extensions that have subscribed to DeveloperPrivate
   // events. Since the only consumer of the DeveloperPrivate API is currently
   // the Apps Developer Tool (which replaces the chrome://extensions page), we
@@ -131,6 +128,8 @@ class DeveloperPrivateEventRouter : public ExtensionRegistryObserver,
   // when, e.g., the Apps Developer Tool throws an error.
   std::set<std::string> extension_ids_;
 
+  base::WeakPtrFactory<DeveloperPrivateEventRouter> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(DeveloperPrivateEventRouter);
 };
 
@@ -218,33 +217,53 @@ class DeveloperPrivateGetItemsInfoFunction
   ~DeveloperPrivateGetItemsInfoFunction() override;
   ResponseAction Run() override;
 
-  void GetIconsOnFileThread(
-      std::map<std::string, ExtensionResource> resource_map);
-  void Finish();
+  void OnInfosGenerated(
+      const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>&
+          infos);
+
+  scoped_ptr<ExtensionInfoGenerator> info_generator_;
 
-  ItemInfoList item_list_;
+  DISALLOW_COPY_AND_ASSIGN(DeveloperPrivateGetItemsInfoFunction);
 };
 
 class DeveloperPrivateGetExtensionsInfoFunction
     : public DeveloperPrivateAPIFunction {
  public:
+  DeveloperPrivateGetExtensionsInfoFunction();
   DECLARE_EXTENSION_FUNCTION("developerPrivate.getExtensionsInfo",
                              DEVELOPERPRIVATE_GETEXTENSIONSINFO);
 
  private:
   ~DeveloperPrivateGetExtensionsInfoFunction() override;
   ResponseAction Run() override;
+
+  void OnInfosGenerated(
+      const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>&
+          infos);
+
+  scoped_ptr<ExtensionInfoGenerator> info_generator_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeveloperPrivateGetExtensionsInfoFunction);
 };
 
 class DeveloperPrivateGetExtensionInfoFunction
     : public DeveloperPrivateAPIFunction {
  public:
+  DeveloperPrivateGetExtensionInfoFunction();
   DECLARE_EXTENSION_FUNCTION("developerPrivate.getExtensionInfo",
                              DEVELOPERPRIVATE_GETEXTENSIONINFO);
 
  private:
   ~DeveloperPrivateGetExtensionInfoFunction() override;
   ResponseAction Run() override;
+
+  void OnInfosGenerated(
+      const std::vector<linked_ptr<api::developer_private::ExtensionInfo>>&
+          infos);
+
+  scoped_ptr<ExtensionInfoGenerator> info_generator_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeveloperPrivateGetExtensionInfoFunction);
 };
 
 class DeveloperPrivateGetProfileConfigurationFunction

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

@@ -54,6 +54,8 @@ linked_ptr<api::developer_private::ItemInfo> MangleExtensionInfo(
   result->allow_file_access = info.file_access.is_active;
   result->wants_file_access = info.file_access.is_enabled;
 
+  result->icon_url = info.icon_url;
+
   result->incognito_enabled = info.incognito_access.is_active;
   result->allow_incognito = info.incognito_access.is_enabled;
 

chrome/browser/extensions/api/developer_private/extension_info_generator.h

@@ -5,44 +5,68 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_API_DEVELOPER_PRIVATE_EXTENSION_INFO_GENERATOR_H_
 #define CHROME_BROWSER_EXTENSIONS_API_DEVELOPER_PRIVATE_EXTENSION_INFO_GENERATOR_H_
 
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/common/extensions/api/developer_private.h"
 
 namespace content {
 class BrowserContext;
 }
 
+namespace gfx {
+class Image;
+}
+
 namespace extensions {
 class ErrorConsole;
 class Extension;
 class ExtensionActionAPI;
 class ExtensionPrefs;
 class ExtensionSystem;
+class ImageLoader;
 class WarningService;
 
 // Generates the developerPrivate api's specification for ExtensionInfo.
+// This class is designed to only have one generation running at a time!
 class ExtensionInfoGenerator {
  public:
   using ExtensionInfoList =
       std::vector<linked_ptr<api::developer_private::ExtensionInfo>>;
 
+  using ExtensionInfosCallback = base::Callback<void(const ExtensionInfoList&)>;
+
   explicit ExtensionInfoGenerator(content::BrowserContext* context);
   ~ExtensionInfoGenerator();
 
-  // Returns the ExtensionInfo for a given |extension| and |state|.
-  scoped_ptr<api::developer_private::ExtensionInfo> CreateExtensionInfo(
-      const Extension& extension,
-      api::developer_private::ExtensionState state);
-  // Returns an ExtensionInfo for the given |extension_id|, if the extension
-  // can be found.
-  scoped_ptr<api::developer_private::ExtensionInfo> CreateExtensionInfo(
-      const std::string& id);
+  // Creates and asynchronously returns an ExtensionInfo for the given
+  // |extension_id|, if the extension can be found.
+  // If the extension cannot be found, an empty vector is passed to |callback|.
+  void CreateExtensionInfo(const std::string& id,
+                           const ExtensionInfosCallback& callback);
 
-  // Returns a collection of ExtensionInfos, optionally including disabled and
-  // terminated.
-  ExtensionInfoList CreateExtensionsInfo(bool include_disabled,
-                                         bool include_terminated);
+  // Creates and asynchronously returns a collection of ExtensionInfos,
+  // optionally including disabled and terminated.
+  void CreateExtensionsInfo(bool include_disabled,
+                            bool include_terminated,
+                            const ExtensionInfosCallback& callback);
 
  private:
+  // Creates an ExtensionInfo for the given |extension| and |state|, and
+  // asynchronously adds it to the |list|.
+  void CreateExtensionInfoHelper(const Extension& extension,
+                                 api::developer_private::ExtensionState state);
+
+  // Callback for the asynchronous image loading.
+  void OnImageLoaded(scoped_ptr<api::developer_private::ExtensionInfo> info,
+                     const gfx::Image& image);
+
+  // Returns the icon url for the default icon to use.
+  const std::string& GetDefaultIconUrl(bool is_app, bool is_disabled);
+
+  // Returns an icon url from the given image, optionally applying a greyscale.
+  std::string GetIconUrlFromImage(const gfx::Image& image,
+                                  bool should_greyscale);
+
   // Various systems, cached for convenience.
   content::BrowserContext* browser_context_;
   ExtensionSystem* extension_system_;
@@ -50,6 +74,24 @@ class ExtensionInfoGenerator {
   ExtensionActionAPI* extension_action_api_;
   WarningService* warning_service_;
   ErrorConsole* error_console_;
+  ImageLoader* image_loader_;
+
+  // The number of pending image loads.
+  size_t pending_image_loads_;
+
+  // Default icons, cached and lazily initialized.
+  std::string default_app_icon_url_;
+  std::string default_extension_icon_url_;
+  std::string default_disabled_app_icon_url_;
+  std::string default_disabled_extension_icon_url_;
+
+  // The list of extension infos that have been generated.
+  ExtensionInfoList list_;
+
+  // The callback to run once all infos have been created.
+  ExtensionInfosCallback callback_;
+
+  base::WeakPtrFactory<ExtensionInfoGenerator> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionInfoGenerator);
 };

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

@@ -55,6 +55,31 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
     InitializeEmptyExtensionService();
   }
 
+  void OnInfosGenerated(linked_ptr<developer::ExtensionInfo>* info_out,
+                        const ExtensionInfoGenerator::ExtensionInfoList& list) {
+    EXPECT_EQ(1u, list.size());
+    if (!list.empty())
+      *info_out = list[0];
+    quit_closure_.Run();
+    quit_closure_.Reset();
+  }
+
+  scoped_ptr<developer::ExtensionInfo> GenerateExtensionInfo(
+      const std::string& extension_id) {
+    linked_ptr<developer::ExtensionInfo> info;
+    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),
+                   base::Unretained(&info)));
+    run_loop.Run();
+    return make_scoped_ptr(info.release());
+  }
+
   const scoped_refptr<const Extension> CreateExtension(
       const std::string& name,
       ListBuilder& permissions) {
@@ -90,11 +115,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(
@@ -136,6 +160,11 @@ class ExtensionInfoGeneratorUnitTest : public ExtensionServiceTestBase {
       }
     }
   }
+
+ private:
+  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)
@@ -203,9 +232,8 @@ TEST_F(ExtensionInfoGeneratorUnitTest, BasicInfoTest) {
   // 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);
+      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);
+      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);
 }

chrome/browser/resources/extensions/extensions.css

@@ -159,6 +159,7 @@ html[dir=rtl] #footer-section {
 
 .extension-list-item {
   background-repeat: no-repeat;
+  background-size: 48px 48px;
   display: -webkit-box;
   min-height: 48px;
 }

chrome/browser/ui/webui/extensions/extension_settings_browsertest.js

@@ -151,6 +151,32 @@ TEST_F('ExtensionSettingsWebUITest', 'testChromeSendHandled', function() {
   this.nextStep();
 });
 
+/**
+ * @param {chrome.developerPrivate.EventType} eventType
+ * @param {function():void} callback
+ * @constructor
+ */
+function UpdateListener(eventType, callback) {
+  this.callback_ = callback;
+  this.eventType_ = eventType;
+  this.onItemStateChangedListener_ = this.onItemStateChanged_.bind(this);
+  chrome.developerPrivate.onItemStateChanged.addListener(
+      this.onItemStateChangedListener_);
+}
+
+UpdateListener.prototype = {
+  /** @private */
+  onItemStateChanged_: function(data) {
+    if (this.eventType_ == data.event_type) {
+      window.setTimeout(function() {
+        chrome.developerPrivate.onItemStateChanged.removeListener(
+            this.onItemStateChangedListener_);
+        this.callback_();
+      }.bind(this), 0);
+    }
+  }
+};
+
 function BasicExtensionSettingsWebUITest() {}
 
 BasicExtensionSettingsWebUITest.prototype = {
@@ -170,30 +196,38 @@ BasicExtensionSettingsWebUITest.prototype = {
 
   /** @protected */
   verifyDisabledWorks: function() {
-    chrome.management.setEnabled(GOOD_CRX_ID, false, function() {
+    var listener = new UpdateListener(
+        chrome.developerPrivate.EventType.UNLOADED,
+        function() {
       var node = getRequiredElement(GOOD_CRX_ID);
       assertTrue(node.classList.contains('inactive-extension'));
       this.nextStep();
     }.bind(this));
+    chrome.management.setEnabled(GOOD_CRX_ID, false);
   },
 
   /** @protected */
   verifyEnabledWorks: function() {
-    chrome.management.setEnabled(GOOD_CRX_ID, true, function() {
+    var listener = new UpdateListener(
+        chrome.developerPrivate.EventType.LOADED,
+        function() {
       var node = getRequiredElement(GOOD_CRX_ID);
       assertFalse(node.classList.contains('inactive-extension'));
       this.nextStep();
     }.bind(this));
+    chrome.management.setEnabled(GOOD_CRX_ID, true);
   },
 
   /** @protected */
   verifyUninstallWorks: function() {
-    var next = this.nextStep.bind(this);
+    var listener = new UpdateListener(
+        chrome.developerPrivate.EventType.UNINSTALLED,
+        function() {
+      assertEquals(null, $(GOOD_CRX_ID));
+      this.nextStep();
+    }.bind(this));
     chrome.test.runWithUserGesture(function() {
-      chrome.management.uninstall(GOOD_CRX_ID, function() {
-        assertEquals(null, $(GOOD_CRX_ID));
-        next();
-      });
+      chrome.management.uninstall(GOOD_CRX_ID);
     });
   },
 };

chrome/test/data/extensions/api_test/developer/generated_output/behllobkkfkfnphdnhnkndlbkcpglgmj.json

@@ -19,7 +19,6 @@
       "specified": false,
       "url": ""
    },
-   "iconUrl": "chrome://extension-icon/behllobkkfkfnphdnhnkndlbkcpglgmj/48/1",
    "id": "behllobkkfkfnphdnhnkndlbkcpglgmj",
    "incognitoAccess": {
       "isActive": false,

chrome/test/data/extensions/api_test/developer/generated_output/bjafgdebaacbbbecmhlhpofkepfkgcpa.json

@@ -19,7 +19,6 @@
       "specified": false,
       "url": ""
    },
-   "iconUrl": "chrome://extension-icon/bjafgdebaacbbbecmhlhpofkepfkgcpa/48/1",
    "id": "bjafgdebaacbbbecmhlhpofkepfkgcpa",
    "incognitoAccess": {
       "isActive": false,

chrome/test/data/extensions/api_test/developer/generated_output/hpiknbiabeeppbpihjehijgoemciehgk.json

@@ -19,7 +19,6 @@
       "specified": false,
       "url": ""
    },
-   "iconUrl": "chrome://extension-icon/hpiknbiabeeppbpihjehijgoemciehgk/48/1",
    "id": "hpiknbiabeeppbpihjehijgoemciehgk",
    "incognitoAccess": {
       "isActive": false,