New issue
Advanced search Search tips

Issue 843995 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 25
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove dead code and improve test coverage for browsing_data

Project Member Reported by dullweber@chromium.org, May 17 2018

Issue description

Tracking bug for code cleanup and improvement

First CL: https://crrev.com/c/1061500
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 23 2018

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 11 2018

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

commit 3c99940b28a021ac262aa3020a9e33114126fb47
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon Jun 11 15:21:56 2018

Remove dead code from CookieTreeModel test

No one is calling these methods, instead GetDisplayNodes() is used.

Bug:  843995 
Change-Id: I76d5a711527eca20497fdc283ca55e47683b37a3
Reviewed-on: https://chromium-review.googlesource.com/1090283
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566005}
[modify] https://crrev.com/3c99940b28a021ac262aa3020a9e33114126fb47/chrome/browser/browsing_data/cookies_tree_model_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 15 2018

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

commit a7940330ec326a06505ef05a03f127633d786e29
Author: Christian Dullweber <dullweber@chromium.org>
Date: Fri Jun 15 08:11:06 2018

Parameterize BrowsingDataRemoverBrowserTests with time range

Currently BrowsingDataRemoverBrowserTests tests deletion for all time.
A few data types are deleted incorrectly when deleting a time range.
To catch this in the future, the tests are parameterized to run for
"All time" and for "Last Hour"

Bug:  843995 
Change-Id: Ib7185423ba002ed3f2efc3398e9c0583f3134d98
Reviewed-on: https://chromium-review.googlesource.com/1100880
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567574}
[modify] https://crrev.com/a7940330ec326a06505ef05a03f127633d786e29/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 18 2018

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

commit e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon Jun 18 15:03:30 2018

Convert BrowsingDataCookieHelper to use CookieManager

Make BrowsingDataCookieHelper compatible with the network service
by converting it to use CookieManager.

Bug:  843995 
Change-Id: I92db772d7ab63ca51fbaabd618f6b4d77fd784b3
Reviewed-on: https://chromium-review.googlesource.com/1101022
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568007}
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/android/preferences/website_preference_bridge.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/browsing_data_cookie_helper.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/browsing_data_cookie_helper.h
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/browsing_data_cookie_helper_unittest.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/cookies_tree_model_unittest.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/mock_browsing_data_cookie_helper.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/mock_browsing_data_cookie_helper.h
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/browsing_data/site_data_size_collector_unittest.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/content_settings/local_shared_objects_container.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/sessions/session_data_deleter.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/chrome/browser/ui/webui/settings/settings_cookies_view_handler.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/content/browser/devtools/protocol/network_handler.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/services/network/cookie_manager.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/services/network/cookie_manager.h
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/services/network/cookie_manager_unittest.cc
[modify] https://crrev.com/e5de5b8e0b9455c0aa62b2cec05a9cfcf734fb74/services/network/public/mojom/cookie_manager.mojom

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 19 2018

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

commit fc8fbc21a5135d16313ed1234d41ad4463e53c6e
Author: Christian Dullweber <dullweber@chromium.org>
Date: Tue Jun 19 10:05:25 2018

Add integration test coverage of cookie tree model

Test that the CookieTreeModel behaves the same as the site data
counter.

Bug:  843995 
Change-Id: Ib226c829dec5a242fca22222961b99c398703d1d
Reviewed-on: https://chromium-review.googlesource.com/1100827
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568386}
[modify] https://crrev.com/fc8fbc21a5135d16313ed1234d41ad4463e53c6e/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 3

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

commit ed8282b7021225ad4dc5ca86562c3fde6a8d4d06
Author: Christian Dullweber <dullweber@chromium.org>
Date: Tue Jul 03 08:28:37 2018

Test session storage deletion

Session Storage is currently not properly tested by
BrowsingDataRemoverBrowserTest because it is not supported by the
SiteDataCounter and the CookieTreeModel. This Cl adds an integration
test for its web-visible behavior.

Bug:  843995 
Change-Id: I3eac6903995e4b4773bbcc08bc3f551001a4cdbb
Reviewed-on: https://chromium-review.googlesource.com/1113306
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572144}
[modify] https://crrev.com/ed8282b7021225ad4dc5ca86562c3fde6a8d4d06/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 9

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

commit 734dc747c4c3b420083b02e516e9988cc7f97095
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon Jul 09 08:54:34 2018

Add test for SessionOnly site data

SessionOnly data deletions are currently not covered by end to end
tests. This CL adds a test for deletion of Cookies, LocalStorage,
SessionStorage, WebSql and ServiceWorkers.

