New issue
Advanced search Search tips

Issue 748484 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Make ChromeBrowsingDataRemoverDelegate usable in unittest

Project Member Reported by msramek@chromium.org, Jul 25 2017

Issue description

ChromeBrowsingDataRemoverDelegate is used by BrowsingDataRemover to delete various data storage backends in chrome/.

Unittests in chrome/ should always be able to use BrowsingDataRemover to test deletion, which means that the real ChromeBrowsingDataRemoverDelegate should always be usable in unittests, without the need to mock it out or to inject nullptr to BrowsingDataRemover.

This is currently not possible due to some dependencies (data storage backends) which can not be used in a unittest environment, see e.g. https://cs.chromium.org/chromium/src/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc?l=1039
 
Cc: bsazonov@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 15 2018

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

commit 8d297d6d7a3ea57b49fec20a4ab0c7419ca529be
Author: Christian Dullweber <dullweber@chromium.org>
Date: Thu Mar 15 13:39:21 2018

Delete bookmarks on profile deletion

When a profile is deleted, all browsing_data is removed and the folder
is marked for deletion on shutdown. Bookmarks are currently not deleted
until the folder is deleted. As bookmarks can contains sensitive data
they should also be removed immediately.
This also changes SigninManagerAndroidTest to use
ChromeBrowsingDataRemoverDelegate.

Bug: 803643, 748484
Change-Id: Icebfcec44d61d8b38939a8cee8dc4a92b6f9e258
Reviewed-on: https://chromium-review.googlesource.com/957040
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543358}
[modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/android/signin/signin_manager_android.cc
[modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/android/signin/signin_manager_android_unittest.cc
[modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
[modify] https://crrev.com/8d297d6d7a3ea57b49fec20a4ab0c7419ca529be/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

I had to figure out how to use the ChromeBrowsingDataRemoverDelegate in unittests on Android. The current issues are DownloadManager and Pnacl (only desktop).

DownloadManager needs a ChromeDownloadManagerDelegate. 
PnaclHost has a InitForTesting method that allows it to be used in tests. 

The best way would probably be if there were no DownloadManager or PnaclHost instances or if they would be replaced by a mock unless the real instance is explicitly created for the test.

Here is a CL with the current status: https://crrev.com/c/964262
Cc: dullweber@chromium.org
Owner: dullweber@chromium.org

Sign in to add a comment