Stop passing raw pointers to base::Value API in c/b/ui

chrome/browser/ui/webui/ntp/app_launcher_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/ntp/app_launcher_handler.h"
 
 #include <stddef.h>
 
 #include <vector>

[jdoerrie, 2017/04/13 09:08:52]

#include <utility>

[vabr (Chromium), 2017/04/13 12:03:14]

Done.

 
 #include "apps/metrics_names.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/i18n/rtl.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_ui_util.h"
@@ skipping 325 matching lines @@
     content::BrowserContext* browser_context,
     const Extension* extension,
     extensions::UninstallReason reason) {
   AppRemoved(extension, true);
 }
 
 void AppLauncherHandler::FillAppDictionary(base::DictionaryValue* dictionary) {
   // CreateAppInfo and ClearOrdinals can change the extension prefs.
   base::AutoReset<bool> auto_reset(&ignore_changes_, true);
 
-  base::ListValue* list = new base::ListValue();
+  auto installed_extensions = base::MakeUnique<base::ListValue>();
   Profile* profile = Profile::FromWebUI(web_ui());
   PrefService* prefs = profile->GetPrefs();
 
   for (std::set<std::string>::iterator it = visible_apps_.begin();
        it != visible_apps_.end(); ++it) {
     const Extension* extension = extension_service_->GetInstalledExtension(*it);
     if (extension && extensions::ui_util::ShouldDisplayInNewTabPage(
             extension, profile)) {
-      list->Append(GetAppInfo(extension));
+      installed_extensions->Append(GetAppInfo(extension));
     }
   }
 
-  dictionary->Set("apps", list);
+  dictionary->Set("apps", std::move(installed_extensions));
 
   const base::ListValue* app_page_names =
       prefs->GetList(prefs::kNtpAppPageNames);
   if (!app_page_names || !app_page_names->GetSize()) {
     ListPrefUpdate update(prefs, prefs::kNtpAppPageNames);
     base::ListValue* list = update.Get();
-    list->Set(0, new base::Value(
+    list->Set(0, base::MakeUnique<base::Value>(
                      l10n_util::GetStringUTF16(IDS_APP_DEFAULT_PAGE_NAME)));

[jdoerrie, 2017/04/13 09:08:52]

SetString

[vabr (Chromium), 2017/04/13 12:03:14]

There is no SetString in ListValue.

-    dictionary->Set("appPageNames",
-                    static_cast<base::ListValue*>(list->DeepCopy()));
+    dictionary->Set("appPageNames", base::MakeUnique<base::Value>(*list));
   } else {
     dictionary->Set("appPageNames",
-                    static_cast<base::ListValue*>(app_page_names->DeepCopy()));
+                    base::MakeUnique<base::Value>(*app_page_names));
   }
 }
 
 std::unique_ptr<base::DictionaryValue> AppLauncherHandler::GetAppInfo(
     const Extension* extension) {
   std::unique_ptr<base::DictionaryValue> app_info(new base::DictionaryValue());
   // CreateAppInfo can change the extension prefs.
   base::AutoReset<bool> auto_reset(&ignore_changes_, true);
   CreateAppInfo(extension, extension_service_, app_info.get());
   return app_info;
@@ skipping 285 matching lines @@
   base::string16 name;
   CHECK(args->GetString(0, &name));
 
   double page_index;
   CHECK(args->GetDouble(1, &page_index));
 
   base::AutoReset<bool> auto_reset(&ignore_changes_, true);
   PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
   ListPrefUpdate update(prefs, prefs::kNtpAppPageNames);
   base::ListValue* list = update.Get();
-  list->Set(static_cast<size_t>(page_index), new base::Value(name));
+  list->Set(static_cast<size_t>(page_index),
+            base::MakeUnique<base::Value>(name));

[jdoerrie, 2017/04/13 09:08:52]

SetString

[vabr (Chromium), 2017/04/13 12:03:14]

There is no SetString in ListValue.
But I at least changed |name| to 8-bit string to avoid the unnecessary
conversions going on inside Value's implementation.

[jdoerrie, 2017/04/13 12:09:41]

True, my bad. I saw 'Set' and immediately assumed a |dict|, sorry about that.
Making name a std::string sounds good to me :)

 }
 
 void AppLauncherHandler::HandleGenerateAppForLink(const base::ListValue* args) {
   std::string url;
   CHECK(args->GetString(0, &url));
   GURL launch_url(url);
 
   base::string16 title;
   CHECK(args->GetString(1, &title));
 
@@ skipping 175 matching lines @@
       base::Value(!extension_id_prompting_.empty()));
 }
 
 bool AppLauncherHandler::ShouldShow(const Extension* extension) const {
   if (ignore_changes_ || !has_loaded_apps_ || !extension->is_app())
     return false;
 
   Profile* profile = Profile::FromWebUI(web_ui());
   return extensions::ui_util::ShouldDisplayInNewTabPage(extension, profile);
 }