FileSystem, IndexedDb and Cache Storage don't work correctly, so they
are not covered yet. (crbug.com/824533 and  crbug.com/840080 ).

This also improves the CookiesTreeModel expectations to be more
precise by counting datatypes with data per host and not just the number
of hosts with data.

Bug:  843995 

Change-Id: I85c810c2178051c191e8287fa22bcccd2eb5060d
Reviewed-on: https://chromium-review.googlesource.com/1117179
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573245}
[modify] https://crrev.com/734dc747c4c3b420083b02e516e9988cc7f97095/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 23

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

commit 5f6cf299f66805f0fe647531d74dd862c93ea178
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon Jul 23 09:14:03 2018

Use empty network filter for deletions without filtering

When deleting with an EmptyBlacklist filter, we shouldn't create a
Network deletion filter to properly use optimized "delete all" code paths.

Bug:  843995 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ifc84f403dc2f2b50471e7fab273d20ebcba3f4f4
Reviewed-on: https://chromium-review.googlesource.com/1068736
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577127}
[modify] https://crrev.com/5f6cf299f66805f0fe647531d74dd862c93ea178/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/5f6cf299f66805f0fe647531d74dd862c93ea178/content/browser/browsing_data/browsing_data_filter_builder_impl.cc
[modify] https://crrev.com/5f6cf299f66805f0fe647531d74dd862c93ea178/content/browser/browsing_data/browsing_data_remover_impl.cc
[modify] https://crrev.com/5f6cf299f66805f0fe647531d74dd862c93ea178/content/public/browser/browsing_data_filter_builder.h
[modify] https://crrev.com/5f6cf299f66805f0fe647531d74dd862c93ea178/services/network/public/mojom/network_context.mojom

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 31

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

commit 8817f77a3271531c0b3320abccd13bf5de0901d2
Author: John Rummell <jrummell@chromium.org>
Date: Fri Aug 31 18:12:59 2018

Update BrowsingDataRemoverBrowserTest to include MediaLicenses

MediaLicenses can be cleared from the ClearBrowsingData dialog, so update the
BrowsingDataRemoverBrowserTest to check them as well. This uses the test-only
External Clear Key CDM to store the license in the file system, if it is
available.

BUG= 843995 ,808690
TEST=new browser_tests pass

Change-Id: Ib4019e6357f04ec6005cfda039ab81c06ab022b5
Reviewed-on: https://chromium-review.googlesource.com/1170175
Commit-Queue: John Rummell <jrummell@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588090}
[modify] https://crrev.com/8817f77a3271531c0b3320abccd13bf5de0901d2/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[add] https://crrev.com/8817f77a3271531c0b3320abccd13bf5de0901d2/content/test/data/browsing_data/media_license.html

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 3

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

commit 72f2d4cd06bcaaad1a8f91a9b7b5559ecf70b8d9
Author: Jan Wilken Dörrie <jdoerrie@chromium.org>
Date: Mon Sep 03 10:29:48 2018

Revert "Update BrowsingDataRemoverBrowserTest to include MediaLicenses"

This reverts commit 8817f77a3271531c0b3320abccd13bf5de0901d2.

Reason for revert: Flaky on Mac ( https://crbug.com/879812 ).

Original change's description:
> Update BrowsingDataRemoverBrowserTest to include MediaLicenses
> 
> MediaLicenses can be cleared from the ClearBrowsingData dialog, so update the
> BrowsingDataRemoverBrowserTest to check them as well. This uses the test-only
> External Clear Key CDM to store the license in the file system, if it is
> available.
> 
> BUG= 843995 ,808690
> TEST=new browser_tests pass
> 
> Change-Id: Ib4019e6357f04ec6005cfda039ab81c06ab022b5
> Reviewed-on: https://chromium-review.googlesource.com/1170175
> Commit-Queue: John Rummell <jrummell@chromium.org>
> Reviewed-by: Martin Šrámek <msramek@chromium.org>
> Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#588090}

TBR=xhwang@chromium.org,jrummell@chromium.org,msramek@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843995 , 808690
Change-Id: I9eefaabb06b959e5f4abbbc92c73777bf3951b14
Reviewed-on: https://chromium-review.googlesource.com/1201851
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588367}
[modify] https://crrev.com/72f2d4cd06bcaaad1a8f91a9b7b5559ecf70b8d9/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[delete] https://crrev.com/7421e54c4f52523bd9e330f331048a5951805b82/content/test/data/browsing_data/media_license.html

Status: Fixed (was: Started)
There are no further plans for improved testing so I'm closing this bug

Sign in to add a comment