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

chrome/browser/web_applications/web_app_win.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_win.h"
 
 #include <shlobj.h>
 
 #include "base/command_line.h"
 #include "base/files/file_enumerator.h"
@@ skipping 165 matching lines @@
     std::vector<base::FilePath>* out_filenames) {
   // Ensure web_app_path exists.
   if (!base::PathExists(web_app_path) &&
       !base::CreateDirectory(web_app_path)) {
     return false;
   }
 
   // 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;
   }
 
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
     NOTREACHED();
     return false;
   }
 
   // Working directory.
@@ skipping 153 matching lines @@
     return;
   }
   command_line.SetProgram(chrome_exe);
   ui::win::SetRelaunchDetailsForWindow(
       command_line.GetCommandLineString(), shortcut_info.title, hwnd);
 
   if (!base::PathExists(web_app_path) && !base::CreateDirectory(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(
     HWND hwnd,
     const web_app::ShortcutInfo& shortcut_info) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Set window's icon to the one we're about to create/update in the web app
   // path. The icon cache will refresh on icon creation.
   base::FilePath web_app_path =
@@ skipping 165 matching lines @@
     CreateShortcutsInPaths(web_app_dir,
                            shortcut_info,
                            paths,
                            SHORTCUT_CREATION_BY_USER,
                            &out_filenames);
     DCHECK_EQ(out_filenames.size(), 1u);
     DCHECK_EQ(out_filenames[0].value(), web_app_dir_shortcut.value());
   } else {
     internals::CheckAndSaveIcon(
         internals::GetIconFilePath(web_app_dir, shortcut_info.title),
-        shortcut_info.favicon);
+        shortcut_info.favicon, false);
   }
   return web_app_dir_shortcut;
 }
 
 void UpdateRelaunchDetailsForApp(Profile* profile,
                                  const extensions::Extension* extension,
                                  HWND hwnd) {
   web_app::GetShortcutInfoForApp(
       extension,
       profile,
       base::Bind(&OnShortcutInfoLoadedForSetRelaunchDetails, hwnd));
 }
 
 void UpdateShortcutsForAllApps(Profile* profile,
                                const base::Closure& callback) {
   callback.Run();
 }
 
 namespace internals {
 
 // Saves |image| to |icon_file| if the file is outdated and refresh shell's
 // 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;
 }
 
 bool CreatePlatformShortcuts(
     const base::FilePath& web_app_path,
     const ShortcutInfo& shortcut_info,
     const extensions::FileHandlersInfo& file_handlers_info,
     const ShortcutLocations& creation_locations,
     ShortcutCreationReason creation_reason) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
@@ skipping 73 matching lines @@
       // Use the web app path shortcut for pinning to avoid having unique
       // numbers in the application name.
       base::FilePath shortcut_to_pin = web_app_path.Append(file_name).
           AddExtension(installer::kLnkExt);
       base::win::TaskbarPinShortcutLink(shortcut_to_pin.value().c_str());
     }
   }
 
   // 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,
                              const ShortcutInfo& shortcut_info) {
   GetShortcutLocationsAndDeleteShortcuts(
       web_app_path, shortcut_info.profile_path, shortcut_info.title, NULL,
       NULL);
 
   // If there are no more shortcuts in the Chrome Apps subdirectory, remove it.
   base::FilePath chrome_apps_dir;
@@ skipping 82 matching lines @@
 
 }  // namespace internals
 
 void UpdateShortcutForTabContents(content::WebContents* web_contents) {
   // UpdateShortcutWorker will delete itself when it's done.
   UpdateShortcutWorker* worker = new UpdateShortcutWorker(web_contents);
   worker->Run();
 }
 
 }  // namespace web_app