Skip to content

Commit

Permalink
Clear browsing data clears data for type kStorageTypeTemporary but no…
Browse files Browse the repository at this point in the history
…t for kStorageTypeSyncable.

This bug is visible in Android for mail.google.com
Even after Clearing all the data, the syncable data was left behind.
This CL should fix this bug.

BUG=180249

Review URL: https://chromiumcodereview.appspot.com/13357004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193738 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
cramya@chromium.org committed Apr 11, 2013
1 parent 3f05e91 commit ff40238
Show file tree
Hide file tree
Showing 18 changed files with 118 additions and 36 deletions.
25 changes: 20 additions & 5 deletions chrome/browser/browsing_data/browsing_data_file_system_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,21 @@ void BrowsingDataFileSystemHelperImpl::FetchFileSystemInfoInFileThread() {
int64 temporary_usage = quota_util->GetOriginUsageOnFileThread(
filesystem_context_, current,
fileapi::kFileSystemTypeTemporary);
int64 syncable_usage = quota_util->GetOriginUsageOnFileThread(
filesystem_context_, current,
fileapi::kFileSystemTypeSyncable);
file_system_info_.push_back(
FileSystemInfo(
current,
origin_enumerator->HasFileSystemType(
fileapi::kFileSystemTypePersistent),
origin_enumerator->HasFileSystemType(
fileapi::kFileSystemTypeTemporary),
origin_enumerator->HasFileSystemType(
fileapi::kFileSystemTypeSyncable),
persistent_usage,
temporary_usage));
temporary_usage,
syncable_usage));
}

BrowserThread::PostTask(
Expand Down Expand Up @@ -174,13 +180,17 @@ BrowsingDataFileSystemHelper::FileSystemInfo::FileSystemInfo(
const GURL& origin,
bool has_persistent,
bool has_temporary,
bool has_syncable,
int64 usage_persistent,
int64 usage_temporary)
int64 usage_temporary,
int64 usage_syncable)
: origin(origin),
has_persistent(has_persistent),
has_temporary(has_temporary),
has_syncable(has_syncable),
usage_persistent(usage_persistent),
usage_temporary(usage_temporary) {
usage_temporary(usage_temporary),
usage_syncable(usage_syncable){
}

BrowsingDataFileSystemHelper::FileSystemInfo::~FileSystemInfo() {}
Expand Down Expand Up @@ -229,9 +239,12 @@ void CannedBrowsingDataFileSystemHelper::AddFileSystem(
if (type == fileapi::kFileSystemTypePersistent) {
file_system->has_persistent = true;
file_system->usage_persistent = size;
} else {
} else if (type == fileapi::kFileSystemTypeTemporary) {
file_system->has_temporary = true;
file_system->usage_temporary = size;
} else {
file_system->has_syncable = true;
file_system->usage_syncable = size;
}
duplicate_origin = true;
break;
Expand All @@ -247,8 +260,10 @@ void CannedBrowsingDataFileSystemHelper::AddFileSystem(
origin,
(type == fileapi::kFileSystemTypePersistent),
(type == fileapi::kFileSystemTypeTemporary),
(type == fileapi::kFileSystemTypeSyncable),
(type == fileapi::kFileSystemTypePersistent) ? size : 0,
(type == fileapi::kFileSystemTypeTemporary) ? size : 0));
(type == fileapi::kFileSystemTypeTemporary) ? size : 0,
(type == fileapi::kFileSystemTypeSyncable) ? size : 0));
}

void CannedBrowsingDataFileSystemHelper::Reset() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ class BrowsingDataFileSystemHelper
const GURL& origin,
bool has_persistent,
bool has_temporary,
bool has_syncable,
int64 usage_persistent,
int64 usage_temporary);
int64 usage_temporary,
int64 usage_syncable);
~FileSystemInfo();

// The origin for which the information is relevant.
Expand All @@ -60,10 +62,14 @@ class BrowsingDataFileSystemHelper
bool has_persistent;
// True if the origin has a temporary file system.
bool has_temporary;
// True if the origin has a syncable file system.
bool has_syncable;
// Persistent file system usage, in bytes.
int64 usage_persistent;
// Temporary file system usage, in bytes.
int64 usage_temporary;
// Syncable file system usage, in bytes.
int64 usage_syncable;
};

// Creates a BrowsingDataFileSystemHelper instance for the file systems
Expand Down
17 changes: 12 additions & 5 deletions chrome/browser/browsing_data/browsing_data_quota_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@

