Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2836363003:
Retire some metrics and update file util methods for JumpList (Closed)
DescriptionRetire some metrics and update file util methods for JumpList
This CL retires some file deletion related metrics for JumpList since
they're no longer needed.
Accordingly, the JumpList file deletion related methods are updated to
return nothing instead of the detailed delete status which is used for
the retiring metrics. This makes the ENUMs for the delete status no
long needed so they're removed. These methods are also refactored a
little bit to make them cleaner.
BUG=40407, 179576
Review-Url: https://codereview.chromium.org/2836363003
Cr-Commit-Position: refs/heads/master@{#468425}
Committed: https://chromium.googlesource.com/chromium/src/+/4fffeff4e2367820ccddaa23a9800aa97d887d51
Patch Set 1 #Patch Set 2 : Update unittest #Patch Set 3 : Git pull and resolve conflicts #Patch Set 4 : Refactor code #
Total comments: 20
Patch Set 5 : Address feedback comments, update comments #
Total comments: 6
Patch Set 6 : Update/revert code comments #Patch Set 7 : Merge branch 'master' of https://chromium.googlesource.com/chromium/src into retiresomejumplistmetrics #
Messages
Total messages: 69 (57 generated)
Description was changed from ========== Merge branch 'master' of https://chromium.googlesource.com/chromium/src into retiresomejumplistmetrics Retire metrics BUG= ========== to ========== Update jumplist_file_util* BUG= ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Description was changed from ========== Update jumplist_file_util* BUG= ========== to ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status. This makes the ENUMs for the delete status no long needed therefore they're removed. The methods are also refactored a little bit due to the change above. BUG=40407, 179576 ==========
Description was changed from ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status. This makes the ENUMs for the delete status no long needed therefore they're removed. The methods are also refactored a little bit due to the change above. BUG=40407, 179576 ========== to ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed therefore they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ==========
chengx@chromium.org changed reviewers: + gab@chromium.org, isherman@chromium.org
This CL doesn't change any JumpList functionality but simply retires some metrics. The methods are updated accordingly. PTAL, thanks!
Description was changed from ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed therefore they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ========== to ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which's used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ==========
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
histograms.xml lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi gab@, gentle ping now for the day as you're 3 hours ahead of me. Thanks!
Description was changed from ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which's used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ========== to ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ==========
chengx@chromium.org changed reviewers: + pmonette@chromium.org
Adding pmonette@ Hi Patrick, can you please take a look of this CL which is basically code cleaning? I created jumplist_file_util.* from scratch, so I know them well.
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Patchset #4 (id:120001) has been deleted
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
chengx@chromium.org changed reviewers: + grt@chromium.org - pmonette@chromium.org
PTAL
https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:126: // Helper method for RunUpdateJumpList to create icon files for the please follow the function comment style guidelines here and elsewhere (https://google.github.io/styleguide/cppguide.html#Function_Comments). for example "Creates icon files for the ..." https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:150: if (!base::DirectoryExists(icon_dir)) base::CreateDirectory returns true if the directory already exists, so i believe this logic here can be simplified to: if (base::CreateDirectory(icon_dir) && base::IsDirectoryEmpty(icon_dir)) CreateIconFiles(icon_dir, page_list, slot_limit); https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:155: // updated later on anyway, as it doesn't involve disk IO. In this case, regarding this comment, are you saying that calling the shell functions to update the jumplist doesn't do disk io? have you verified this? based on your timings indicating that some of the shell functions can take a very long time, wouldn't you expect that the reason is due to disk io? https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:397: icon_dir_ = profile_->GetPath().Append(chrome::kJumpListIconDirname); this directory isn't used anymore, right? i think it makes more sense to cache the profile path in the jumplist instance and then append chrome::kJumpListIconDirname + kFoo as needed at the various points of use rather than the current chopping and adding. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist_file_util.cc (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:36: if (info.IsDirectory() || !::DeleteFile(current.value().c_str())) { nit: omit braces https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:45: return; nit: i slightly prefer breaking out of the loop here so that there's only one exit path to the function. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:87: if (base::IsDirectoryEmpty(path)) if you no longer care whether RemoveDirectory succeeds or fails, remove the call to IsDirectoryEmpty. RD won't do anything if the directory isn't empty. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist_file_util.h (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.h:36: // Delete the content in the directory at |path| and records the runtime to UMA. Delete -> Deletes https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.h:37: // DeleteDirectoryContentAndLogRuntime is explicited called for certain folders' this sentence documents the callers rather than this function. i don't think it belongs here.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
gab@chromium.org changed reviewers: - gab@chromium.org
PTAL~ https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:126: // Helper method for RunUpdateJumpList to create icon files for the On 2017/04/28 07:37:07, grt (UTC plus 2) wrote: > please follow the function comment style guidelines here and elsewhere > (https://google.github.io/styleguide/cppguide.html#Function_Comments). for > example "Creates icon files for the ..." Done. I've updated the comments throughout the file. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:150: if (!base::DirectoryExists(icon_dir)) On 2017/04/28 07:37:07, grt (UTC plus 2) wrote: > base::CreateDirectory returns true if the directory already exists, so i believe > this logic here can be simplified to: > > if (base::CreateDirectory(icon_dir) && base::IsDirectoryEmpty(icon_dir)) > CreateIconFiles(icon_dir, page_list, slot_limit); Done. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:155: // updated later on anyway, as it doesn't involve disk IO. In this case, On 2017/04/28 07:37:07, grt (UTC plus 2) wrote: > regarding this comment, are you saying that calling the shell functions to > update the jumplist doesn't do disk io? have you verified this? based on your > timings indicating that some of the shell functions can take a very long time, > wouldn't you expect that the reason is due to disk io? No, I didn't mean that. Calling the shell functions to update the jumplist involves disk IO I think although I am not 100% sure. I am saying we don't need to write these jumplist links into the JumpListIcons folder as what we do to the icons. I've removed this comment to avoid confusion. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:397: icon_dir_ = profile_->GetPath().Append(chrome::kJumpListIconDirname); On 2017/04/28 07:37:07, grt (UTC plus 2) wrote: > this directory isn't used anymore, right? i think it makes more sense to cache > the profile path in the jumplist instance and then append > chrome::kJumpListIconDirname + kFoo as needed at the various points of use > rather than the current chopping and adding. Agreed. I'll use another small CL for this as it doesn't well fit this CL. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist_file_util.cc (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:36: if (info.IsDirectory() || !::DeleteFile(current.value().c_str())) { On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > nit: omit braces Done. However, I think braces are allowed here. https://google.github.io/styleguide/cppguide.html#Conditionals https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:45: return; On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > nit: i slightly prefer breaking out of the loop here so that there's only one > exit path to the function. Done. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:87: if (base::IsDirectoryEmpty(path)) On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > if you no longer care whether RemoveDirectory succeeds or fails, remove the call > to IsDirectoryEmpty. RD won't do anything if the directory isn't empty. Done. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist_file_util.h (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.h:36: // Delete the content in the directory at |path| and records the runtime to UMA. On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > Delete -> Deletes Done. https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.h:37: // DeleteDirectoryContentAndLogRuntime is explicited called for certain folders' On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > this sentence documents the callers rather than this function. i don't think it > belongs here. Agreed. I added this comment to avoid confusion about why this method is needed. Anyway, I've deleted this sentence.
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Retire some metrics and update file operation methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ========== to ========== Retire some metrics, update file operation methods, update comments for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. This CL also updates all the comments according to the Google C++ Style Guide. BUG=40407, 179576 ==========
Description was changed from ========== Retire some metrics, update file operation methods, update comments for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. This CL also updates all the comments according to the Google C++ Style Guide. BUG=40407, 179576 ========== to ========== Retire metrics, update file IO methods, update comments for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. This CL also updates all the comments according to the Google C++ Style Guide. BUG=40407, 179576 ==========
Description was changed from ========== Retire metrics, update file IO methods, update comments for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. This CL also updates all the comments according to the Google C++ Style Guide. BUG=40407, 179576 ========== to ========== Retire metrics, update file util methods, update comments for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. This CL also updates all the comments according to the Google C++ Style Guide. BUG=40407, 179576 ==========
https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist_file_util.cc (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:36: if (info.IsDirectory() || !::DeleteFile(current.value().c_str())) { On 2017/04/28 22:29:29, chengx wrote: > On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > > nit: omit braces > > Done. However, I think braces are allowed here. > https://google.github.io/styleguide/cppguide.html#Conditionals you're correct. one of the more important goals stated in the style guide is to be consistent with surrounding code (https://google.github.io/styleguide/cppguide.html#Goals). in general, the windows code in chromium (and possible much of chromium) omits braces when the conditional and its body are each one line. a quick survey of this directory shows that this holds here. there may be other parts of the codebase that always include braces, so you may need to include them when working there. https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:67: // Delays jumplist updates to allow collapsing of redundant update requests. the "These comments should be descriptive" clause applies to function comments. this constant doesn't delay anything on its own. i think a better style for this would be to describe what the constant is used for. maybe: // The delay before updating the JumpList to prevent update storms. https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:71: // Timeouts jumplist updates for users with extremely slow OS. // The maximum allowed time for JumpListUpdater::BeginUpdate. Updates taking longer than this are discarded to prevent bogging down slow machines. https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:96: // Retrieves the path to a temporary file. please revert this blanket comment change. as mentioned above, the remark was specifically about function comments. if you wish to update other comments in this file, please do so in another CL after re-reading the Comments section of the style guide (https://google.github.io/styleguide/cppguide.html#Comments).
Patchset #6 (id:180001) has been deleted
Patchset #6 (id:200001) has been deleted
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Retire metrics, update file util methods, update comments for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. This CL also updates all the comments according to the Google C++ Style Guide. BUG=40407, 179576 ========== to ========== Retire some metrics and update file util methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ==========
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #6 (id:210006) has been deleted
I've addressed all the feedback comments in the new patch set. PTAL, thanks! https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... File chrome/browser/win/jumplist_file_util.cc (right): https://codereview.chromium.org/2836363003/diff/140001/chrome/browser/win/jum... chrome/browser/win/jumplist_file_util.cc:36: if (info.IsDirectory() || !::DeleteFile(current.value().c_str())) { On 2017/05/01 09:45:36, grt (UTC plus 2) wrote: > On 2017/04/28 22:29:29, chengx wrote: > > On 2017/04/28 07:37:08, grt (UTC plus 2) wrote: > > > nit: omit braces > > > > Done. However, I think braces are allowed here. > > https://google.github.io/styleguide/cppguide.html#Conditionals > > you're correct. one of the more important goals stated in the style guide is to > be consistent with surrounding code > (https://google.github.io/styleguide/cppguide.html#Goals). in general, the > windows code in chromium (and possible much of chromium) omits braces when the > conditional and its body are each one line. a quick survey of this directory > shows that this holds here. there may be other parts of the codebase that always > include braces, so you may need to include them when working there. Cool. Thanks for this reminder! https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:67: // Delays jumplist updates to allow collapsing of redundant update requests. On 2017/05/01 09:45:36, grt (UTC plus 2) wrote: > the "These comments should be descriptive" clause applies to function comments. > this constant doesn't delay anything on its own. i think a better style for this > would be to describe what the constant is used for. maybe: > > // The delay before updating the JumpList to prevent update storms. Done. https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:71: // Timeouts jumplist updates for users with extremely slow OS. On 2017/05/01 09:45:36, grt (UTC plus 2) wrote: > // The maximum allowed time for JumpListUpdater::BeginUpdate. Updates taking > longer than this are discarded to prevent bogging down slow machines. Done. https://codereview.chromium.org/2836363003/diff/160001/chrome/browser/win/jum... chrome/browser/win/jumplist.cc:96: // Retrieves the path to a temporary file. On 2017/05/01 09:45:36, grt (UTC plus 2) wrote: > please revert this blanket comment change. as mentioned above, the remark was > specifically about function comments. if you wish to update other comments in > this file, please do so in another CL after re-reading the Comments section of > the style guide (https://google.github.io/styleguide/cppguide.html#Comments). Done. I've reverted all those bad changes.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
lgtm, thanks.
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by chengx@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by chengx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from isherman@chromium.org, grt@chromium.org Link to the patchset: https://codereview.chromium.org/2836363003/#ps250001 (title: "Merge branch 'master' of https://chromium.googlesource.com/chromium/src into retiresomejumplistmetrics")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 250001, "attempt_start_ts": 1493674506039280,
"parent_rev": "45aeaf16b7bcce4ea13a322b3960ff1f4f3602b0", "commit_rev":
"4fffeff4e2367820ccddaa23a9800aa97d887d51"}
Message was sent while issue was closed.
Description was changed from ========== Retire some metrics and update file util methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 ========== to ========== Retire some metrics and update file util methods for JumpList This CL retires some file deletion related metrics for JumpList since they're no longer needed. Accordingly, the JumpList file deletion related methods are updated to return nothing instead of the detailed delete status which is used for the retiring metrics. This makes the ENUMs for the delete status no long needed so they're removed. These methods are also refactored a little bit to make them cleaner. BUG=40407, 179576 Review-Url: https://codereview.chromium.org/2836363003 Cr-Commit-Position: refs/heads/master@{#468425} Committed: https://chromium.googlesource.com/chromium/src/+/4fffeff4e2367820ccddaa23a980... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:250001) as https://chromium.googlesource.com/chromium/src/+/4fffeff4e2367820ccddaa23a980... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
