Prevent SHChangeNotify on app list drag on Windows.

Prevent SHChangeNotify on app list drag on Windows.

This CL fixes an issue with all taskbar icons flashing on every app list
drag due to a SHChangeNotify being fired. This was being caused an overwrite
of the web app path shortcut which is created for drag and drop to the taskbar.
The fix skips the shortcut creation if it already exists (but still updates the icon).

This CL also extracts a GetIconFilePath function.

BUG=356961
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260439

[benwells, 2014-03-27 01:53:22]

lgtm

https://codereview.chromium.org/213833005/diff/20001/chrome/browser/web_appli...
File chrome/browser/web_applications/web_app_win.cc (right):

https://codereview.chromium.org/213833005/diff/20001/chrome/browser/web_appli...
chrome/browser/web_applications/web_app_win.cc:348:
web_app::internals::CheckAndSaveIcon(
Nit: you don't need web_app:: here.

https://codereview.chromium.org/213833005/diff/20001/chrome/browser/web_appli...
chrome/browser/web_applications/web_app_win.cc:457:
web_app::internals::GetIconFilePath(web_app_path, shortcut_info.title);
Nit: you don't need the namespace here as you're in web_app::internals

[calamity, 2014-03-27 13:16:24]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-03-27 13:16:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/calamity@chromium.org/213833005/40001

[commit-bot: I haz the power, 2014-03-27 13:16:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 13:16:42]

Failed to apply patch for chrome/browser/web_applications/web_app_win.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/web_applications/web_app_win.cc
  Hunk #4 succeeded at 333 (offset 1 line).
  Hunk #5 succeeded at 403 (offset 1 line).
  Hunk #6 succeeded at 440 (offset 1 line).
  Hunk #7 FAILED at 450.
  Hunk #8 succeeded at 541 (offset -2 lines).
  1 out of 8 hunks FAILED -- saving rejects to file
chrome/browser/web_applications/web_app_win.cc.rej

Patch:       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
1cf48002624b3cc6c5c7e390b71f94e65e31f88e..0fe48e9fc6265be46347626f75139cb2f0b18bdf
100644
--- a/chrome/browser/web_applications/web_app_win.cc
+++ b/chrome/browser/web_applications/web_app_win.cc
@@ -32,8 +32,6 @@ namespace {
 const base::FilePath::CharType kIconChecksumFileExt[] =
     FILE_PATH_LITERAL(".ico.md5");
 
-// Width and height of icons exported to .ico files.
-
 // Calculates checksum of an icon family using MD5.
 // The checksum is derived from all of the icons in the family.
 void GetImageCheckSum(const gfx::ImageFamily& image, base::MD5Digest* digest) {
@@ -173,12 +171,8 @@ bool CreateShortcutsInPaths(
   }
 
   // Generates file name to use with persisted ico and shortcut file.
-  base::FilePath file_name =
-      web_app::internals::GetSanitizedFileName(shortcut_info.title);
-
-  // Creates an ico file to use with shortcut.
-  base::FilePath icon_file = web_app_path.Append(file_name).AddExtension(
-      FILE_PATH_LITERAL(".ico"));
+  base::FilePath icon_file =
+      web_app::internals::GetIconFilePath(web_app_path, shortcut_info.title);
   if (!web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon))
{
     return false;
   }
@@ -214,8 +208,11 @@ bool CreateShortcutsInPaths(
 
   bool success = true;
   for (size_t i = 0; i < shortcut_paths.size(); ++i) {
-    base::FilePath shortcut_file = shortcut_paths[i].Append(file_name).
-        AddExtension(installer::kLnkExt);
+    base::FilePath shortcut_file =
+        shortcut_paths[i]
+            .Append(
+                 web_app::internals::GetSanitizedFileName(shortcut_info.title))
+            .AddExtension(installer::kLnkExt);
     if (creation_reason == web_app::SHORTCUT_CREATION_AUTOMATED) {
       // Check whether there is an existing shortcut to this app.
       std::vector<base::FilePath> shortcut_files =
@@ -335,10 +332,23 @@ base::FilePath CreateShortcutInWebAppDir(
   std::vector<base::FilePath> paths;
   paths.push_back(web_app_dir);
   std::vector<base::FilePath> out_filenames;
-  CreateShortcutsInPaths(web_app_dir, shortcut_info, paths,
-                         SHORTCUT_CREATION_BY_USER, &out_filenames);
-  DCHECK_EQ(out_filenames.size(), 1u);
-  return out_filenames[0];
+  base::FilePath web_app_dir_shortcut =
+      web_app_dir.Append(internals::GetSanitizedFileName(shortcut_info.title))
+          .AddExtension(installer::kLnkExt);
+  if (!PathExists(web_app_dir_shortcut)) {
+    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);
+  }
+  return web_app_dir_shortcut;
 }
 
 namespace internals {
@@ -392,8 +402,7 @@ bool CreatePlatformShortcuts(
     return false;
 
   if (pin_to_taskbar) {
-    base::FilePath file_name =
-        web_app::internals::GetSanitizedFileName(shortcut_info.title);
+    base::FilePath file_name = GetSanitizedFileName(shortcut_info.title);
     // 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).
@@ -430,8 +439,7 @@ void UpdatePlatformShortcuts(
     // GetShortcutLocationsAndDeleteShortcuts will have deleted it. In that
     // case, re-pin it.
     if (was_pinned_to_taskbar) {
-      base::FilePath file_name =
-          web_app::internals::GetSanitizedFileName(shortcut_info.title);
+      base::FilePath file_name = GetSanitizedFileName(shortcut_info.title);
       // 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).
@@ -442,10 +450,9 @@ void UpdatePlatformShortcuts(
 
   // If an icon file exists, and is out of date, replace it with the new icon
   // and let the shell know the icon has been modified.
-  base::FilePath icon_file = web_app_path.Append(file_name).AddExtension(
-      FILE_PATH_LITERAL(".ico"));
+  base::FilePath icon_file = GetIconFilePath(web_app_path,
shortcut_info.title);
   if (base::PathExists(icon_file)) {
-    web_app::internals::CheckAndSaveIcon(icon_file, shortcut_info.favicon);
+    CheckAndSaveIcon(icon_file, shortcut_info.favicon);
   }
 }
 
@@ -535,6 +542,12 @@ std::vector<base::FilePath> GetShortcutPaths(
   return shortcut_paths;
 }
 
+base::FilePath GetIconFilePath(const base::FilePath& web_app_path,
+                               const base::string16& title) {
+  return web_app_path.Append(GetSanitizedFileName(title))
+      .AddExtension(FILE_PATH_LITERAL(".ico"));
+}
+
 }  // namespace internals
 
 }  // namespace web_app

[calamity, 2014-03-28 00:14:29]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2014-03-28 00:16:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/calamity@chromium.org/213833005/60001

[cmp, 2014-03-30 16:35:35]

The CQ bit was unchecked by cmp@chromium.org

[cmp, 2014-03-30 16:37:43]

The CQ bit was checked by cmp@chromium.org

[commit-bot: I haz the power, 2014-03-30 16:37:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/calamity@chromium.org/213833005/60001

[commit-bot: I haz the power, 2014-03-30 17:10:41]

Change committed as 260439