Destroy web_app::ShortcutInfo on UI thread

chrome/browser/web_applications/web_app.cc

Index: chrome/browser/web_applications/web_app.cc
diff --git a/chrome/browser/web_applications/web_app.cc b/chrome/browser/web_applications/web_app.cc
index d0da8543b40f3381214cef41238a0f50b9bd3003..4b5355c4dcd5e7a7ad46badf6f549f00c25a8ae0 100644
--- a/chrome/browser/web_applications/web_app.cc
+++ b/chrome/browser/web_applications/web_app.cc
@@ -90,15 +90,13 @@ void UpdateAllShortcutsForShortcutInfo(
     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,
@@ -130,11 +128,15 @@ void ScheduleCreatePlatformShortcut(
     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
@@ -161,6 +163,14 @@ base::FilePath GetSanitizedFileName(const base::string16& 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() {}
@@ -430,10 +440,14 @@ void DeleteAllShortcuts(Profile* profile, const extensions::Extension* app) {
   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,