New issue
Advanced search Search tips

Issue 883701 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Team-Security-UX



Sign in to add a comment

SiteDataDeleteHelper doesn't actually delete site data

Project Member Reported by dullweber@chromium.org, Sep 13

Issue description

The name suggests something different but SiteDataDeleteHelper actually only deletes cookies. For some reason it fetches a whole CookiesTreeModel including all other data types but it doesn't delete them. The other data types are deleted by ClearStorageData and ClearLocalStorageData.

https://cs.chromium.org/chromium/src/chrome/browser/android/preferences/website_preference_bridge.cc?l=672&rcl=130be1c380e444e53a2e25302f50629af94682a5

This means that the recent addition of the MediaLicenseHelper doesn't remove media licenses and a lot of data is fetched for no reason. 
We should convert it to only touch cookies and add separate media license deletion.
Some tests would be nice as well. 

 

Comment 1 Deleted

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 20

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

commit 582374a82a4ba13f289fe3a0a99336fde9504ce9
Author: Christian Dullweber <dullweber@chromium.org>
Date: Tue Nov 20 08:45:14 2018

Replace CookiesTreeModel with CookieManager in WebsiteSettingsPreference

The SiteDataDeleteHelper in WebsiteSettingsPreferences is only used to
delete cookies but creates a CookiesTreeModel for various data types.
Instead, we can use CookieManager directly to fetch and delete cookies.

Bug:  883701 
Change-Id: I08efb5d1e4d96902c9f018ca673a019f1334d2a8
Reviewed-on: https://chromium-review.googlesource.com/c/1337617
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609629}
[modify] https://crrev.com/582374a82a4ba13f289fe3a0a99336fde9504ce9/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferencesTest.java
[modify] https://crrev.com/582374a82a4ba13f289fe3a0a99336fde9504ce9/chrome/browser/android/preferences/website_preference_bridge.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 21

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

commit 374346ef83e808ae208142e38f5ab71fb473236a
Author: Christian Dullweber <dullweber@chromium.org>
Date: Wed Nov 21 10:26:11 2018

Add media license deletion to 'Reset site'

MediaLicense deletion was added in crrev.com/c/1213242 but due to a
confusion about the purpose of SiteDataDeleteHelper, it didn't work
correctly. This CL correctly implements MediaLicense deletion when
a site is reset from Settings > SiteSettings > AllSites > [Site].

Tested manually by adding a LOG statement to OriginMatcher in
website_preference_bridge.cc and testing that offline content stored
from https://shaka-player-demo.appspot.com/demo/ triggers the LOG
statement when resetting a site.

Bug:  883701 
Change-Id: I28b93c44daea757084dc5b6e667a2d36bc00b70f
Reviewed-on: https://chromium-review.googlesource.com/c/1337623
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609985}
[modify] https://crrev.com/374346ef83e808ae208142e38f5ab71fb473236a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteDataCleaner.java
[modify] https://crrev.com/374346ef83e808ae208142e38f5ab71fb473236a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java
[modify] https://crrev.com/374346ef83e808ae208142e38f5ab71fb473236a/chrome/browser/android/preferences/website_preference_bridge.cc

Status: Fixed (was: Assigned)

Sign in to add a comment