New issue
Advanced search Search tips

Issue 741934 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK on removing browsing data

Project Member Reported by brettw@chromium.org, Jul 12 2017

Issue description

This problem may result in browsing data not being removed (I'm not entirely sure how this is used) so marking as a P1 in case that's true.

In a debug build I signed into Chrome using my @google.com account. Then I went to settings and logged out (at the top), and accepted the "clear data" prompt.

This asserts in chrome_browsing_data_remover_delegate.cc with an invalid GURL. WebsiteSettingsFilterAdapter has this code:
  // Website settings only use origin-scoped patterns. The only content setting
  // currently supported by ChromeBrowsingDataRemoverDelegate is
  // DURABLE_STORAGE, which also only uses origin-scoped patterns. Such patterns
  // can be directly translated to a GURL.
  GURL url(primary_pattern.ToString());
  DCHECK(url.is_valid());
  return predicate.Run(url);

This comment and the code below are incorrect. In my case, primary_pattern.ToString() is returning the string "ca-service.corp.google.com" which is not a URL. This is because the content setting uses a wildcard scheme. GURL reduces to an empty GURL which is invalid.

I'm assuming data associated with this policy will not be removed as a result.
 
Cc: msramek@chromium.org
Owner: dullweber@chromium.org
Thanks for reporting!

The "clear data" prompt deleted ALL_DATA_TYPES, which includes DATA_TYPE_CONTENT_SETTINGS, and the comment indeed isn't true for content settings in general.

Content settings currently can't fully support filtered deletion (with WebsiteSettingsFilterAdapter), and don't need to, since they're not deleted by any feature that uses that.

We should always delete content settings completely.

Christian, I know that you fixed this in one of your open CLs - would you mind extracting that part and fixing this high priority bug first?
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 14 2017

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

commit e6175fbae15f0377390f8262593a5d3da4305f17
Author: Christian Dullweber <dullweber@chromium.org>
Date: Fri Jul 14 09:20:27 2017

Make content settings non-filterable

Content Settings don't need to be filterable and using
WebsiteSettingsFilterAdapter doesn't work in some cases.

Bug:  741934 
Change-Id: I765cd9683079fa7592afc3aedc0ea0f243d782ed
Reviewed-on: https://chromium-review.googlesource.com/570241
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486711}
[modify] https://crrev.com/e6175fbae15f0377390f8262593a5d3da4305f17/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/e6175fbae15f0377390f8262593a5d3da4305f17/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/e6175fbae15f0377390f8262593a5d3da4305f17/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

Status: Fixed (was: Assigned)
Content settings shouldn't be filtered anymore

Sign in to add a comment