New issue
Advanced search Search tips

Issue 859527 link

Starred by 2 users
Status: Fixed
Owner:
dullweber@chromium.org
Closed: Jul 6
Cc:
tzik@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Deletion of an empty filesystem doesn't work correctly

Project Member Reported by dullweber@chromium.org, Jul 2 
Chrome Version: 69.0.3472.3 (dev)
OS: Linux

To keep track of  https://crbug.com/840080#c9  

What steps will reproduce the problem?
 1. Visit example.com
 2. Execute "window.webkitRequestFileSystem(TEMPORARY, 1024, console.log)" in the console
 3. Delete cookies for "Last hour" at chrome://settings/clearBrowserData
 4. Check chrome://settings/siteData

What is the expected result?
example.com should not be listed 

What happens instead?
example.com is still listed for file system storage.

Deletion for "All time" works correctly. 
This only seems to happen for "empty" filesystems. If data is written, the deletion is working correctly.

The issue can also be reproduced by the test in https://crrev.com/c/1122856 (BrowsingDataRemoverBrowserTestP.EmptyFileSystemDeletion/1)

Comment 1 by dullweber@chromium.org, Jul 3 

The issue seems to be in QuotaDatabase::SetOriginLastAccessTime. 
This method will add a row to OriginInfoTable but won't initialize last_modified_time. 
When deleting data for the last hour, we look for entries that were modified during this time. This means we miss entries that were only accessed but never modified. 
To fix this, we can either set last_modified_time when intially accessing an entry or we could query for "last_modified_time==begin or last_modified_time==0 and last_access_time==begin" when we decide which entries to remove.

I created a CL for the first proposed solution: https://crrev.com/c/1122856

Comment 2 by dullweber@chromium.org, Jul 3 

We could also not create an last_accessed entry if there is no row yet. Why would we need to keep track of data that doesn't exist but was accessed?

Comment 3 by dullweber@chromium.org, Jul 3

Cc: tzik@chromium.org
Components: Blink>Storage>Quota
+tzik@chromium.org for my questions about QuotaManager. What do you think about the issue mentioned above?

Comment 4 by dullweber@chromium.org, Jul 3 

the idea from comment #2 actually doesn't make sense because there is filesystem data that needs to be removed.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 5 

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

commit 76a1ee1935b9500b3ac1e5ff26066811c467bfe8
Author: Christian Dullweber <dullweber@chromium.org>
Date: Thu Jul 05 22:14:56 2018

Quota: Fix deletion of empty FileSystem and empty WebSql entries

When accessing but not writing to a storage system, an entry is
added to OriginInfoTable in QuotaDatabase. This entry doesn't initialize
last_modified_date, so it is not removed when deleting data for a time
range that is not "All time", which queries for the last time a storage
was modified.

TBR=jsbell
Bug:  859527 

Change-Id: I6e8645ea8e69c2fed3e6863d240c530abb2bc0d5
Reviewed-on: https://chromium-review.googlesource.com/1122856
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572862}
[modify] https://crrev.com/76a1ee1935b9500b3ac1e5ff26066811c467bfe8/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/76a1ee1935b9500b3ac1e5ff26066811c467bfe8/storage/browser/fileapi/file_system_quota_util.h
[modify] https://crrev.com/76a1ee1935b9500b3ac1e5ff26066811c467bfe8/storage/browser/quota/quota_database.cc

Comment 6 by dullweber@chromium.org, Jul 6

Owner: dullweber@chromium.org
Status: Fixed (was: Untriaged)
►

Sign in to add a comment