They should get deleted when the browser stops using them, does that not happen?

(Even that might be surprising privacy-wise, I guess, but I don't know a way to avoid that).

Also what OS did you notice this on, in case it matters?

Actual deletion code is supposed to be this:
https://cs.chromium.org/chromium/src/net/disk_cache/simple/simple_synchronous_entry.cc?rcl=b64ac7c8bee2f677d0a2dffa83c9740094a0ed7f&l=1172

I noticed this on Linux when debugging an issue that only happened on network_service_browser_tests on linux_chromium_rel_ng in this CL: https://crrev.com/c/1071574
Failing Bot:
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_rel_ng/102621

The test contains three steps:
1. Visit a page on localhost
2. Delete Browsing Data
3. Check the profile directory for the string "localhost"

Chrome is restarted between each of the tests, so I wonder while the cache file would still be in use?

The test is successful on other tests but not on network_service_browser_tests. Maybe it is related to the network service changes itself but it looks like otherwise the simple cache isn't used?

Simple cache is default on Linux. And I just checked, and I can't reproduce your overall description from the UI --- but then the cache entry needs to be open when deleted to get a todelete_ file in the first place, and that's a little hard to trigger by hand.

Looking at what that test is doing; not everything data-remover works with network service yet, though the cache remover is supposed to (except for that one cache clearing method on storage partition...)

(I was mostly asking about OS since screwing up deleting an open file on Windows is quite likely).

Ah, I think I got confused about which cache backend is enabled and by the TODO, thanks.

My observation is that without --enable-features=NetworkService, almost all files from the Cache directory are removed by clearing browsing data. With NetworkService enabled, many cache files are renamed to todelete_*. I guess I should file a new bug about the network service?

I think I'd pretty much own it either way, so I'll just take a look from this report.

[119868:120085:0525/120908.266580:ERROR:simple_synchronous_entry.cc(321)] 0xb24f629e5d0 Doom has open files c1dbdb96abf11576_0
[119868:120085:0525/120908.268509:ERROR:simple_synchronous_entry.cc(1171)] 0xb24f629e5d0 CloseFile 0 todelete_c1dbdb96abf11576_0_1
[119868:120085:0525/120908.268566:ERROR:simple_synchronous_entry.cc(1176)] Tried to delete:/usr/local/google/home/morlovich/.cache/chromium/Default/Cache/todelete_c1dbdb96abf11576_0_1
[119868:120085:0525/120908.268619:ERROR:simple_synchronous_entry.cc(1178)] 1

So we try to delete the file, and base::DeleteFile return true. Yet the file is still there. Sandbox too right?

Looks like lstat returns ENOENT so base::DeleteFile thinks it doesn't have anything to do. I'll poke at it this a bit more to get a better sense for the sandbox stuff (as I think I'll need it), but will probably just ultimately ask sandbox people for help.

@dullweber: after chatting with mmenke@, probably the best thing for both you & us is to exclude your new test in network service filter (I think it may be https://cs.chromium.org/chromium/src/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter, but I am not sure), 
and land it, so you can proceed and we get documentation of this being broken w/network service that turns into coverage when it's fixed.

Oh, and so an important privacy features get an awesome end-to-end test that makes sure it's really working.

tsepez fixed the sandbox, so probably worth re-testing (well, just asked the CQ for that)

Thanks! The fix didn't stick yet but I still have to investigate some additional failures on Windows and ChromeOS anyway.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fce11fb546da34fedb212072105b58a1063addb1

commit fce11fb546da34fedb212072105b58a1063addb1
Author: Maks Orlovich <morlovich@chromium.org>
Date: Wed May 30 20:26:00 2018

S13n: Drop filter on BrowsingDataRemoverBrowserTest.StorageRemovedFromDisk w/NS

...As the fix for  bug 846800  is back in.

Bug:  846702 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ifb4ac1fe64091b7ba3f56485dfc753c8900269f9
Reviewed-on: https://chromium-review.googlesource.com/1079257
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562976}
[modify] https://crrev.com/fce11fb546da34fedb212072105b58a1063addb1/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter