Extension: Fix DictionaryValue leak around webstore installer code.

WebstoreInstallHelper::Delegate::OnWebstoreParseSuccess passes
around unowned DictionaryValue pointers. In two cases in
dashboardPrivate and WebstoreStandaloneInstaller, there were
leaks.

Fix this by changing the method to pass owned pointers instead.

Bug: None
Test: None
Change-Id: I2165aa5cacac0d926cbeb8761ed4dc28a4610047
Reviewed-on: https://chromium-review.googlesource.com/c/1321865Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606149}

chrome/browser/chromeos/app_mode/kiosk_app_data.cc

@@ -221,13 +221,12 @@ class KioskAppData::WebstoreDataParser
   }
 
   // WebstoreInstallHelper::Delegate overrides:
-  void OnWebstoreParseSuccess(const std::string& id,
-                              const SkBitmap& icon,
-                              base::DictionaryValue* parsed_manifest) override {
-    // Takes ownership of |parsed_manifest|.
-    extensions::Manifest manifest(
-        extensions::Manifest::INVALID_LOCATION,
-        std::unique_ptr<base::DictionaryValue>(parsed_manifest));
+  void OnWebstoreParseSuccess(
+      const std::string& id,
+      const SkBitmap& icon,
+      std::unique_ptr<base::DictionaryValue> parsed_manifest) override {
+    extensions::Manifest manifest(extensions::Manifest::INVALID_LOCATION,
+                                  std::move(parsed_manifest));
 
     if (!IsValidKioskAppManifest(manifest)) {
       ReportFailure();

chrome/browser/extensions/api/dashboard_private/dashboard_private_api.cc

@@ -99,9 +99,9 @@ DashboardPrivateShowPermissionPromptForDelegatedInstallFunction::Run() {
 
 void DashboardPrivateShowPermissionPromptForDelegatedInstallFunction::
     OnWebstoreParseSuccess(
-    const std::string& id,
-    const SkBitmap& icon,
-    base::DictionaryValue* parsed_manifest) {
+        const std::string& id,
+        const SkBitmap& icon,
+        std::unique_ptr<base::DictionaryValue> parsed_manifest) {
   CHECK_EQ(params_->details.id, id);
   CHECK(parsed_manifest);
 
@@ -110,12 +110,8 @@ void DashboardPrivateShowPermissionPromptForDelegatedInstallFunction::
 
   std::string error;
   dummy_extension_ = ExtensionInstallPrompt::GetLocalizedExtensionForDisplay(
-      parsed_manifest,
-      Extension::FROM_WEBSTORE,
-      id,
-      localized_name,
-      std::string(),
-      &error);
+      parsed_manifest.get(), Extension::FROM_WEBSTORE, id, localized_name,
+      std::string(), &error);
 
   if (!dummy_extension_.get()) {
     OnWebstoreParseFailure(params_->details.id,

chrome/browser/extensions/api/dashboard_private/dashboard_private_api.h

@@ -42,9 +42,10 @@ class DashboardPrivateShowPermissionPromptForDelegatedInstallFunction
   ExtensionFunction::ResponseAction Run() override;
 
   // WebstoreInstallHelper::Delegate:
-  void OnWebstoreParseSuccess(const std::string& id,
-                              const SkBitmap& icon,
-                              base::DictionaryValue* parsed_manifest) override;
+  void OnWebstoreParseSuccess(
+      const std::string& id,
+      const SkBitmap& icon,
+      std::unique_ptr<base::DictionaryValue> parsed_manifest) override;
   void OnWebstoreParseFailure(const std::string& id,
                               InstallHelperResultCode result,
                               const std::string& error_message) override;

chrome/browser/extensions/api/webstore_private/webstore_private_api.cc

@@ -253,10 +253,10 @@ WebstorePrivateBeginInstallWithManifest3Function::Run() {
 void WebstorePrivateBeginInstallWithManifest3Function::OnWebstoreParseSuccess(
     const std::string& id,
     const SkBitmap& icon,
-    base::DictionaryValue* parsed_manifest) {
+    std::unique_ptr<base::DictionaryValue> parsed_manifest) {
   CHECK_EQ(details().id, id);
   CHECK(parsed_manifest);
-  parsed_manifest_.reset(parsed_manifest);
+  parsed_manifest_ = std::move(parsed_manifest);
   icon_ = icon;
 
   std::string localized_name =

chrome/browser/extensions/api/webstore_private/webstore_private_api.h

@@ -59,9 +59,10 @@ class WebstorePrivateBeginInstallWithManifest3Function
   ExtensionFunction::ResponseAction Run() override;
 
   // WebstoreInstallHelper::Delegate:
-  void OnWebstoreParseSuccess(const std::string& id,
-                              const SkBitmap& icon,
-                              base::DictionaryValue* parsed_manifest) override;
+  void OnWebstoreParseSuccess(
+      const std::string& id,
+      const SkBitmap& icon,
+      std::unique_ptr<base::DictionaryValue> parsed_manifest) override;
   void OnWebstoreParseFailure(const std::string& id,
                               InstallHelperResultCode result,
                               const std::string& error_message) override;

chrome/browser/extensions/webstore_install_helper.cc

@@ -139,7 +139,7 @@ void WebstoreInstallHelper::ReportResultsIfComplete() {
     return;
 
   if (error_.empty() && parsed_manifest_)
-    delegate_->OnWebstoreParseSuccess(id_, icon_, parsed_manifest_.release());
+    delegate_->OnWebstoreParseSuccess(id_, icon_, std::move(parsed_manifest_));
   else
     delegate_->OnWebstoreParseFailure(id_, parse_error_, error_);
 }

chrome/browser/extensions/webstore_install_helper.h

@@ -48,11 +48,11 @@ class WebstoreInstallHelper : public base::RefCounted<WebstoreInstallHelper>,
     };
 
     // Called when we've successfully parsed the manifest and decoded the icon
-    // in the utility process. Ownership of parsed_manifest is transferred.
+    // in the utility process.
     virtual void OnWebstoreParseSuccess(
         const std::string& id,
         const SkBitmap& icon,
-        base::DictionaryValue* parsed_manifest) = 0;
+        std::unique_ptr<base::DictionaryValue> parsed_manifest) = 0;
 
     // Called to indicate a parse failure. The |result_code| parameter should
     // indicate whether the problem was with the manifest or icon.

chrome/browser/extensions/webstore_standalone_installer.cc

@@ -306,7 +306,7 @@ void WebstoreStandaloneInstaller::OnWebstoreResponseParseFailure(
 void WebstoreStandaloneInstaller::OnWebstoreParseSuccess(
     const std::string& id,
     const SkBitmap& icon,
-    base::DictionaryValue* manifest) {
+    std::unique_ptr<base::DictionaryValue> manifest) {
   CHECK_EQ(id_, id);
 
   if (!CheckRequestorAlive()) {
@@ -314,7 +314,7 @@ void WebstoreStandaloneInstaller::OnWebstoreParseSuccess(
     return;
   }
 
-  manifest_.reset(manifest);
+  manifest_ = std::move(manifest);
   icon_ = icon;
 
   OnManifestParsed();

chrome/browser/extensions/webstore_standalone_installer.h

@@ -171,9 +171,10 @@ class WebstoreStandaloneInstaller
   void OnWebstoreResponseParseFailure(const std::string& error) override;
 
   // WebstoreInstallHelper::Delegate interface implementation.
-  void OnWebstoreParseSuccess(const std::string& id,
-                              const SkBitmap& icon,
-                              base::DictionaryValue* parsed_manifest) override;
+  void OnWebstoreParseSuccess(
+      const std::string& id,
+      const SkBitmap& icon,
+      std::unique_ptr<base::DictionaryValue> parsed_manifest) override;
   void OnWebstoreParseFailure(const std::string& id,
                               InstallHelperResultCode result_code,
                               const std::string& error_message) override;