BrowsingDataQuotaHelper::QuotaInfo::QuotaInfo()
: temporary_usage(0),
persistent_usage(0) {}
persistent_usage(0),
syncable_usage(0) {}

BrowsingDataQuotaHelper::QuotaInfo::QuotaInfo(const std::string& host)
: host(host),
temporary_usage(0),
persistent_usage(0) {}
persistent_usage(0),
syncable_usage(0) {}

BrowsingDataQuotaHelper::QuotaInfo::QuotaInfo(const std::string& host,
int64 temporary_usage,
int64 persistent_usage)
int64 persistent_usage,
int64 syncable_usage)
: host(host),
temporary_usage(temporary_usage),
persistent_usage(persistent_usage) {}
persistent_usage(persistent_usage),
syncable_usage(syncable_usage) {}

BrowsingDataQuotaHelper::QuotaInfo::~QuotaInfo() {}

Expand All @@ -44,12 +48,15 @@ bool BrowsingDataQuotaHelper::QuotaInfo::operator <(
return this->host < rhs.host;
if (this->temporary_usage != rhs.temporary_usage)
return this->temporary_usage < rhs.temporary_usage;
if (this->syncable_usage != rhs.syncable_usage)
return this->syncable_usage < rhs.syncable_usage;
return this->persistent_usage < rhs.persistent_usage;
}

bool BrowsingDataQuotaHelper::QuotaInfo::operator ==(
const BrowsingDataQuotaHelper::QuotaInfo& rhs) const {
return this->host == rhs.host &&
this->temporary_usage == rhs.temporary_usage &&
this->persistent_usage == rhs.persistent_usage;
this->persistent_usage == rhs.persistent_usage &&
this->syncable_usage == rhs.syncable_usage;
}
4 changes: 3 additions & 1 deletion chrome/browser/browsing_data/browsing_data_quota_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class BrowsingDataQuotaHelper
explicit QuotaInfo(const std::string& host);
QuotaInfo(const std::string& host,
int64 temporary_usage,
int64 persistent_usage);
int64 persistent_usage,
int64 syncable_usage);
~QuotaInfo();

// Certain versions of MSVC 2008 have bad implementations of ADL for nested
Expand All @@ -53,6 +54,7 @@ class BrowsingDataQuotaHelper
std::string host;
int64 temporary_usage;
int64 persistent_usage;
int64 syncable_usage;
};

typedef std::list<QuotaInfo> QuotaInfoArray;
Expand Down
18 changes: 15 additions & 3 deletions chrome/browser/browsing_data/browsing_data_quota_helper_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,24 @@ void BrowsingDataQuotaHelperImpl::GotOrigins(
pending_hosts_.insert(std::make_pair(itr->host(), type));

DCHECK(type == quota::kStorageTypeTemporary ||
type == quota::kStorageTypePersistent);
type == quota::kStorageTypePersistent ||
type == quota::kStorageTypeSyncable);

// Calling GetOriginsModifiedSince() for all types by chaining callbacks.
if (type == quota::kStorageTypeTemporary) {
quota_manager_->GetOriginsModifiedSince(
quota::kStorageTypePersistent,
base::Time(),
base::Bind(&BrowsingDataQuotaHelperImpl::GotOrigins,
weak_factory_.GetWeakPtr()));
} else if (type == quota::kStorageTypePersistent) {
quota_manager_->GetOriginsModifiedSince(
quota::kStorageTypeSyncable,
base::Time(),
base::Bind(&BrowsingDataQuotaHelperImpl::GotOrigins,
weak_factory_.GetWeakPtr()));
} else {
// type == quota::kStorageTypePersistent
DCHECK(type == quota::kStorageTypeSyncable);
ProcessPendingHosts();
}
}
Expand Down Expand Up @@ -139,6 +147,9 @@ void BrowsingDataQuotaHelperImpl::GotHostUsage(const std::string& host,
case quota::kStorageTypePersistent:
quota_info_[host].persistent_usage = usage;
break;
case quota::kStorageTypeSyncable:
quota_info_[host].syncable_usage = usage;
break;
default:
NOTREACHED();
}
Expand All @@ -163,7 +174,8 @@ void BrowsingDataQuotaHelperImpl::OnComplete() {
QuotaInfo* info = &itr->second;
// Skip unused entries
if (info->temporary_usage <= 0 &&
info->persistent_usage <= 0)
info->persistent_usage <= 0 &&
info->syncable_usage <= 0)
continue;

