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

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

Issue 2836363003: Retire some metrics and update file util methods for JumpList (Closed)
Patch Set: Merge branch 'master' of https://chromium.googlesource.com/chromium/src into retiresomejumplistmetrics Created 3 years, 8 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
« no previous file with comments | « chrome/browser/win/jumplist_file_util.h ('k') | chrome/browser/win/jumplist_file_util_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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
index 9dd5b11e3041070d2fad91ac470742862438e25f..78f79a4b21a2d43afa0dce8b29824276be5cb3b8 100644
--- a/chrome/browser/win/jumplist_file_util.cc
+++ b/chrome/browser/win/jumplist_file_util.cc
@@ -11,16 +11,16 @@
#include "base/metrics/histogram_macros.h"
#include "base/threading/thread_restrictions.h"
-FolderDeleteResult DeleteFiles(const base::FilePath& path,
- const base::FilePath::StringType& pattern,
- int max_file_deleted) {
+void DeleteFiles(const base::FilePath& path,
+ const base::FilePath::StringType& pattern,
+ 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.
@@ -31,119 +31,61 @@ FolderDeleteResult DeleteFiles(const base::FilePath& path,
info.find_data().dwFileAttributes & ~FILE_ATTRIBUTE_READONLY);
}
- if (info.IsDirectory()) {
- // JumpListIcons{,Old} directories shouldn't have sub-directories.
- // If any of them does for unknown reasons, don't delete them. Instead,
- // increment the failure count and record this information.
- delete_status = FAIL_SUBDIRECTORY_EXISTS;
- failure_count++;
- } else if (!::DeleteFile(current.value().c_str())) {
+ // JumpListIcons* directories shouldn't have sub-directories. If any of them
+ // does for unknown reasons, don't delete them. Instead, increment the
+ // failure count.
+ if (info.IsDirectory() || !::DeleteFile(current.value().c_str()))
failure_count++;
- } else {
+ 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.
- return failure_count ? FAIL_DELETE_MAX_FILES_WITH_ERRORS : delete_status;
- }
- if (failure_count >= max_file_deleted) {
- // The desired max number of failures have been hit.
- return FAIL_MAX_DELETE_FAILURES;
- }
+
+ // The desired max number of files have been deleted, or the desired max
+ // number of failures have been hit.
+ if (success_count >= max_file_deleted || failure_count >= max_file_deleted)
+ break;
}
- return delete_status;
}
-FolderDeleteResult DeleteDirectoryContent(const base::FilePath& path,
- int max_file_deleted) {
+void DeleteDirectoryContent(const base::FilePath& path, int max_file_deleted) {
base::ThreadRestrictions::AssertIOAllowed();
- if (path.empty())
- return SUCCEED;
-
- // For JumpListIcons{,Old} directories, since their names are shorter than
- // MAX_PATH, hitting the code in the if-block below is unexpected.
- if (path.value().length() >= MAX_PATH)
- return FAIL_INVALID_FILE_PATH;
+ if (path.empty() || path.value().length() >= MAX_PATH)
+ return;
DWORD attr = GetFileAttributes(path.value().c_str());
// We're done if we can't find the path.
if (attr == INVALID_FILE_ATTRIBUTES)
- return SUCCEED;
+ return;
// Try to clear the read-only bit if we find it.
if ((attr & FILE_ATTRIBUTE_READONLY) &&
!SetFileAttributes(path.value().c_str(),
attr & ~FILE_ATTRIBUTE_READONLY)) {
- return FAIL_READ_ONLY_DIRECTORY;
+ return;
}
- // If |path| is a file, simply delete it. However, since JumpListIcons{,Old}
- // are directories, hitting the code inside the if-block below is unexpected.
+ // If |path| is a file, simply delete it. However, since JumpListIcons* are
+ // directories, hitting the code inside the if-block below is unexpected.
if (!(attr & FILE_ATTRIBUTE_DIRECTORY)) {
::DeleteFile(path.value().c_str());
- return FAIL_DELETE_SINGLE_FILE;
+ return;
}
// If |path| is a directory, delete at most |max_file_deleted| files in it.
- return DeleteFiles(path, L"*", max_file_deleted);
+ DeleteFiles(path, L"*", max_file_deleted);
}
-FolderDeleteResult DeleteDirectory(const base::FilePath& path,
- int max_file_deleted) {
+void DeleteDirectory(const base::FilePath& path, int max_file_deleted) {
base::ThreadRestrictions::AssertIOAllowed();
- // Delete at most |max_file_deleted| files in |path|.
- FolderDeleteResult delete_status =
- DeleteDirectoryContent(path, max_file_deleted);
- // Since DeleteDirectoryContent() can only delete at most |max_file_deleted|
- // files, its return value cannot indicate if |path| is empty or not.
- // Instead, use PathIsDirectoryEmpty to check if |path| is empty and remove it
- // if it is.
- if (base::IsDirectoryEmpty(path) &&
- !::RemoveDirectory(path.value().c_str())) {
- delete_status = FAIL_REMOVE_RAW_DIRECTORY;
- }
- return delete_status;
-}
-
-void DeleteDirectoryAndLogResults(const base::FilePath& path,
- int max_file_deleted) {
- DirectoryStatus dir_status = NON_EXIST;
- if (base::DirectoryExists(path)) {
- FolderDeleteResult delete_status = DeleteDirectory(path, max_file_deleted);
- UMA_HISTOGRAM_ENUMERATION("WinJumplist.DeleteStatusJumpListIconsOld",
- delete_status, FolderDeleteResult::END);
- if (base::DirectoryExists(path))
- dir_status = base::IsDirectoryEmpty(path) ? EMPTY : NON_EMPTY;
- }
+ // Delete at most |max_file_deleted| files in |path|.
+ DeleteDirectoryContent(path, max_file_deleted);
- UMA_HISTOGRAM_ENUMERATION("WinJumplist.DirectoryStatusJumpListIconsOld",
- dir_status, DIRECTORY_STATUS_END);
+ ::RemoveDirectory(path.value().c_str());
}
-void DeleteDirectoryContentAndLogResults(const base::FilePath& path,
+void DeleteDirectoryContentAndLogRuntime(const base::FilePath& path,
int max_file_deleted) {
- // TODO(chengx): Remove the UMA histogram after fixing http://crbug.com/40407.
SCOPED_UMA_HISTOGRAM_TIMER("WinJumplist.DeleteDirectoryContentDuration");
- DirectoryStatus dir_status = NON_EXIST;
-
- // Delete the content in |path|. If |path| doesn't exist, create one.
- if (base::DirectoryExists(path)) {
- FolderDeleteResult delete_status =
- DeleteDirectoryContent(path, kFileDeleteLimit);
-
- UMA_HISTOGRAM_ENUMERATION("WinJumplist.DeleteStatusJumpListIcons",
- delete_status, FolderDeleteResult::END);
-
- if (base::DirectoryExists(path))
- dir_status = base::IsDirectoryEmpty(path) ? EMPTY : NON_EMPTY;
- } else if (base::CreateDirectory(path)) {
- dir_status = EMPTY;
- }
-
- UMA_HISTOGRAM_ENUMERATION("WinJumplist.DirectoryStatusJumpListIcons",
- dir_status, DIRECTORY_STATUS_END);
+ DeleteDirectoryContent(path, kFileDeleteLimit);
}
« no previous file with comments | « chrome/browser/win/jumplist_file_util.h ('k') | chrome/browser/win/jumplist_file_util_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698