Destroy web_app::ShortcutInfo on UI thread

chrome/browser/web_applications/web_app.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/web_applications/web_app.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 72 matching lines @@
   return web_app::GetWebAppDataDirectory(shortcut_info.profile_path,
                                          shortcut_info.extension_id,
                                          shortcut_info.url);
 }
 
 void UpdateAllShortcutsForShortcutInfo(
     const base::string16& old_app_title,
     const base::Closure& callback,
     std::unique_ptr<web_app::ShortcutInfo> shortcut_info) {
   base::FilePath shortcut_data_dir = GetShortcutDataDir(*shortcut_info);
-  base::Closure task = base::Bind(&web_app::internals::UpdatePlatformShortcuts,
-                                  shortcut_data_dir, old_app_title,
-                                  base::Passed(&shortcut_info));
-  if (callback.is_null()) {
-    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, task);
-  } else {
-    BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE, task,
-                                    callback);
-  }
+  auto* shortcut_info_ptr = shortcut_info.get();

[tapted, 2017/02/27 06:57:37]

perhaps spell out what the auto type is to make this easier to reason about

[tzik, 2017/02/27 07:42:12]

Done.

+  BrowserThread::PostTaskAndReply(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&web_app::internals::UpdatePlatformShortcuts,
+                 shortcut_data_dir, old_app_title, shortcut_info_ptr),

[tapted, 2017/02/27 06:57:37]

the raw pointer here feels bad.. and shouldn't it need base::Unretained? 

(same comments below)

[tzik, 2017/02/27 07:42:12]

Added base::Unretained().
base::Bind() does enforce Unretained only when it's a receiver of a method and a
raw pointer to a non-refcounted types. It does not yet to parameters.

+      base::Bind(&web_app::internals::DeleteShortcutInfoOnUIThread,
+                 base::Passed(&shortcut_info), callback));
 }
 
 void OnImageLoaded(std::unique_ptr<web_app::ShortcutInfo> shortcut_info,
                    web_app::ShortcutInfoCallback callback,
                    const gfx::ImageFamily& image_family) {
   // If the image failed to load (e.g. if the resource being loaded was empty)
   // use the standard application icon.
   if (image_family.empty()) {
     gfx::Image default_icon =
         ResourceBundle::GetSharedInstance().GetImageNamed(IDR_APP_DEFAULT_ICON);
@@ skipping 11 matching lines @@
   }
 
   callback.Run(std::move(shortcut_info));
 }
 
 void ScheduleCreatePlatformShortcut(
     web_app::ShortcutCreationReason reason,
     const web_app::ShortcutLocations& locations,
     std::unique_ptr<web_app::ShortcutInfo> shortcut_info) {
   base::FilePath shortcut_data_dir = GetShortcutDataDir(*shortcut_info);
-  BrowserThread::PostTask(
+
+  auto* shortcut_info_ptr = shortcut_info.get();
+  BrowserThread::PostTaskAndReply(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
           base::IgnoreResult(&web_app::internals::CreatePlatformShortcuts),
-          shortcut_data_dir, base::Passed(&shortcut_info), locations, reason));
+          shortcut_data_dir, shortcut_info_ptr, locations, reason),
+      base::Bind(&web_app::internals::DeleteShortcutInfoOnUIThread,
+                 base::Passed(&shortcut_info), base::Closure()));
 }
 
 }  // namespace
 
 namespace web_app {
 
 // The following string is used to build the directory name for
 // shortcuts to chrome applications (the kind which are installed
 // from a CRX).  Application shortcuts to URLs use the {host}_{path}
 // for the name of this directory.  Hosts can't include an underscore.
 // By starting this string with an underscore, we ensure that there
 // are no naming conflicts.
 static const char kCrxAppPrefix[] = "_crx_";
 
 namespace internals {
 
 base::FilePath GetSanitizedFileName(const base::string16& name) {
 #if defined(OS_WIN)
   base::string16 file_name = name;
 #else
   std::string file_name = base::UTF16ToUTF8(name);
 #endif
   base::i18n::ReplaceIllegalCharactersInPath(&file_name, '_');
   return base::FilePath(file_name);
 }
 
+void DeleteShortcutInfoOnUIThread(
+    std::unique_ptr<web_app::ShortcutInfo> shortcut_info,
+    const base::Closure& callback) {
+  shortcut_info.reset();
+  if (callback)
+    callback.Run();
+}
+
 }  // namespace internals
 
 ShortcutInfo::ShortcutInfo() {}
 ShortcutInfo::~ShortcutInfo() {}
 
 ShortcutLocations::ShortcutLocations()
     : on_desktop(false),
       applications_menu_location(APP_MENU_LOCATION_NONE),
       in_quick_launch_bar(false) {
 }
@@ skipping 249 matching lines @@
   GetShortcutInfoForApp(
       app, profile, base::Bind(&CreateShortcutsWithInfo, reason, locations));
 }
 
 void DeleteAllShortcuts(Profile* profile, const extensions::Extension* app) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   std::unique_ptr<ShortcutInfo> shortcut_info(
       ShortcutInfoForExtensionAndProfile(app, profile));
   base::FilePath shortcut_data_dir = GetShortcutDataDir(*shortcut_info);
-  BrowserThread::PostTask(
+  auto* shortcut_info_ptr = shortcut_info.get();
+
+  BrowserThread::PostTaskAndReply(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&web_app::internals::DeletePlatformShortcuts,
-                 shortcut_data_dir, base::Passed(&shortcut_info)));
+                 shortcut_data_dir, shortcut_info_ptr),
+      base::Bind(&web_app::internals::DeleteShortcutInfoOnUIThread,
+                 base::Passed(&shortcut_info), base::Closure()));
 }
 
 void UpdateAllShortcuts(const base::string16& old_app_title,
                         Profile* profile,
                         const extensions::Extension* app,
                         const base::Closure& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   GetShortcutInfoForApp(
       app, profile,
@@ skipping 37 matching lines @@
 
 #if defined(OS_LINUX)
 std::string GetWMClassFromAppName(std::string app_name) {
   base::i18n::ReplaceIllegalCharactersInPath(&app_name, '_');
   base::TrimString(app_name, "_", &app_name);
   return app_name;
 }
 #endif
 
 }  // namespace web_app