info->host = itr->first;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ TEST_F(BrowsingDataQuotaHelperTest, FetchData) {
{"http://example.com/", quota::kStorageTypeTemporary, 1},
{"https://example.com/", quota::kStorageTypeTemporary, 10},
{"http://example.com/", quota::kStorageTypePersistent, 100},
{"https://example.com/", quota::kStorageTypeSyncable, 1},
{"http://example2.com/", quota::kStorageTypeTemporary, 1000},
};

Expand All @@ -152,8 +153,8 @@ TEST_F(BrowsingDataQuotaHelperTest, FetchData) {

std::set<QuotaInfo> expected, actual;
actual.insert(quota_info().begin(), quota_info().end());
expected.insert(QuotaInfo("example.com", 11, 100));
expected.insert(QuotaInfo("example2.com", 1000, 0));
expected.insert(QuotaInfo("example.com", 11, 100, 1));
expected.insert(QuotaInfo("example2.com", 1000, 0, 0));
EXPECT_TRUE(expected == actual);
}

Expand All @@ -162,6 +163,7 @@ TEST_F(BrowsingDataQuotaHelperTest, IgnoreExtensionsAndDevTools) {
{"http://example.com/", quota::kStorageTypeTemporary, 1},
{"https://example.com/", quota::kStorageTypeTemporary, 10},
{"http://example.com/", quota::kStorageTypePersistent, 100},
{"https://example.com/", quota::kStorageTypeSyncable, 1},
{"http://example2.com/", quota::kStorageTypeTemporary, 1000},
{"chrome-extension://abcdefghijklmnopqrstuvwxyz/",
quota::kStorageTypeTemporary, 10000},
Expand All @@ -180,8 +182,8 @@ TEST_F(BrowsingDataQuotaHelperTest, IgnoreExtensionsAndDevTools) {

std::set<QuotaInfo> expected, actual;
actual.insert(quota_info().begin(), quota_info().end());
expected.insert(QuotaInfo("example.com", 11, 100));
expected.insert(QuotaInfo("example2.com", 1000, 0));
expected.insert(QuotaInfo("example.com", 11, 100, 1));
expected.insert(QuotaInfo("example2.com", 1000, 0, 0));
EXPECT_TRUE(expected == actual);
}

Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/browsing_data/browsing_data_remover.cc
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,14 @@ void BrowsingDataRemover::ClearQuotaManagedDataOnIOThread() {
quota::kStorageTypeTemporary, delete_begin_,
base::Bind(&BrowsingDataRemover::OnGotQuotaManagedOrigins,
base::Unretained(this)));

// Do the same for syncable quota.
++quota_managed_storage_types_to_delete_count_;
quota_manager_->GetOriginsModifiedSince(
quota::kStorageTypeSyncable, delete_begin_,
base::Bind(&BrowsingDataRemover::OnGotQuotaManagedOrigins,
base::Unretained(this)));

}

void BrowsingDataRemover::OnGotQuotaManagedOrigins(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ void MockBrowsingDataFileSystemHelper::DeleteFileSystemOrigin(
}

void MockBrowsingDataFileSystemHelper::AddFileSystem(
const GURL& origin, bool has_persistent, bool has_temporary) {
const GURL& origin, bool has_persistent, bool has_temporary,
bool has_syncable) {
response_.push_back(BrowsingDataFileSystemHelper::FileSystemInfo(
origin, has_persistent, has_temporary, 0, 0));
origin, has_persistent, has_temporary, has_syncable, 0, 0, 0));
file_systems_[origin.spec()] = true;
}

void MockBrowsingDataFileSystemHelper::AddFileSystemSamples() {
AddFileSystem(GURL("http://fshost1:1/"), false, true);
AddFileSystem(GURL("http://fshost2:2/"), true, false);
AddFileSystem(GURL("http://fshost3:3/"), true, true);
AddFileSystem(GURL("http://fshost1:1/"), false, true, false);
AddFileSystem(GURL("http://fshost2:2/"), true, false, true);
AddFileSystem(GURL("http://fshost3:3/"), true, true, true);
}

void MockBrowsingDataFileSystemHelper::Notify() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class MockBrowsingDataFileSystemHelper : public BrowsingDataFileSystemHelper {
// Adds a specific filesystem.
void AddFileSystem(const GURL& origin,
bool has_persistent,
bool has_temporary);
bool has_temporary,
bool has_syncable);

