Partial stack from adding a DCHECK

#3 storage::QuotaManagerProxy::NotifyStorageModified()
#4 storage::SandboxFileSystemBackendDelegate::DeleteOriginDataOnFileTaskRunner()
#5 storage::FileSystemContext::DeleteDataForOriginOnFileTaskRunner()
#6 (anonymous namespace)::BrowsingDataFileSystemHelperImpl::DeleteFileSystemOriginInFileThread()

Yeah, the FileSystem code and QuotaManager code don't play nicely together:

* FileSystemContext::DeleteDataForOriginOnFileTaskRunner takes an origin, and loops over all storage types, and tries to delete (origin, type) pair

* SandboxFileSystemBackendDelegate::DeleteOriginDataOnFileTaskRunner tries to delete the directory if present, but regardless calls QuotaManagerProxy::NotifyStorageModified() with the (origin, type) pair.

So two possibilities: 

* Make DeleteOriginDataOnFileTaskRunner not call NotifyStorageModified if the directory wasn't present. I worry that this could lead quota getting out of sync if there is anything else lurking in the system which might delete the directory.

* Make NotifyStorageModified not add an entry if one is not present (e.g. pass along an update_only) flag.

I'll pursue the latter as it's more scoped.

Alternately, just make NotifyStorageModified a no-op if the delta is 0...

Meh. That degenerates into the first possibility. I'll just do that.

This also means we have bogus entries that are never getting cleaned up, so we'll need to wipe those somehow.

https://chromium-review.googlesource.com/c/chromium/src/+/719390 fixes this part of the overall issue.

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

commit dc603004d4141272f96898375dceb3690c8d6cce
Author: Joshua Bell <jsbell@chromium.org>
Date: Tue Oct 17 22:28:24 2017

Don't create quota-last-modified entries for nonexistent filesystems

When asked by chrome://settings/siteData to delete an origin, the
sandboxed file system code enumerates the types (temporary,
persistent, syncable) and tries to delete each (origin, type) pair.
This would create "last modified" entries for pair even if there was
no such file system in use, and those entries would confuse later
cleanup attempts by chrome://settings/clearBrowserData

The fix is simple: don't notify the quota system if deleting the
directory resulted in a zero-byte change. This prevents the creation
of bogus entries.

Cleanup of bogus entries was hindered by two additional subtle bugs:

* When asked to delete an origin which didn't exist, the File System
   quota client would try to 'rm -rf' an empty path, which
   (fortunately) fails. Detect that case and early-exit with success
   (since there's nothing to delete).

* When asked to delete an unsupported type the IndexedDB quota client
   would report failure; it should report success instead like the
   other quota clients.

With those changes in place, chrome://settings/clearBrowserData cleans
up any bogus entries left behind by the first part of the bug.

Bug:  774666 
Change-Id: Ie49810ef0408b1ae3ad3e2e7546929f1177c5963
Reviewed-on: https://chromium-review.googlesource.com/719390
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509572}
[modify] https://crrev.com/dc603004d4141272f96898375dceb3690c8d6cce/content/browser/indexed_db/indexed_db_quota_client.cc
[modify] https://crrev.com/dc603004d4141272f96898375dceb3690c8d6cce/content/browser/indexed_db/indexed_db_quota_client_unittest.cc
[modify] https://crrev.com/dc603004d4141272f96898375dceb3690c8d6cce/storage/browser/fileapi/obfuscated_file_util.cc
[modify] https://crrev.com/dc603004d4141272f96898375dceb3690c8d6cce/storage/browser/fileapi/obfuscated_file_util_unittest.cc
[modify] https://crrev.com/dc603004d4141272f96898375dceb3690c8d6cce/storage/browser/fileapi/sandbox_file_system_backend_delegate.cc

Thanks for fixing this issue! Do you think this is safe to be merged to M63? 

Safe, but doesn't seem high priority enough to merge to me.

We are currently receiving feedback reports from users how are confused that the Clear Browsing Data dialog doesn't show zero sites with site data after deleting cookies ( https://crbug.com/769425 ). There have been other issues as well that have been fixed in 63 but these quota entries would also be counted and are probably part of this issue.

I think it would be good to merge this fix, as we otherwise wouldn't know if there are additional problems when 63 is launched and we are still receiving reports of incomplete deletions.

Sure, worth a shot.

Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit 8c24c2523717ec174542f424276b71a8281f6847
Author: Joshua Bell <jsbell@chromium.org>
Date: Mon Oct 23 22:30:33 2017

Don't create quota-last-modified entries for nonexistent filesystems

When asked by chrome://settings/siteData to delete an origin, the
sandboxed file system code enumerates the types (temporary,
persistent, syncable) and tries to delete each (origin, type) pair.
This would create "last modified" entries for pair even if there was
no such file system in use, and those entries would confuse later
cleanup attempts by chrome://settings/clearBrowserData

The fix is simple: don't notify the quota system if deleting the
directory resulted in a zero-byte change. This prevents the creation
of bogus entries.

Cleanup of bogus entries was hindered by two additional subtle bugs:

* When asked to delete an origin which didn't exist, the File System
   quota client would try to 'rm -rf' an empty path, which
   (fortunately) fails. Detect that case and early-exit with success
   (since there's nothing to delete).

* When asked to delete an unsupported type the IndexedDB quota client
   would report failure; it should report success instead like the
   other quota clients.

With those changes in place, chrome://settings/clearBrowserData cleans
up any bogus entries left behind by the first part of the bug.

Bug:  774666 
Change-Id: Ie49810ef0408b1ae3ad3e2e7546929f1177c5963
Reviewed-on: https://chromium-review.googlesource.com/719390
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509572}(cherry picked from commit dc603004d4141272f96898375dceb3690c8d6cce)
Reviewed-on: https://chromium-review.googlesource.com/734422
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#169}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/8c24c2523717ec174542f424276b71a8281f6847/content/browser/indexed_db/indexed_db_quota_client.cc
[modify] https://crrev.com/8c24c2523717ec174542f424276b71a8281f6847/content/browser/indexed_db/indexed_db_quota_client_unittest.cc
[modify] https://crrev.com/8c24c2523717ec174542f424276b71a8281f6847/storage/browser/fileapi/obfuscated_file_util.cc
[modify] https://crrev.com/8c24c2523717ec174542f424276b71a8281f6847/storage/browser/fileapi/obfuscated_file_util_unittest.cc
[modify] https://crrev.com/8c24c2523717ec174542f424276b71a8281f6847/storage/browser/fileapi/sandbox_file_system_backend_delegate.cc

Thanks for pushing to get this merged! Other part should be landing soon and we can let that bake.