Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1917)

Unified Diff: chrome/browser/win/jumplist_file_util.cc

Issue 2752063002: Remove JumpListIconsOld directory and set upper limit for delete attempts (Closed)
Patch Set: Address feedback comments. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/win/jumplist_file_util.cc
diff --git a/chrome/browser/win/jumplist_file_util.cc b/chrome/browser/win/jumplist_file_util.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ebbe8389398a8ad2e1cc324428023d5462079014
--- /dev/null
+++ b/chrome/browser/win/jumplist_file_util.cc
@@ -0,0 +1,120 @@
+// Copyright (c) 2017 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/win/jumplist_file_util.h"
+
+#include <Shlwapi.h>
+#include <windows.h>
+
+#include "base/files/file_enumerator.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/threading/thread_restrictions.h"
+
+const FolderDeleteResult DeleteFiles(const base::FilePath& path,
+ const base::FilePath::StringType& pattern,
+ const int max_file_deleted) {
+ int success_count = 0;
+ int failure_count = 0;
+ FolderDeleteResult delete_status = SUCCEED;
+
+ base::FileEnumerator traversal(
+ path, false,
+ base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES, pattern);
+ for (base::FilePath current = traversal.Next(); !current.empty();
+ current = traversal.Next()) {
+ // Try to clear the read-only bit if we find it.
+ base::FileEnumerator::FileInfo info = traversal.GetInfo();
+ if (info.find_data().dwFileAttributes & FILE_ATTRIBUTE_READONLY) {
+ SetFileAttributes(
+ current.value().c_str(),
+ info.find_data().dwFileAttributes & ~FILE_ATTRIBUTE_READONLY);
+ }
+
+ if (info.IsDirectory()) {
+ // JumpListIcons{,Old} directories shouldn't have sub-directories.
+ // If it does for unknown reasons, don't try to delete it. Instead,
+ // increment failure count and record this information in |delete_status|.
+ delete_status = FAIL_SUBDIRECTORY_EXISTS;
+ failure_count++;
+ } else if (!::DeleteFile(current.value().c_str())) {
+ failure_count++;
+ } else {
+ success_count++;
+ }
+ // If it deletes max_file_deleted files with any attempt failures, record
+ // this information in |delete_status|.
+ if (success_count >= max_file_deleted) {
+ // The desired max number of files have been deleted.
+ if (failure_count)
+ delete_status = FAIL_DELETE_MAX_FILES_WITH_ERRORS;
+ return delete_status;
+ }
+ if (failure_count >= max_file_deleted) {
+ // The desired max number of failures have been hit.
+ return FAIL_MAX_DELETE_FAILURES;
+ }
+ }
+ return delete_status;
+}
+
+const FolderDeleteResult DeleteDirectoryContent(const base::FilePath& path,
+ const int max_file_deleted) {
+ base::ThreadRestrictions::AssertIOAllowed();
+
+ FolderDeleteResult delete_status = SUCCEED;
+
+ if (path.empty())
+ return delete_status;
+
+ // For JumpListIcons{,Old} directories, since their names are shorter than
+ // MAX_PATH, hitting the code block below is unexpected.
+ if (path.value().length() >= MAX_PATH) {
+ return FAIL_INVALID_FILE_PATH;
+ }
+
+ DWORD attr = GetFileAttributes(path.value().c_str());
+ // We're done if we can't find the path.
+ if (attr == INVALID_FILE_ATTRIBUTES)
+ return delete_status;
+ // We may need to clear the read-only bit.
+ if ((attr & FILE_ATTRIBUTE_READONLY) &&
+ !SetFileAttributes(path.value().c_str(),
+ attr & ~FILE_ATTRIBUTE_READONLY)) {
+ return FAIL_READ_ONLY_DIRECTORY;
+ }
+
+ // If |path| is a file, simply delete it.
+ // Since JumpListIcons{,Old} are directories, hitting the code block below is
+ // unexpected.
+ if (!(attr & FILE_ATTRIBUTE_DIRECTORY)) {
+ ::DeleteFile(path.value().c_str());
+ return FAIL_DELETE_SINGLE_FILE;
+ }
+
+ // If |path| is a directory, delete all its contents but save the raw
+ // directory.
+ return DeleteFiles(path, L"*", max_file_deleted);
+}
+
+const FolderDeleteResult DeleteDirectory(const base::FilePath& path,
+ const int max_file_deleted) {
+ base::ThreadRestrictions::AssertIOAllowed();
+ // Delete the contents in the directory first.
+ FolderDeleteResult delete_status =
+ DeleteDirectoryContent(path, max_file_deleted);
+ // Since DeleteDirectoryContent can only delete maximum files allowed, its
+ // return cannot be used to indicate if the directory is empty now. Therefore,
+ // check if the directory is empty using ::PathIsDirectoryEmpty directly.
+ if (::PathIsDirectoryEmpty(path.value().c_str()) &&
+ !::RemoveDirectory(path.value().c_str()))
+ delete_status = FAIL_REMOVE_RAW_DIRECTORY;
+ return delete_status;
+}
+
+void DeleteDirectoryAndLogResults(const base::FilePath& path,
+ const int max_file_deleted) {
+ FolderDeleteResult delete_status = DeleteDirectory(path, max_file_deleted);
+ UMA_HISTOGRAM_ENUMERATION("WinJumplist.DeleteStatusJumpListIconsOld",
+ delete_status, END);
Ilya Sherman 2017/03/22 22:24:00 FWIW, you might want to add histogram test coverag
chengx 2017/03/23 21:16:11 I would probably not add histogram test for this i
+}

Powered by Google App Engine
This is Rietveld 408576698