Destroy web_app::ShortcutInfo on UI thread

chrome/browser/web_applications/web_app.h

Index: chrome/browser/web_applications/web_app.h
diff --git a/chrome/browser/web_applications/web_app.h b/chrome/browser/web_applications/web_app.h
index 1e4234920bfc7d50a934120ed5b8d6a01a645c23..e9f4358d4160d9b362f3724438c3112d5f12c81e 100644
--- a/chrome/browser/web_applications/web_app.h
+++ b/chrome/browser/web_applications/web_app.h
@@ -58,8 +58,9 @@ struct ShortcutInfo {
 
  private:
   // ShortcutInfo must not be copied; generally it is passed around via
-  // scoped_ptrs. This is to allow passing ShortcutInfos between threads,
-  // despite ImageFamily having a non-thread-safe reference count.
+  // unique_ptr. Since ImageFamily has a non-thread-safe reference count in
+  // its member and is bound to UI thread, destroy ShortcutInfo instance
+  // on UI thread.
   DISALLOW_COPY_AND_ASSIGN(ShortcutInfo);
 };
 
@@ -229,24 +230,23 @@ std::vector<base::FilePath> GetShortcutPaths(
 // shortcut, and is also used as the UserDataDir for platform app shortcuts.
 // |shortcut_info| contains info about the shortcut to create, and
 // |creation_locations| contains information about where to create them.
-bool CreatePlatformShortcuts(
-    const base::FilePath& shortcut_data_path,
-    std::unique_ptr<ShortcutInfo> shortcut_info,
-    const ShortcutLocations& creation_locations,
-    ShortcutCreationReason creation_reason);
+bool CreatePlatformShortcuts(const base::FilePath& shortcut_data_path,
+                             ShortcutInfo* shortcut_info,

[Matt Giuca, 2017/03/01 08:49:55]

This should take a const ShortcutInfo&, not a pointer.

(It looks like all four implementations don't need a pointer.)

If there's a reason I'm not seeing, feel free to ignore.

[tapted, 2017/03/01 10:38:57]

Yah - I think base::Bind requires types passed to const& to be movable (or
copyable) - they just get stored in a std::tuple. (but I might be wrong - if
const& works then \o/)

[tzik, 2017/03/01 12:34:04]

It's not common, but we have base::ConstRef() wrapper to do that. Updated.

+                             const ShortcutLocations& creation_locations,
+                             ShortcutCreationReason creation_reason);
 
 // Delete all the shortcuts we have added for this extension. This is the
 // platform specific implementation of the DeleteAllShortcuts function, and
 // is executed on the FILE thread.
 void DeletePlatformShortcuts(const base::FilePath& shortcut_data_path,
-                             std::unique_ptr<ShortcutInfo> shortcut_info);
+                             ShortcutInfo* shortcut_info);

[Matt Giuca, 2017/03/01 08:49:55]

Same.

[tzik, 2017/03/01 12:34:04]

Done.

 
 // Updates all the shortcuts we have added for this extension. This is the
 // platform specific implementation of the UpdateAllShortcuts function, and
 // is executed on the FILE thread.
 void UpdatePlatformShortcuts(const base::FilePath& shortcut_data_path,
                              const base::string16& old_app_title,
-                             std::unique_ptr<ShortcutInfo> shortcut_info);
+                             ShortcutInfo* shortcut_info);

[Matt Giuca, 2017/03/01 08:49:55]

Same.

[tzik, 2017/03/01 12:34:04]

Done.

 
 // Delete all the shortcuts for an entire profile.
 // This is executed on the FILE thread.
@@ -256,6 +256,10 @@ void DeleteAllShortcutsForProfile(const base::FilePath& profile_path);
 // on-disk file name .
 base::FilePath GetSanitizedFileName(const base::string16& name);
 
+// Clears |shortcut_info| and invokes |callback| unless it's null.
+void DeleteShortcutInfoOnUIThread(std::unique_ptr<ShortcutInfo> shortcut_info,
+                                  const base::Closure& callback);
+
 }  // namespace internals
 
 }  // namespace web_app