// Adds some FilesystemInfo samples.
void AddFileSystemSamples();
Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/browsing_data/mock_browsing_data_quota_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,18 @@ void MockBrowsingDataQuotaHelper::RevokeHostQuota(const std::string& host) {
void MockBrowsingDataQuotaHelper::AddHost(
const std::string& host,
int64 temporary_usage,
int64 persistent_usage) {
int64 persistent_usage,
int64 syncable_usage) {
response_.push_back(QuotaInfo(
host,
temporary_usage,
persistent_usage));
persistent_usage,
syncable_usage));
}

void MockBrowsingDataQuotaHelper::AddQuotaSamples() {
AddHost("quotahost1", 1, 2);
AddHost("quotahost2", 10, 20);
AddHost("quotahost1", 1, 2, 1);
AddHost("quotahost2", 10, 20, 10);
}

void MockBrowsingDataQuotaHelper::Notify() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class MockBrowsingDataQuotaHelper : public BrowsingDataQuotaHelper {

void AddHost(const std::string& host,
int64 temporary_usage,
int64 persistent_usage);
int64 persistent_usage,
int64 syncable_usage);
void AddQuotaSamples();
void Notify();

Expand Down
8 changes: 7 additions & 1 deletion chrome/browser/ui/webui/quota_internals_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ void QuotaInternalsProxy::RequestInfo(
base::Bind(&QuotaInternalsProxy::DidGetGlobalUsage,
weak_factory_.GetWeakPtr()));

quota_manager_->GetGlobalUsage(
quota::kStorageTypeSyncable,
base::Bind(&QuotaInternalsProxy::DidGetGlobalUsage,
weak_factory_.GetWeakPtr()));

quota_manager_->DumpQuotaTable(
base::Bind(&QuotaInternalsProxy::DidDumpQuotaTable,
weak_factory_.GetWeakPtr()));
Expand Down Expand Up @@ -150,7 +155,8 @@ void QuotaInternalsProxy::DidGetHostUsage(const std::string& host,
quota::StorageType type,
int64 usage) {
DCHECK(type == quota::kStorageTypeTemporary ||
type == quota::kStorageTypePersistent);
type == quota::kStorageTypePersistent ||
type == quota::kStorageTypeSyncable);

PerHostStorageInfo info(host, type);
info.set_usage(usage);
Expand Down
3 changes: 2 additions & 1 deletion content/browser/renderer_host/quota_dispatcher_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ class QuotaDispatcherHost::RequestQuotaDispatcher

void Start() {
DCHECK(type_ == quota::kStorageTypeTemporary ||
type_ == quota::kStorageTypePersistent);
type_ == quota::kStorageTypePersistent ||
type_ == quota::kStorageTypeSyncable);
if (type_ == quota::kStorageTypePersistent) {
quota_manager()->GetPersistentHostQuota(
host_,
Expand Down
10 changes: 10 additions & 0 deletions content/browser/storage_partition_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ void ClearOriginOnIOThread(
origins,
quota::kStorageTypePersistent);
}
if (storage_mask & StoragePartition::kQuotaManagedSyncableStorage) {
ClearQuotaManagedOriginsOnIOThread(quota_manager,
origins,
quota::kStorageTypeSyncable);
}
}

void ClearAllDataOnIOThread(
Expand Down Expand Up @@ -103,6 +108,11 @@ void ClearAllDataOnIOThread(
quota::kStorageTypePersistent, base::Time(),
base::Bind(&ClearQuotaManagedOriginsOnIOThread, quota_manager));
}
if (storage_mask & StoragePartition::kQuotaManagedSyncableStorage) {
quota_manager->GetOriginsModifiedSince(
quota::kStorageTypeSyncable, base::Time(),
base::Bind(&ClearQuotaManagedOriginsOnIOThread, quota_manager));
}
}

void ClearedShaderCacheOnIOThread(base::Closure callback) {
Expand Down
3 changes: 3 additions & 0 deletions content/public/browser/storage_partition.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class StoragePartition {
// Local shader storage.
kShaderStorage = 1 << 5,

// Corresponds to quota::kStorageTypeSyncable.
kQuotaManagedSyncableStorage = 1 << 6,

kAllStorage = -1,
};

Expand Down
Loading

0 comments on commit ff40238

Please sign in to comment.