Do not do unnecessary recursive deletes for files.

Do not do unnecessary recursive deletes for files.

Committed: https://crrev.com/16a4bcaea29984b40fbeb934801b1c2b6648bc73
Cr-Commit-Position: refs/heads/master@{#363419}

[Lei Zhang, 2015-11-25 21:38:20]

thestig@chromium.org changed reviewers:
+ megjablon@chromium.org, tzik@chromium.org, zea@chromium.org

[Lei Zhang, 2015-11-25 21:38:21]

megjablon: chrome/ and components/
tzik: storage/
zea: sync/

[tzik, 2015-11-26 04:48:15]

lgtm

[megjablon, 2015-11-27 17:56:15]

megjablon@chromium.org changed reviewers:
+ kundaji@chromium.org

[megjablon, 2015-11-27 17:56:16]

Deferring to kundaji@ who set up our leveldb

[Not at Google. Contact bengr, 2015-11-30 19:50:25]

https://chromiumcodereview.appspot.com/1471073008/diff/1/components/data_redu...
File components/data_reduction_proxy/core/browser/data_store_impl.cc (right):

https://chromiumcodereview.appspot.com/1471073008/diff/1/components/data_redu...
components/data_reduction_proxy/core/browser/data_store_impl.cc:150:
base::DeleteFile(profile_path_.Append(kDBName), false);
It seems to me that the recursive delete is necessary here.

Given that the LevelDB is corrupt, we should delete all the files. This is
expected to happen very rarely, so not sure there is any observable benefit to
making this non-recursive.

The same should be true of all cases in this cl which are deleting a corrupt
LevelDB.

[Nicolas Zea, 2015-11-30 21:27:50]

sync lgtm

[Lei Zhang, 2015-12-03 21:39:07]

kundaji/megjablon: Still need someone to sanity check the remaining chrome/
changes, if you don't mind.

https://codereview.chromium.org/1471073008/diff/1/components/data_reduction_p...
File components/data_reduction_proxy/core/browser/data_store_impl.cc (right):

https://codereview.chromium.org/1471073008/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_store_impl.cc:150:
base::DeleteFile(profile_path_.Append(kDBName), false);
On 2015/11/30 19:50:24, kundaji wrote:
> It seems to me that the recursive delete is necessary here.
> 
> Given that the LevelDB is corrupt, we should delete all the files. This is
> expected to happen very rarely, so not sure there is any observable benefit to
> making this non-recursive.
> 
> The same should be true of all cases in this cl which are deleting a corrupt
> LevelDB.

Ah, I see, LevelDB takes a directory and not a file. Reverted.

[Not at Google. Contact bengr, 2015-12-04 22:01:26]

kundaji@chromium.org changed reviewers:
+ zelidrag@chromium.org

[Not at Google. Contact bengr, 2015-12-04 22:05:37]

lgtm

Adding zei@ who wrote chrome/browser/chromeos/system_logs/debug_log_writer.cc.

Remaining files in chrome/ are tests, so LGTM.
chrome/browser/download/download_browsertest.cc
chrome/browser/profiles/profile_info_cache_unittest.cc
chrome/installer/util/copy_tree_work_item_unittest.cc

[Lei Zhang, 2015-12-05 02:05:26]

On 2015/12/04 22:05:37, kundaji wrote:
> lgtm
> 
> Adding zei@ who wrote chrome/browser/chromeos/system_logs/debug_log_writer.cc.

s/zei/zel/

but looking at the code:

|tar_file_path| is created with IntializeLogFile(), which calls
base::File::Initialize(), rather than base::CreateDirectory(). So it's a file.

|compressed_output_path| is initialized as
tar_file_path.AddExtension(FILE_PATH_LITERAL(".gz")) ... one would hope that's a
file.

[Lei Zhang, 2015-12-05 02:05:39]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2015-12-05 02:05:39]

The patchset sent to the CQ was uploaded after l-g-t-m from zea@chromium.org,
tzik@chromium.org
Link to the patchset: https://codereview.chromium.org/1471073008/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2015-12-05 02:08:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1471073008/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1471073008/40001

[commit-bot: I haz the power, 2015-12-05 04:13:31]

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

[commit-bot: I haz the power, 2015-12-05 04:13:31]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)

[Lei Zhang, 2015-12-07 07:25:54]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2015-12-07 07:26:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1471073008/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1471073008/40001

[commit-bot: I haz the power, 2015-12-07 08:36:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-07 08:37:30]

Description was changed from

==========
Do not do unnecessary recursive deletes for files.
==========

to

==========
Do not do unnecessary recursive deletes for files.

Committed: https://crrev.com/16a4bcaea29984b40fbeb934801b1c2b6648bc73
Cr-Commit-Position: refs/heads/master@{#363419}
==========

[commit-bot: I haz the power, 2015-12-07 08:37:32]

Patchset 3 (id:??) landed as
https://crrev.com/16a4bcaea29984b40fbeb934801b1c2b6648bc73
Cr-Commit-Position: refs/heads/master@{#363419}