Don't send a SHChangeNotify for creating an app icon when creating a shortcut.

chrome/browser/web_applications/web_app_win.cc

Index: chrome/browser/web_applications/web_app_win.cc
diff --git a/chrome/browser/web_applications/web_app_win.cc b/chrome/browser/web_applications/web_app_win.cc
index 10bad7198f3101659a5f1bd4bf09e1e40b068141..7968e16a944eab9f9d94f46ad4eaadc0bcf2d5d2 100644
--- a/chrome/browser/web_applications/web_app_win.cc
+++ b/chrome/browser/web_applications/web_app_win.cc
@@ -183,7 +183,8 @@ bool CreateShortcutsInPaths(
   // Generates file name to use with persisted ico and shortcut file.
   base::FilePath icon_file =
       web_app::internals::GetIconFilePath(web_app_path, shortcut_info.title);
-  if (!web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon)) {
+  if (!web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon,
+                                            true)) {
     return false;
   }
 
@@ -357,7 +358,7 @@ void CreateIconAndSetRelaunchDetails(const base::FilePath& web_app_path,
     return;
 
   ui::win::SetAppIconForWindow(icon_file.value(), hwnd);
-  web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon);
+  web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon, false);
 }
 
 void OnShortcutInfoLoadedForSetRelaunchDetails(
@@ -543,7 +544,7 @@ base::FilePath CreateShortcutInWebAppDir(const base::FilePath& web_app_dir,
   } else {
     internals::CheckAndSaveIcon(
         internals::GetIconFilePath(web_app_dir, shortcut_info.title),
-        shortcut_info.favicon);
+        shortcut_info.favicon, false);
   }
   return web_app_dir_shortcut;
 }
@@ -568,19 +569,23 @@ namespace internals {
 // icon cache to ensure correct icon is displayed. Returns true if icon_file
 // is up to date or successfully updated.
 bool CheckAndSaveIcon(const base::FilePath& icon_file,
-                      const gfx::ImageFamily& image) {
-  if (ShouldUpdateIcon(icon_file, image)) {
-    if (SaveIconWithCheckSum(icon_file, image)) {
-      // Refresh shell's icon cache. This call is quite disruptive as user would
-      // see explorer rebuilding the icon cache. It would be great that we find
-      // a better way to achieve this.
-      SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST | SHCNF_FLUSHNOWAIT,
-                     NULL, NULL);
-    } else {
-      return false;
-    }
-  }
+                      const gfx::ImageFamily& image,
+                      bool is_shortcut_creation) {
+  if (!ShouldUpdateIcon(icon_file, image))
+    return true;
+
+  if (!SaveIconWithCheckSum(icon_file, image))
+    return false;
+
+  // Shortcut creation doesn't require a SHChangeNotify as the icon will be

[Matt Giuca, 2015/03/25 03:31:18]

will be ...?

(I hate cliffhangers.)

[calamity, 2015/03/25 04:37:23]

Moved to header comment.

+  if (is_shortcut_creation)
+    return true;
 
+  // Refresh shell's icon cache. This call is quite disruptive as user would
+  // see explorer rebuilding the icon cache. It would be great that we find
+  // a better way to achieve this.
+  SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST | SHCNF_FLUSHNOWAIT, NULL,
+                 NULL);

[calamity, 2015/03/24 07:35:08]

This logic should be identical to the old code except for the
is_shortcut_creation part.

[Matt Giuca, 2015/03/25 03:31:18]

I like the refactoring you did here, but I think this last part should be inside
an if statement:

if (!is_shortcut_creation) {
  SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST | SHCNF_FLUSHNOWAIT, NULL,
                 NULL);
}

Because the return value is the same anyway; I prefer not to think of this as an
early exit, but conditional execution of the SHChangeNotify.

[calamity, 2015/03/25 04:37:23]

Done.

   return true;
 }
 
@@ -674,7 +679,7 @@ void UpdatePlatformShortcuts(
 
   // Update the icon if necessary.
   base::FilePath icon_file = GetIconFilePath(web_app_path, shortcut_info.title);
-  CheckAndSaveIcon(icon_file, shortcut_info.favicon);
+  CheckAndSaveIcon(icon_file, shortcut_info.favicon, false);
 }
 
 void DeletePlatformShortcuts(const base::FilePath& web_app_path,