New issue
Advanced search Search tips

Issue 668114 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 607897



Sign in to add a comment

Move parts of BrowsingDataRemover to content

Project Member Reported by msramek@chromium.org, Nov 23 2016

Issue description

Some web platform features (in the content layer), notably Clear-Site-Data, currently perform browsing data deletion by calling 

ContentBrowserClient::ClearCookies()
ContentBrowserClient::ClearCache()
ContentBrowserClient::ClearSiteData()

This means an unnecessary roundtrip through the embedder at worst, and no or invalid functionality (if the embedder does not have proper implementation of these methods) at worst.

This should not be the case. Cookies and cache are known to the content layer, and web platform features should know how to delete them, and should do so regardless of the embedder.

See this document: https://docs.google.com/document/d/1I6m4QwbTNhG6wdtazamhTnArJN-UMUGqpvwH6InBEaM/ for more details.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 13 2016

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

commit 3248025c236a2f55f79ff58f696b500f8787f28a
Author: msramek <msramek@chromium.org>
Date: Tue Dec 13 15:35:19 2016

Extract embedder-specific data types from BrowsingDataRemover

Define BrowsingDataRemoverDelegate, a class to which BrowsingDataRemover
will delegate all embedder-specific deletions. This class is in 1:1
relationship to BrowsingDataRemover and is owned by it.

Create a Chrome-specific subclass ChromeBrowsingDataRemoverDelegate by
extracting all Chrome datatypes from BrowsingDataRemover::RemoveImpl().

This is the first step of the implementation plan. See the design doc for more
background and explanation of the next steps:
https://docs.google.com/document/d/1I6m4QwbTNhG6wdtazamhTnArJN-UMUGqpvwH6InBEaM/edit#heading=h.m5pzu7ah79n9

This change is covered by BrowsingDataRemoverTest and
BrowsingDataRemoverBrowserTest.

NOTES FOR REVIEWERS:
1. This CL tries to keep the diffs at minimum for easier review, even if the
method ordering could be improved. That will be improved in a follow-up CL.
2. However, the diffs widely differ in BrowsingDataRemover::RemoveImpl and
ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData. Fortunately, one can
convince themself that they are still equivalent by trusting the test coverage
(58 unittests and 6 browsertests).
3. One can convince themself that no datatype was left out by checking the
list of asynchronous tasks, such as "waiting_for_clear_cache_",
"waiting_for_clear_cookies_" etc. present in both .h files and in the AllDone()
method of both classes.
4. The distribution of datatypes might not be final. For example, downloads are
known to content but have a chrome/ dependency. Similarly, the DNS cache lives
in net/ but is retrieved via a class in chrome/. Future CLs might correct their
placement based on how strong the chrome/ dependencies are.

Patchset 3: Simplify the "bool waiting_for_clear_<type>" paradigm by encapsulating it to a SubTask class.

BUG=668114

Review-Url: https://codereview.chromium.org/2554413002
Cr-Commit-Position: refs/heads/master@{#438175}

[modify] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/BUILD.gn
[modify] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/browsing_data_remover.h
[add] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/browsing_data_remover_delegate.h
[modify] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/browsing_data_remover_factory.cc
[modify] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[add] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[add] https://crrev.com/3248025c236a2f55f79ff58f696b500f8787f28a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 4 2017

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

commit 1c8e19d3f6a71793b80525aa995723ece0574756
Author: msramek <msramek@chromium.org>
Date: Wed Jan 04 20:04:53 2017

Reduce BrowsingDataRemover's dependencies on Chrome

1. Remove TimePeriod-related methods from BDR and its tests. Explanation:

TimePeriod represents the deletion options provided in the Chrome UI, and
is used in communication between Chrome UI and BDR, and thus there is no
reason for content/ to know about it. In fact, if BDR was moved to content/
while still containing TimePeriod-related methods, we would have to introduce
a new build target in components/ to make TimePeriod usable in all three of
chrome/, ios/, and content/.

2. Remove the TimeRange class from BDR entirely. Explanation:

TimeRange is a class inside BDR which would move to content/ together with it.
The UI needs a way to convert TimePeriod to TimeRange. Such a conversion cannot
live in content/ (which doesn't know about TimePeriod) nor components/ (which
wouldn't know about TimeRange). It could be detached from BDR to a separate
file in chrome/.

However, the update cost of detaching it to a separate file is the same as
breaking it into a pair of base::Time arguments. Here are reasons why the latter
is better:
-> It reduces the complexity of keeping two confusingly similar TimePeriod and
TimeRange classes
-> Consistency - basically all data storage backends, and also the BDRDelegate,
have an interface with two base::Time arguments rather than base::TimeRange
-> It can be argued that this definition does not logically belong to BDR;
if anyone, it should be base/time/ who defines a class representing
a base::Time interval class

3. Replace Profile with BrowserContext inside BDR (as that's the content/
equivalent).

4. Split the REMOVE_DOWNLOADS section to two parts. The first part (download
history deletion) stays in content/, the second part (download path reset) moves
to the embedder.

5. Add a comment explaining that the |may_delete_history| variable must be honored
in content/ (while deleting download history), but must be provided by the embedder
(from policy or supervised users) through ContentBrowserClient. This is not possible
to fix while BDR is still in chrome/, as chrome/ cannot depend on
ChromeContentBrowserClient.

TBR=jochen@chromium.org
BUG=668114

Review-Url: https://codereview.chromium.org/2578723002
Cr-Commit-Position: refs/heads/master@{#441440}

[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/android/signin/signin_manager_android.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/browsing_data/browsing_data_remover.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/chromeos/profiles/profile_helper.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/net/sdch_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/profile_resetter/profile_resetter.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/profiles/profiles_state.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/ui/webui/options/clear_browser_data_handler.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/components/browsing_data/core/browsing_data_utils.cc
[modify] https://crrev.com/1c8e19d3f6a71793b80525aa995723ece0574756/components/browsing_data/core/browsing_data_utils.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 10 2017

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

commit dc0c3729d77398a6a36eb717f51b5a56bcad38dc
Author: msramek <msramek@chromium.org>
Date: Tue Jan 10 09:29:03 2017

Remove the content settings dependencies from BrowsingDataFilterBuilder et al.

As a part of BrowsingDataRemover's migration to //content, we will also
need to move the auxiliary classes such as BrowsingDataFilterBuilder and
its subclasses.

This is currently not possible, as BrowsingDataFilterBuilder has
a dependency on content settings, which are a //chrome feature. As splitting
the BrowsingDataFilterBuilder and its subclasses into //content and //chrome
versions would be an overkill, we choose a simpler approach, where
ChromeBrowsingDataRemoverDelegate contains an adapter to convert
origin-scoped content settings patterns to a GURL, which is then used with
a regular GURL filter.

Note that a domain-scoped content settings pattern cannot be converted
to a GURL, which means we lose the ability to delete those patterns.
However, this functionality was added only for completeness; none of the
website settings or content settings supported by
ChromeBrowsingDataRemoverDelegate uses domain-scoped patterns. This is
verified using a DCHECK.

We remove the associated tests for content settings pattern filters.
The current functionality of website settings and content settings deletion
is still covered by BrowsingDataRemoverTest and by GURL filter tests of
BrowsingDataFilterBuilder subclasses.

Finally, remove one instance of the content settings pattern filter from
WebsitePreferenceBridge. This can be replaced by simply setting a NULL
value in HostContentSettingsMap.

BUG=668114

Review-Url: https://codereview.chromium.org/2612903005
Cr-Commit-Position: refs/heads/master@{#442538}

[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/android/preferences/website_preference_bridge.cc
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/browsing_data_filter_builder.h
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/origin_filter_builder.cc
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/origin_filter_builder.h
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/origin_filter_builder_unittest.cc
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/registrable_domain_filter_builder.cc
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/registrable_domain_filter_builder.h
[modify] https://crrev.com/dc0c3729d77398a6a36eb717f51b5a56bcad38dc/chrome/browser/browsing_data/registrable_domain_filter_builder_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 10 2017

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

commit 78976d92399cc5d737657fb15269fd4d987208e7
Author: msramek <msramek@chromium.org>
Date: Tue Jan 10 13:11:41 2017

Split BrowsingDataRemover into an abstract interface and implementation.

As a part of the migration of BrowsingDataRemover to content/, we split
it to a pure abstract interface BrowsingDataRemover that declares
- Subclasses and enums
- Removal methods.
- Observer methods.
- Inspection methods for testing.
and its implementation BrowsingDataRemoverImpl.

In the future, BrowsingDataRemover will live in content/public/ to be
used by embedders and BrowsingDataRemoverImpl in content/browsing_data/
as its only implementation. The unittest and browsertest, whose
content-related parts will be later extracted to content/browsing_data/,
have been updated to make a static_cast<> to BrowsingDataRemoverImpl
(as this is a valid assumption).

Note that the SubTask subclass has been duplicated in
BrowsingDataRemoverImpl and ChromeBrowsingDataRemoverDelegate. This is
because the fact that the deletion is organized into subtasks is an
implementation detail that does not belong to the abstract interface
of BrowsingDataRemover; and the fact that both classes do it the same
way is a "coincidence". TODO: Investigate how to avoid this duplication.

BUG=668114

Review-Url: https://codereview.chromium.org/2613833004
Cr-Commit-Position: refs/heads/master@{#442566}

[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/BUILD.gn
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover_factory.cc
[rename] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[add] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover_test_util.cc
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover_test_util.h
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/78976d92399cc5d737657fb15269fd4d987208e7/chrome/browser/chrome_content_browser_client_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 12 2017

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

commit 8733eb80974a809a19c16535bfd39c867845acab
Author: msramek <msramek@chromium.org>
Date: Thu Jan 12 15:15:00 2017

Move HISTORY and PASSWORDS from BrowsingDataRemover to the delegate

In https://codereview.chromium.org/2554413002/, BrowsingDataRemover was
split into content-related and embedder-specific datatypes
(BrowsingDataRemover[Impl] and [Chrome]BrowsingDataRemoverDelegate,
respectively).

The split was done according to where the relevant data storage backends
resided. This CL intends to make the split more precise by looking at it
more semantically.

1. While SSLHostStateDelegate lives in content/, it's state is deleted
   as a part of the HISTORY datatype, which is not a content/ concept.
   Therefore, this was moved to ChromeBrowsingDataRemoverDelegate.

2. The DCHECK verifying that all UI entrypoints to history deletion
   were disabled if !|may_delete_history| was also moved to
   ChromeBrowsingDataRemoverDelegate, as UI is also not a content/
   concept.

3. The deletion of auth cache is done in BrowsingDataRemoverDelegate
   for both the COOKIES and PASSWORDS datatypes. However, PASSWORDS
   are not a content/ concept. Therefore, BrowsingDataRemoverImpl now
   deletes the auth cache for COOKIES, and
   ChromeBrowsingDataRemoverDelegate for PASSWORDS. This is currently
   done by the duplication of the code, as some duplication is
   necessary anyway, and only about ~10 lines of code are duplicated
   unnecessarily; this seems to be too few to justify creating a new
   file in content/public to be shared between content/ and chrome/.

   TODO: This is the second duplication between the two classes. If
   this keeps happenning, consider putting the shared functionality
   to content/public anyway.

BUG=668114

Review-Url: https://codereview.chromium.org/2617403005
Cr-Commit-Position: refs/heads/master@{#443233}

[modify] https://crrev.com/8733eb80974a809a19c16535bfd39c867845acab/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/8733eb80974a809a19c16535bfd39c867845acab/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/8733eb80974a809a19c16535bfd39c867845acab/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 17 2017

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

commit 1e12faa9d69777beeccec850b17f79b98682d2a7
Author: msramek <msramek@chromium.org>
Date: Tue Jan 17 13:12:26 2017

Move plugins to ChromeBrowsingDataRemoverDelegate.

Plugins are known to //content and can be deleted in bulk by
PluginDataRemover in //content/public. However, the filtered deletion of
plugin data is done by BrowsingDataFlashLSOHelper which has strong
//chrome dependencies (through PepperFlashSettingsManager, PluginPrefs
etc.). It is not clear yet whether these dependencies can be resolved,
and so it should be assumed that the filtered deletion of plugin data
must remain in //chrome.

As it makes no sense to split the PLUGINS datatype into bulk deletion
performed in //chrome and filtered deletion performed in //content, we
move the entire datatype into //chrome - at least for now. Note that
no functionality is lost, as //content can still use PluginDataRemover
for bulk removal of plugin data directly.

BUG=668114

Review-Url: https://codereview.chromium.org/2637043002
Cr-Commit-Position: refs/heads/master@{#444037}

[modify] https://crrev.com/1e12faa9d69777beeccec850b17f79b98682d2a7/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/1e12faa9d69777beeccec850b17f79b98682d2a7/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/1e12faa9d69777beeccec850b17f79b98682d2a7/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/1e12faa9d69777beeccec850b17f79b98682d2a7/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[modify] https://crrev.com/1e12faa9d69777beeccec850b17f79b98682d2a7/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/1e12faa9d69777beeccec850b17f79b98682d2a7/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 23 2017

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

commit 29b98cde67e8350d241fad23847ef72fbb939de5
Author: msramek <msramek@chromium.org>
Date: Mon Jan 23 12:47:54 2017

Split BrowsingDataRemoverTest into two tests for Impl and Delegate.

BrowsingDataRemoverImplTest will eventually be moved to content/ together
with BrowsingDataRemoverImpl.

All tests and their helper classes have been moved to one or the other unittest
based on whether the tested datatype is present in content/ or only in chrome/.

A few changes had to be made:
- The *ImplTest class was converted to use BrowserContext instead of
  TestingProfile, as it will use either BrowserContext or TestingBrowserContext
  after it's moved to content/.
- URLRequestContext is now retrieved through StoragePartition where it
  originally was directly retrieved through TestingProfile. This is because
  BrowserContext does not directly expose a GetURLRequestContext method.
- The downloads test also tested the behavior of ChromeDownloadManagerDelegate.
  As downloads are a content/ datatype, the main part of the test was moved
  to *ImplTest, but a section testing ChromeDownloadManagerDelegate was moved
  to *DelegateTest.

There are a few dependencies that were not yet removed in this CL, but will be
removed in a followup:
1. *DelegateTest needs to see and be friend of BrowsingDataRemoverImpl to test
   the usage of BrowsingDataFilterBuilder. To fix this, we need to improve
   BrowsingDataFilterBuilder first
   (uploaded as https://codereview.chromium.org/2647683002/).
2. *ImplTest refers to extensions, which are not known to content/. This is
   caused by BrowsingDataHelper::EXTENSION mask which is respected in
   StoragePartition deletions. To fix this, BrowsingDataHelper must be improved
   first (also coming soon).
3. *ImplTest works with BrowsingContext, but needs to use a TestingProfile to
   create an instance of BrowsingDataRemoverImpl. This is because the factory
   does not accept a TestingBrowsingContext. This will be fixed naturally when
   we move BrowsingDataRemoverImpl to content/ and replace the factory with
   something like ::CreateFor(BrowsingContext*).

BUG=668114

Review-Url: https://codereview.chromium.org/2641853002
Cr-Commit-Position: refs/heads/master@{#445367}

[modify] https://crrev.com/29b98cde67e8350d241fad23847ef72fbb939de5/chrome/browser/browsing_data/browsing_data_remover_impl.h
[add] https://crrev.com/29b98cde67e8350d241fad23847ef72fbb939de5/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[delete] https://crrev.com/77cbaaa8e8168b2725c4853ebffcfb8ca802e35b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[add] https://crrev.com/29b98cde67e8350d241fad23847ef72fbb939de5/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/29b98cde67e8350d241fad23847ef72fbb939de5/chrome/test/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 23 2017

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

commit e5dce61417dc3e705837e9bc90d14dacbb45c694
Author: jkrcal <jkrcal@chromium.org>
Date: Mon Jan 23 13:11:49 2017

Revert of Split BrowsingDataRemoverTest into two tests for Impl and Delegate. (patchset #3 id:40001 of https://codereview.chromium.org/2641853002/ )

Reason for revert:
Breaks the buildbot in ChromiumChromiumos on Linux ChromiumOS Full

Original issue's description:
> Split BrowsingDataRemoverTest into two tests for Impl and Delegate.
>
> BrowsingDataRemoverImplTest will eventually be moved to content/ together
> with BrowsingDataRemoverImpl.
>
> All tests and their helper classes have been moved to one or the other unittest
> based on whether the tested datatype is present in content/ or only in chrome/.
>
> A few changes had to be made:
> - The *ImplTest class was converted to use BrowserContext instead of
>   TestingProfile, as it will use either BrowserContext or TestingBrowserContext
>   after it's moved to content/.
> - URLRequestContext is now retrieved through StoragePartition where it
>   originally was directly retrieved through TestingProfile. This is because
>   BrowserContext does not directly expose a GetURLRequestContext method.
> - The downloads test also tested the behavior of ChromeDownloadManagerDelegate.
>   As downloads are a content/ datatype, the main part of the test was moved
>   to *ImplTest, but a section testing ChromeDownloadManagerDelegate was moved
>   to *DelegateTest.
>
> There are a few dependencies that were not yet removed in this CL, but will be
> removed in a followup:
> 1. *DelegateTest needs to see and be friend of BrowsingDataRemoverImpl to test
>    the usage of BrowsingDataFilterBuilder. To fix this, we need to improve
>    BrowsingDataFilterBuilder first
>    (uploaded as https://codereview.chromium.org/2647683002/).
> 2. *ImplTest refers to extensions, which are not known to content/. This is
>    caused by BrowsingDataHelper::EXTENSION mask which is respected in
>    StoragePartition deletions. To fix this, BrowsingDataHelper must be improved
>    first (also coming soon).
> 3. *ImplTest works with BrowsingContext, but needs to use a TestingProfile to
>    create an instance of BrowsingDataRemoverImpl. This is because the factory
>    does not accept a TestingBrowsingContext. This will be fixed naturally when
>    we move BrowsingDataRemoverImpl to content/ and replace the factory with
>    something like ::CreateFor(BrowsingContext*).
>
>
> BUG=668114
>
> Review-Url: https://codereview.chromium.org/2641853002
> Cr-Commit-Position: refs/heads/master@{#445367}
> Committed: https://chromium.googlesource.com/chromium/src/+/29b98cde67e8350d241fad23847ef72fbb939de5

TBR=bauerb@chromium.org,msramek@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=668114

Review-Url: https://codereview.chromium.org/2648223003
Cr-Commit-Position: refs/heads/master@{#445370}

[modify] https://crrev.com/e5dce61417dc3e705837e9bc90d14dacbb45c694/chrome/browser/browsing_data/browsing_data_remover_impl.h
[delete] https://crrev.com/df762a58be5c19144ebb9930918ac1b0448378f7/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[add] https://crrev.com/e5dce61417dc3e705837e9bc90d14dacbb45c694/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[delete] https://crrev.com/df762a58be5c19144ebb9930918ac1b0448378f7/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/e5dce61417dc3e705837e9bc90d14dacbb45c694/chrome/test/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 23 2017

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

commit 44727f4d42756a3d89e1466ba662548e40c752c8
Author: msramek <msramek@chromium.org>
Date: Mon Jan 23 17:48:57 2017

Reland "Split BrowsingDataRemoverTest into two tests for Impl and Delegate."

This relands https://codereview.chromium.org/2641853002/ which was reverted
in https://codereview.chromium.org/2641853002/.

Patchset #1 is the exact copy of the original CL.
Patchset #2 is the fix.

The breakage occured in the test
ChromeBrowsingDataRemoverDelegateTest.RemoveMultipleTypesHistoryProhibited
and was not spotted locally, as this test is behind the ifdef:

"defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON)"

The problem was that the helper class StoragePartitionRemovalData was
missing, as it had been moved to BrowsingDataRemoverImplTest during
the split. To avoid copying the class into this test file, we changed
the mask from COOKIES to PASSWORDS (which, unlike COOKIES, do not live
in StoragePartition). This does not affect the validy of the test, as its
purpose is to verify that if history deletion is prohibited and HISTORY
is present in the mask, it is not deleted, but other types still are.

BUG=668114

Review-Url: https://codereview.chromium.org/2651673002
Cr-Commit-Position: refs/heads/master@{#445402}

[modify] https://crrev.com/44727f4d42756a3d89e1466ba662548e40c752c8/chrome/browser/browsing_data/browsing_data_remover_impl.h
[add] https://crrev.com/44727f4d42756a3d89e1466ba662548e40c752c8/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[delete] https://crrev.com/59c02c3aa8f983c2adecb654bd71e2ada5fb4d8e/chrome/browser/browsing_data/browsing_data_remover_unittest.cc
[add] https://crrev.com/44727f4d42756a3d89e1466ba662548e40c752c8/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/44727f4d42756a3d89e1466ba662548e40c752c8/chrome/test/BUILD.gn

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 24 2017

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

commit c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6
Author: msramek <msramek@chromium.org>
Date: Tue Jan 24 12:01:05 2017

Consolidate Origin- and RegistrableDomain- FilterBuilder into one class

...called BrowsingDataFilterBuilderImpl.

The new class will eventually live in content/ (together with
BrowsingDataRemoverImpl) but will also be used by chrome/, therefore
we also create an abstract interface BrowsingDataFilterBuilder.

We also consolidate the tests into one file. As the new class is theoretically
able to handle origins and registrable domains at the same time, we add new
tests for these cases (although this situation does not occur in production).

The previous state with two different classes increased the complexity in
ChromeContentBrowserClientTest, which we now simplified.

We also introduced a Copy() method to the new class which allowed us to
duplicate the filter builder in BrowsingDataRemoverImplTest and
ChromeBrowsingDataRemoverDelegateTest where needed, which means we no longer
rely on passing the class via reference, thus we don't need to use the private
BrowsingDataRemoverImpl::RemoveInternal method, and therefore we could remove
friend declarations from BrowsingDataRemoverImpl. As a side effect, calling
the public methods now means that checks for filterable datatypes are
performed, and since HISTORY and PASSWORDS are not yet marked as filterable,
we had to disable the corresponding tests.

TBR=bauerb@chromium.org
BUG=668114

Review-Url: https://codereview.chromium.org/2647683002
Cr-Commit-Position: refs/heads/master@{#445706}

[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/BUILD.gn
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/android/preferences/pref_service_bridge.cc
[delete] https://crrev.com/acc84fbd540feec6c0fa6b2165b0da7eeb8d0456/chrome/browser/browsing_data/browsing_data_filter_builder.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_filter_builder.h
[add] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_filter_builder_impl.cc
[add] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_filter_builder_impl.h
[rename] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_filter_builder_impl_unittest.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[delete] https://crrev.com/acc84fbd540feec6c0fa6b2165b0da7eeb8d0456/chrome/browser/browsing_data/origin_filter_builder.cc
[delete] https://crrev.com/acc84fbd540feec6c0fa6b2165b0da7eeb8d0456/chrome/browser/browsing_data/origin_filter_builder.h
[delete] https://crrev.com/acc84fbd540feec6c0fa6b2165b0da7eeb8d0456/chrome/browser/browsing_data/registrable_domain_filter_builder.cc
[delete] https://crrev.com/acc84fbd540feec6c0fa6b2165b0da7eeb8d0456/chrome/browser/browsing_data/registrable_domain_filter_builder.h
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/c7ee9d5e15a8d98d5f5fe2afcff74bec2f3ae7d6/chrome/test/BUILD.gn

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 24 2017

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

commit bf350a623625a7a12a96add5421e59a1faa66fb5
Author: msramek <msramek@chromium.org>
Date: Tue Jan 24 14:36:45 2017

Remove password manager's dependency of BrowsingDataHelper

As a part of the browsing_data/ refactoring in crbug.com/668114,
the BrowsingDataHelper class will no longer exist in its current form.
Before that happens, this CL removes the class' single external
dependency which is password_manager/.

The method BrowsingDataHelper::IsWebScheme() is copied to the password_manager/
codebase together with its tests. Specifically, in ChromePasswordManagerClient,
since this class contains one of its two callsites, and is reachable by
the second callsite.

It's up to discussion whether that is the semantically correct place to put
it; however, the method refers to //content and //extensions, therefore it must
be placed in //chrome/browser. Finding a different definition of a "web scheme"
would avoid that, however, this CL does not attempt that as doing so would
change the functionality of password manager.

BUG=668114

Review-Url: https://codereview.chromium.org/2656453002
Cr-Commit-Position: refs/heads/master@{#445722}

[modify] https://crrev.com/bf350a623625a7a12a96add5421e59a1faa66fb5/chrome/browser/password_manager/chrome_password_manager_client.cc
[modify] https://crrev.com/bf350a623625a7a12a96add5421e59a1faa66fb5/chrome/browser/password_manager/chrome_password_manager_client.h
[modify] https://crrev.com/bf350a623625a7a12a96add5421e59a1faa66fb5/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
[modify] https://crrev.com/bf350a623625a7a12a96add5421e59a1faa66fb5/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 3 2017

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

commit a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6
Author: msramek <msramek@chromium.org>
Date: Fri Feb 03 13:34:13 2017

Move BrowsingDataFilterBuilder[Impl] to content/

BrowsingDataFilterBuilder[Impl] is to be used by both content/ and chrome/
parts of the browsing_data/ codebase. Therefore, we move it to
content/browsing_data with a public interface in content/public/.

Note that this CL is the *first* part of browsing_data/ codebase that is
moving to content/; BrowsingDataFilterBuilder has no users in content/ yet.
BrowsingDataRemover[Impl] will move soon after. The intention of this CL
is to make the upcoming change smaller.

BUG=668114

Review-Url: https://codereview.chromium.org/2657453006
Cr-Commit-Position: refs/heads/master@{#447983}

[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/BUILD.gn
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/browsing_data_remover_delegate.h
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/chrome/test/BUILD.gn
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/browser/BUILD.gn
[add] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/browser/browsing_data/OWNERS
[rename] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/browser/browsing_data/browsing_data_filter_builder_impl.cc
[rename] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/browser/browsing_data/browsing_data_filter_builder_impl.h
[rename] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/browser/browsing_data/browsing_data_filter_builder_impl_unittest.cc
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/public/browser/BUILD.gn
[rename] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/public/browser/browsing_data_filter_builder.h
[modify] https://crrev.com/a3c7cfd551d34f0edd4fc8c84d3aa172eba303c6/content/test/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 8 2017

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

commit 7e5c61fd4a063db154f623e75bc71bdcc4bf65fc
Author: msramek <msramek@chromium.org>
Date: Wed Feb 08 11:21:32 2017

Introduce the concept of web schemes capable of storage

Cookies, local storage, IndexedDB, etc. can be used for various URL schemes,
including Chrome-specific and internal ones such as chrome-extension://,
chrome://, chrome-devtools:// etc.

However, in the browsing_data/ codebase, when the user chooses to delete
"site data", we must only delete data from actual websites that user visited,
not data from Chrome's internal modules; that means data associated with
schemes like http[s]://, file://, etc.

Until now, this definition was defined as
ChildProcessSecurityPolicy::IsWebSafeScheme() minus the embedder-specific
schemes chrome-extension:// and chrome-devtools://.

As we're moving parts of browsing_data/ codebase to content/ (crbug.com/668114),
this approach is not satisfactory. We need a definition that can be used
in content/, and embedders can possibly add more schemes, but not remove
them.

This CL introduces such definition to url_utils.h.

BUG=668114

Review-Url: https://codereview.chromium.org/2661543003
Cr-Commit-Position: refs/heads/master@{#448957}

[modify] https://crrev.com/7e5c61fd4a063db154f623e75bc71bdcc4bf65fc/chrome/browser/browsing_data/browsing_data_helper.cc
[modify] https://crrev.com/7e5c61fd4a063db154f623e75bc71bdcc4bf65fc/chrome/browser/browsing_data/browsing_data_helper_unittest.cc
[modify] https://crrev.com/7e5c61fd4a063db154f623e75bc71bdcc4bf65fc/url/url_util.cc
[modify] https://crrev.com/7e5c61fd4a063db154f623e75bc71bdcc4bf65fc/url/url_util.h

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 14 2017

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

commit 1c2b3ca05716cc40a8a89f51527b0eef52dddde6
Author: msramek <msramek@chromium.org>
Date: Tue Mar 14 17:57:56 2017

Split browsing data masks between content and embedder

As a part of the refactoring to move content-related browsing data
infrastructure to content, we split BrowsingDataRemover::RemoveDataMask
and BrowsingDataHelper::OriginTypeMask between content and embedder.

The CL consists of the following steps:

1. BrowsingDataRemover::RemoveDataMask now only contains content datatypes.
Chrome-specific datatypes are moved to ChromeBrowsingDataRemoverDelegate.
The enum in ChromeBrowsingDataRemoverDelegate also copies all values
from BrowsingDataRemover so that this enum inheritance better mimics
class inheritance. Note that this is a second attempt at this change, following
the discussion in https://codereview.chromium.org/2697123004/ where we tried
converting enums into pointers first.

The concept of FILTERABLE_DATATYPES has been moved to
ChromeBrowsingDataRemoverDelegate, since all content data types are filterable.

2. The same exercise is done for BrowsingDataHelper::OriginTypeMask. Its
first two items, UNPROTECTED_WEB and PROTECTED_WEB correspond to the boolean
returned by content::SpecialStoragePolicy::IsProtected(). Therefore, they
are moved to content as a new enum in BrowsingDataRemover. The third item,
EXTENSION, is a Chrome-specific concept and stays in chrome/ code, as a new
enum in ChromeBrowsingDataRemoverDelegate.

BrowsingDataRemoverImpl is now able to tell about each origin whether it is
UNPROTECTED_WEB or PROTECTED_WEB. It delegates the decision what is an
EXTENSION to ChromeBrowsingDataRemoverDelegate. A new method has been added
to the BrowsingDataRemoverDelegate for this purpose.

BrowsingDataHelper now serves no other purpose than recognizing webby and
extension URLs for various classes in browsing_data/ and can be eventually
deprecated (though doing so in this CL would significantly blow it in size).

The behavior of UNPROTECTED_WEB and PROTECTED_WEB is already sufficiently
tested in StoragePartition-related tests of BrowsingDataRemoverTest. We just
had to replace MockExtensionSpecialStoragePolicy with the regular
MockSpecialStoragePolicy. Testcases from BrowsingDataHelperTest have been moved
to ChromeBrowsingDataRemoverDelegateTest to cover the remaining EXTENSION type.

3. Since BrowsingDataRemover and ChromeBrowsingDataRemoverDelegate now host
both the data type and origin type enums, we changed the naming to call out
the difference; all data types are now prefixed with DATA_TYPE, all origin
types with ORIGIN_TYPE.

4. All other files contain equivalent replacements.

TBR=dbeam@chromium.org,jochen@chromium.org
BUG=668114

Review-Url: https://codereview.chromium.org/2733393003
Cr-Commit-Position: refs/heads/master@{#456757}

[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/android/preferences/pref_service_bridge.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/android/signin/signin_manager_android.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_helper.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_helper.h
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_helper_unittest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_remover_delegate.h
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/mock_browsing_data_remover.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/browsing_data/mock_browsing_data_remover.h
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/chromeos/profiles/profile_helper.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/net/sdch_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/profile_resetter/profile_resetter.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/profiles/profiles_state.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/ui/webui/options/clear_browser_data_handler.cc
[modify] https://crrev.com/1c2b3ca05716cc40a8a89f51527b0eef52dddde6/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 15 2017

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

commit f4ab8376fc0e86d3a8e6516256711c9da85002c3
Author: msramek <msramek@chromium.org>
Date: Wed Mar 15 10:10:40 2017

Introduce DATA_TYPE_DOM_STORAGE for DOM-Accessible storage

Clear-Site-Data specifies a "storage" datatype, representing DOM-accessible
storage. This corresponds to the 7 storage datatypes in BrowsingDataRemover,
and possibly others defined in the embedder.

We define the DATA_TYPE_EMBEDDER_DOM_STORAGE pseudo-datatype that serves as
a signal for the embedder delegate to delete its DOM-accessible storage.
Then, we define DATA_TYPE_DOM_STORAGE as a group datatype (bitmask) of the
abovementioned 7 storage datatypes and DATA_TYPE_EMBEDDER_STORAGE.

See the design doc for the rationale for this approach.
https://docs.google.com/document/d/1I6m4QwbTNhG6wdtazamhTnArJN-UMUGqpvwH6InBEaM/edit#heading=h.q2f5zo9hgn7o

Currently, the only datatype in Chrome that is related to DOM storage is
DURABLE_PERMISSION (considering that PLUGIN_DATA is currently deleted
by Clear-Site-Data as part of "cookies"). The other datatypes, such as
WEB_APP_DATA and SITE_USAGE_DATA are only metadata that Chrome keeps.
Therefore, replacing DATA_TYPE_SITE_DATA with DATA_TYPE_DOM_STORAGE in
ChromeContentBrowserClient::ClearSiteData() is also a functional change
to Clear-Site-Data, weakening it to only delete DOM-accessible storage.

BUG=668114

Review-Url: https://codereview.chromium.org/2745323004
Cr-Commit-Position: refs/heads/master@{#457046}

[modify] https://crrev.com/f4ab8376fc0e86d3a8e6516256711c9da85002c3/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/f4ab8376fc0e86d3a8e6516256711c9da85002c3/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/f4ab8376fc0e86d3a8e6516256711c9da85002c3/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/f4ab8376fc0e86d3a8e6516256711c9da85002c3/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/f4ab8376fc0e86d3a8e6516256711c9da85002c3/chrome/browser/chrome_content_browser_client_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Mar 21 2017

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

commit dfe95153bd3532636ab0badcf624c3caaa197099
Author: msramek <msramek@chromium.org>
Date: Tue Mar 21 10:37:13 2017

Move the WebCacheManager call from BrowsingDataRemoverImpl to the delegate

...since WebCacheManager has a dependency on content/browser, and thus
cannot be used from BrowsingDataRemoverImpl once it moves there.

BUG=668114

Review-Url: https://codereview.chromium.org/2757573004
Cr-Commit-Position: refs/heads/master@{#458367}

[modify] https://crrev.com/dfe95153bd3532636ab0badcf624c3caaa197099/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/dfe95153bd3532636ab0badcf624c3caaa197099/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 23 2017

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

commit 143b4e10c91a42f3d43ca721b4ec137a2495c2e0
Author: msramek <msramek@chromium.org>
Date: Thu Mar 23 16:34:05 2017

Remove OriginFilterBuilderTest.

The OriginFilterBuilder class was removed in
https://codereview.chromium.org/2647683002, where this test was also
removed from the build target, but the file was accidentally left in
the codebase.

The testcases from this file today live in BrowsingDataFilterBuilderTest.

BUG=668114

Review-Url: https://codereview.chromium.org/2770963003
Cr-Commit-Position: refs/heads/master@{#459112}

[delete] https://crrev.com/33100857169da33f18778cb1f3073e607ee48e27/chrome/browser/browsing_data/origin_filter_builder_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 23 2017

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

commit 2232811f6fb779616100f1c3fed25ebe8f545703
Author: msramek <msramek@chromium.org>
Date: Thu Mar 23 16:50:29 2017

Change MockBrowsingDataRemover to MockBrowsingDataRemoverDelegate

BrowsingDataRemover is about to move to content/, and does not expect
to be subclassed from the embedder. The responsibility of embedder
specialization is now on BrowsingDataRemoverDelegate.

Therefore, tests in the embedder should now mock the delegate instead
of the remover itself. This CL changes MockBrowsingDataRemover to
MockBrowsingDataRemoverDelegate and updates its (currently only one)
callsite.

Related changes:
- Added missing forward declarations to BrowsingDataRemoverDelegate
  (the code compiled, as its only implementation did have the correct
  includes)
- BrowsingDataRemoverDelegate takes BrowsingDataFilterBuilder through
  a const ref (not a unique_ptr), therefore we need to make a copy
  in MockBrowsingDataRemoverDelegate
- For consistency, the MockBrowsingDataRemoverDelegate's ExpectCall
  methods now also take BrowsingDataFilterBuilder as a const ref;
  this creates an unnecessary copy in the current test, but is
  a cleaner interface if the class is reused
- StoragePartitionHttpCacheDataRemover has been changed to tolerate
  non-exist URLRequestContextGetter-s (since BrowsingDataRemover
  now calls it to delete cache even in this test)

BUG=668114

Review-Url: https://codereview.chromium.org/2768883002
Cr-Commit-Position: refs/heads/master@{#459121}

[modify] https://crrev.com/2232811f6fb779616100f1c3fed25ebe8f545703/chrome/browser/BUILD.gn
[modify] https://crrev.com/2232811f6fb779616100f1c3fed25ebe8f545703/chrome/browser/browsing_data/browsing_data_remover_delegate.h
[delete] https://crrev.com/712a32b9cdf3b50e63eb25f62e901eff8e84ccf9/chrome/browser/browsing_data/mock_browsing_data_remover.cc
[delete] https://crrev.com/712a32b9cdf3b50e63eb25f62e901eff8e84ccf9/chrome/browser/browsing_data/mock_browsing_data_remover.h
[add] https://crrev.com/2232811f6fb779616100f1c3fed25ebe8f545703/chrome/browser/browsing_data/mock_browsing_data_remover_delegate.cc
[add] https://crrev.com/2232811f6fb779616100f1c3fed25ebe8f545703/chrome/browser/browsing_data/mock_browsing_data_remover_delegate.h
[modify] https://crrev.com/2232811f6fb779616100f1c3fed25ebe8f545703/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/2232811f6fb779616100f1c3fed25ebe8f545703/components/browsing_data/content/storage_partition_http_cache_data_remover.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 27 2017

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

commit ee2be6e582be6b73f96eb7caf150e57fb1817dc4
Author: msramek <msramek@chromium.org>
Date: Mon Mar 27 19:28:01 2017

Move StoragePartitionHttpCacheDataRemover to content/

StoragePartitionHttpCacheDataRemover is used to delete the contents of a cache
obtained from a given StoragePartition. There are three callsites:
BrowsingDataRemover (which will soon move to content/), CacheCounter (chrome/)
and WebViewGuest (in extensions/).

The original plan was that StoragePartitionHttpCacheDataRemover would move
to content together with BrowsingDataRemover, and the latter two callsites
would then request cache deletion using a DATA_TYPE_CACHE request
to BrowsingDataRemover rather than StoragePartitionHttpCacheDataRemover.

However, this is not possible, as WebViewGuest requests a deletion for
a particular non-default StoragePartition (unlike BrowsingDataRemover, which
uses BrowserContext::GetDefaultStoragePartition()).

Therefore, we move StoragePartitionHttpCacheDataRemover to content/ first
and expose it in the content/public interface through
StoragePartition::ClearHttpAndMediaCaches(). Together with it, we move the
ConditionalCacheDeletionHelper class (which is only used by the former
for conditional deletions and can eventually be folded into it).
Additionally, we move the class' browsertest which also necessitates the move
of CacheTestUtil to content/public/test. This can be eventually folded into
BrowsingDataRemover's test utils once BrowsingDataRemover into content/,
since browsing_data/ tests are the only users of these utils.

BUG=668114

Review-Url: https://codereview.chromium.org/2757923002
Cr-Commit-Position: refs/heads/master@{#459851}

[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/chrome/browser/browsing_data/cache_counter_browsertest.cc
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/chrome/browser/browsing_data/conditional_cache_counting_helper_browsertest.cc
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/chrome/test/BUILD.gn
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/components/browsing_data/content/BUILD.gn
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/BUILD.gn
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/browsing_data/conditional_cache_deletion_helper.cc
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/browsing_data/conditional_cache_deletion_helper.h
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/browsing_data/conditional_cache_deletion_helper_browsertest.cc
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/browsing_data/storage_partition_http_cache_data_remover.cc
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/browsing_data/storage_partition_http_cache_data_remover.h
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/browser/storage_partition_impl.h
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/public/browser/storage_partition.h
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/public/test/cache_test_util.cc
[rename] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/public/test/cache_test_util.h
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/content/test/BUILD.gn
[modify] https://crrev.com/ee2be6e582be6b73f96eb7caf150e57fb1817dc4/extensions/browser/guest_view/web_view/web_view_guest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 6 2017

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

commit c9f101b745929a50b391abf446cefb6b2f4b4bd6
Author: msramek <msramek@chromium.org>
Date: Thu Apr 06 17:29:51 2017

Fix the multi-threaded access to WeakPtr<BrowsingDataRemoverImpl>

Previously, we depended on a WeakPtr to access BrowsingDataRemoverImpl
to gain access to its delegate on which we called the embedder origin
type matching method.

With this CL, the delegate provides a callback to a static method,
eliminating the need for any pointers.

To prevent a similar bug from happening in the future, we proactively
bind the WeakPtr<BrowsingDataRemoverImpl> to the UI thread, so it
would immediately DCHECK if it was misused on the IO thread.

BUG=705538,668114

Review-Url: https://codereview.chromium.org/2781083002
Cr-Commit-Position: refs/heads/master@{#462530}

[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/browsing_data_remover_delegate.h
[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/mock_browsing_data_remover_delegate.cc
[modify] https://crrev.com/c9f101b745929a50b391abf446cefb6b2f4b4bd6/chrome/browser/browsing_data/mock_browsing_data_remover_delegate.h

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 12 2017

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

commit bb8dc5c563daf902f09b6f0ab6079d3ec66a089f
Author: msramek <msramek@chromium.org>
Date: Wed Apr 12 23:16:08 2017

Move BrowsingDataRemoverImpl:: CompletionInhibitor to the public interface

The CompletionInhibitor interface is removed in favor of a simpler
SetWouldCompleteCallbackForTesting() method defined in BrowsingDataRemover.
Its implementation BrowsingDataRemoverCompletionInhibitor is updated to use
the new interface.

This is necessary due to the fact that CompletionInhibitor was used both
in chrome/ and in BrowsingDataRemoverImplTest which will move to content/,
and thus its definition in BrowsingDataRemoverImpl (which will move to
non-public content/) would not be visible in chrome/.

On the way, this CL also resolves a TODO to make CompletionInhibitor non-static.

TBR=dbeam@chromium.org
BUG= 483528 , 668114

Review-Url: https://codereview.chromium.org/2802013002
Cr-Commit-Position: refs/heads/master@{#464199}

[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/browsing_data/browsing_data_remover.h
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/browsing_data/browsing_data_remover_impl.h
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/browsing_data/browsing_data_remover_test_util.cc
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/browsing_data/browsing_data_remover_test_util.h
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/profiles/profile_manager_browsertest.cc
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/ui/webui/options/clear_browser_data_browsertest.cc
[modify] https://crrev.com/bb8dc5c563daf902f09b6f0ab6079d3ec66a089f/chrome/browser/ui/webui/profile_helper_browsertest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 26 2017

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

commit e169ccbe5d1f406337c2b5e758f58b7966c51b6c
Author: msramek <msramek@chromium.org>
Date: Wed Apr 26 05:21:41 2017

Move BrowsingDataRemover to content/

Changes in this CL:

1. The public interfaces BrowsingDataRemover and BrowsingDataRemoverDelegate
move to content/public/browser. BrowsingDataRemoverImpl moves to
content/browser/browsing_data.

2. Previously, BrowsingDataRemoverImpl was a KeyedService created by
BrowsingDataRemoverFactory. After this CL, it is owned by BrowserContext.
Conversely, ChromeBrowsingRemoverDelegate (which stays in chrome/) becomes
a KeyedService. The former BrowsingDataRemoverFactory is thus adapted into
ChromeBrowsingDataRemoverDelegateFactory.

BrowsingDataRemoverDelegate is now owned by the embedder (rather than
BrowsingDataRemover) and supplied by embedders through
BrowserContext::GetBrowsingDataRemoverDelegate(). Profile (in chrome/)
supplies ChromeBrowsingDataRemoverDelegate. The implementation for other
embedders is left as TODO.

3. BrowsingDataRemoverImplTest moves to content/browser/browsing/data.
browsing_data_remover_test_util that is used by this test as well as various
tests in chrome/ is moved to content/public/test.

4. BrowsingDataRemover can only delete download history if this is feature is
not disabled by administrator. Previously, BrowsingDataRemover queried
PrefService directly. We now pass this information through
BrowsingDataRemoverDelegate. The default value is true,
ChromeBrowsingDataRemoverDelegate queries PrefService.

5. The removal of ContentBrowserClient interfaces ClearCookies(), ClearCache(),
ClearSiteData() and their replacement by BrowsingDataRemover is left as a TODO.

6. All other changes are adding / removing includes, updating BUILD files,
replacing BrowsingDataRemoverFactory with BrowserContext::GetBrowsingDataRemover,
formatting (mostly triggered by prepending content:: to class names), and the
removal of forgotten "#ifdef (EXTENSIONS)" in BrowsingDataRemoverImplTest.

TBR=dbeam@chromium.org
BUG=668114

Review-Url: https://codereview.chromium.org/2827523003
Cr-Commit-Position: refs/heads/master@{#467241}

[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/BUILD.gn
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/android/browsing_data/browsing_data_bridge.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/android/signin/signin_manager_android.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[delete] https://crrev.com/ac3c4618a593af5860af38169befa3890e248b34/chrome/browser/browsing_data/browsing_data_remover_factory.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.cc
[add] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_factory.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/mock_browsing_data_remover_delegate.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/browsing_data/mock_browsing_data_remover_delegate.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/chrome_content_browser_client_unittest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/chromeos/profiles/profile_helper.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/chromeos/profiles/profile_helper.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/extensions/api/browsing_data/browsing_data_api.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/extensions/api/browsing_data/browsing_data_api.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/net/sdch_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profile_resetter/profile_resetter.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profile_resetter/profile_resetter.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profiles/profile_impl.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profiles/profile_manager_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/profiles/profiles_state.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ssl/chrome_ssl_host_state_delegate_test.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/options/clear_browser_data_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/options/clear_browser_data_handler.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/options/clear_browser_data_handler.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/profile_helper_browsertest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/chrome/test/BUILD.gn
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/browser/BUILD.gn
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/browser/browser_context.cc
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/browser/browsing_data/browsing_data_remover_impl.cc
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/browser/browsing_data/browsing_data_remover_impl.h
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/browser/browsing_data/browsing_data_remover_impl_unittest.cc
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/browser/BUILD.gn
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/browser/browser_context.h
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/browser/browsing_data_remover.h
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/browser/browsing_data_remover_delegate.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/browser/content_browser_client.h
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/test/browsing_data_remover_test_util.cc
[rename] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/test/browsing_data_remover_test_util.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/public/test/cache_test_util.h
[modify] https://crrev.com/e169ccbe5d1f406337c2b5e758f58b7966c51b6c/content/test/BUILD.gn

Project Member

Comment 24 by bugdroid1@chromium.org, May 20 2017

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

commit 507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308
Author: msramek <msramek@chromium.org>
Date: Sat May 20 12:45:32 2017

Remove the ClearCookies and ClearCache methods from ContentBrowserClient

These methods were used by content/ to request the deletion of cookies
and cache from embedder. This should no longer be necessary, since
BrowsingDataRemover has been moved to content/ and can be used directly.
Embedders that need to execute additional deletions can still subclass
BrowsingDataRemoverDelegate and implement them there.

There were only two ContentBrowserClient subclasses that used
the methods:

1. ChromeContentBrowserClient::ClearCache() was used by dev_tools/.
This was replaced by an equivalent call to BrowsingDataRemover.

2. In android_webview, ClearCookies() and ClearCache() were defined,
but not called. ClearCache() used android_webview::RemoveHttpDiskCache
for cache deletion, which is in fact a subset of functionality that
already exists in BrowsingDataRemover, and can be replaced by it.

BUG=668114

Review-Url: https://codereview.chromium.org/2892953002
Cr-Commit-Position: refs/heads/master@{#473434}

[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/android_webview/BUILD.gn
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/android_webview/browser/aw_content_browser_client.h
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/android_webview/browser/aw_contents.cc
[delete] https://crrev.com/1b298605df3e00d8bb378fb2c17a8014d0d1c578/android_webview/browser/net_disk_cache_remover.cc
[delete] https://crrev.com/1b298605df3e00d8bb378fb2c17a8014d0d1c578/android_webview/browser/net_disk_cache_remover.h
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/content/browser/devtools/protocol/network_handler.cc
[modify] https://crrev.com/507bbc4d6eaf6ebd7b5ed1e63b611169e0d13308/content/public/browser/content_browser_client.h

Project Member

Comment 25 by bugdroid1@chromium.org, May 29 2017

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

commit 168dc2f3cb966091dcca1abb798b5c4ebd7c5437
Author: msramek <msramek@chromium.org>
Date: Mon May 29 19:01:27 2017

Define DATA_TYPE_SITE_DATA in terms of DATA_TYPE_DOM_STORAGE.

DATA_TYPE_SITE_DATA, i.e. "Cookies and site data" as presented to the
user in the Clear Browsing Data UI, is a superset of DATA_TYPE_DOM_STORAGE,
which represents the "storage" datatype in Clear-Site-Data.

Define it as such to avoid duplication.

Furthermore, this CL fixes a bug where DATA_TYPE_EMBEDDER_DOM_STORAGE,
one of the components of DATA_TYPE_DOM_STORAGE, was not a part of
DATA_TYPE_SITE_DATA. While this had no effect on DATA_TYPE_SITE_DATA,
it meant that DATA_TYPE_EMBEDDER_DOM_STORAGE was not part of
FILTERABLE_DATATYPES, making Clear-Site-Data for "storage" DCHECK().

BUG=668114

Review-Url: https://codereview.chromium.org/2910923002
Cr-Commit-Position: refs/heads/master@{#475370}

[modify] https://crrev.com/168dc2f3cb966091dcca1abb798b5c4ebd7c5437/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/168dc2f3cb966091dcca1abb798b5c4ebd7c5437/chrome/browser/extensions/api/browsing_data/browsing_data_test.cc

Project Member

Comment 26 by bugdroid1@chromium.org, May 31 2017

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

commit 86b6bc73feadfa473080ed1e7ec8aebd38a1cc68
Author: msramek <msramek@chromium.org>
Date: Wed May 31 12:21:10 2017

Implement GetBrowsingDataRemoverDelegate() for BrowserContext subclasses

As a part of the refactoring to move BrowsingDataRemover to content/,
the GetBrowsingDataRemoverDelegate() method has been defined in
BrowserContext with a default "return nulltpr" implementation, overriden
by ProfileImpl to return ChromeBrowsingDataRemoverDelegate.

However, similar methods in BrowserContext do not have a default
implementation, but are declared as pure virtual. We change this method
to be pure virtual as well.

ProfileImpl and OffTheRecordProfile use
ChromeBrowsingDataRemoverDelegate. So does TestingProfile, which
means that the helper TestingProfileWithDelegate can now be removed
from ChromeBrowsingDataRemoverDelegateTest. All other subclasses
return nullptr.

BUG=668114

Review-Url: https://codereview.chromium.org/2898443002
Cr-Commit-Position: refs/heads/master@{#475885}

[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/android_webview/browser/aw_browser_context.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/android_webview/browser/aw_browser_context.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/browser/profiles/off_the_record_profile_impl.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/browser/profiles/off_the_record_profile_impl.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/browser/ui/app_list/test/fake_profile.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/browser/ui/app_list/test/fake_profile.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chrome/test/base/testing_profile.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chromecast/browser/cast_browser_context.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/chromecast/browser/cast_browser_context.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/browser/browser_context.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/browser/download/download_manager_impl_unittest.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/public/browser/browser_context.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/public/test/test_browser_context.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/public/test/test_browser_context.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/shell/browser/shell_browser_context.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/content/shell/browser/shell_browser_context.h
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/headless/lib/browser/headless_browser_context_impl.cc
[modify] https://crrev.com/86b6bc73feadfa473080ed1e7ec8aebd38a1cc68/headless/lib/browser/headless_browser_context_impl.h

Sign in to add a comment