New issue
Advanced search Search tips

Issue 839798 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove duplicated HistoryServiceObserver::OnURLsDeleted method

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

Issue description

There are currently two versions of HistoryServiceObserver::OnURLsDeleted. 
Remove the old method and change the new one to use a struct to pass deletion information to history observers.
 
Project Member

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

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

commit 73cedc1c0e054ac516f0130e81e8e34bdd17aae6
Author: Christian Dullweber <dullweber@chromium.org>
Date: Tue May 08 12:21:41 2018

Change HistoryServiceObserver::OnUrlsDeleted to pass info as a struct

Introduce a class that hold deletion related information to pass it to
history observer. This will simplify future changes and the deprecation
of the old OnUrlsDeleted method.
Also migrate Media- and SiteEngagementService to the new method.

Bug:  839798 
Change-Id: I77ded53325eb098f0ce156d24e0cb48a9166d0f5
Reviewed-on: https://chromium-review.googlesource.com/1043751
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556765}
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/browsing_data/browsing_data_history_observer_service.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/browsing_data/browsing_data_history_observer_service.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/engagement/site_engagement_service.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/engagement/site_engagement_service.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/history/android/android_provider_backend.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/history/android/android_provider_backend_unittest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/media/media_engagement_browsertest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/media/media_engagement_service.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/media/media_engagement_service.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/media/media_engagement_service_unittest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/resource_coordinator/local_site_characteristics_data_store.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/resource_coordinator/local_site_characteristics_data_store.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/chrome/browser/resource_coordinator/local_site_characteristics_data_store_unittest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/expire_history_backend.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/expire_history_backend_unittest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_backend.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_backend_notifier.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_backend_unittest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_service.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_service.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_service_observer.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_types.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/history_types.h
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/browser/typed_url_sync_bridge_unittest.cc
[modify] https://crrev.com/73cedc1c0e054ac516f0130e81e8e34bdd17aae6/components/history/core/test/history_backend_db_base_test.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 14 2018

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

commit b294bdc5d5dd74b05f177d21e4e4c2f41120f16e
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon May 14 08:58:45 2018

Migrate HistoryServiceObservers in components/history/

Change HistoryServiceObservers to use the new OnUrlsDeleted method.

Bug:  839798 
Change-Id: Ia5e01bcf60a43d7b1112f69f816a06e9ae1e51ab
Reviewed-on: https://chromium-review.googlesource.com/1051767
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558232}
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/browsing_history_service.cc
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/browsing_history_service.h
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/history_backend_unittest.cc
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/in_memory_history_backend.cc
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/in_memory_history_backend.h
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/top_sites_impl.cc
[modify] https://crrev.com/b294bdc5d5dd74b05f177d21e4e4c2f41120f16e/components/history/core/browser/top_sites_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, May 14 2018

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

commit b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon May 14 09:04:56 2018

Migrate HistoryServiceObserver::OnUrlsDeleted

Change HistoryServiceObservers to use the new OnUrlsDeleted method for
predictors and favicon_cache.

Bug:  839798 
Change-Id: Ibcf4d48eabe3649282c2246be73e8a8d263dc98d
Reviewed-on: https://chromium-review.googlesource.com/1051888
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558233}
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/chrome/browser/predictors/autocomplete_action_predictor.cc
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/chrome/browser/predictors/autocomplete_action_predictor.h
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/chrome/browser/predictors/resource_prefetch_predictor.cc
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/chrome/browser/predictors/resource_prefetch_predictor.h
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/components/sync_sessions/favicon_cache.cc
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/components/sync_sessions/favicon_cache.h
[modify] https://crrev.com/b2eb8c94961ed9b5f6e9d58501785e72dd6a3a15/components/sync_sessions/favicon_cache_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 14 2018

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

commit d31e1815c58990ff2633fc009ef4ffa2f2749370
Author: Christian Dullweber <dullweber@chromium.org>
Date: Mon May 14 10:30:26 2018

Migrate HistoryServiceObserver in components/omnibox/

Change HistoryServiceObservers to use the new OnUrlsDeleted method.

Bug:  839798 
Change-Id: I01d159fdac5f0cf6463a62de76923c0658325da7
Reviewed-on: https://chromium-review.googlesource.com/1051907
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558250}
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/favicon_cache.cc
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/favicon_cache.h
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/favicon_cache_unittest.cc
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/history_quick_provider_unittest.cc
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/in_memory_url_index.cc
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/in_memory_url_index.h
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/in_memory_url_index_unittest.cc
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/shortcuts_backend.cc
[modify] https://crrev.com/d31e1815c58990ff2633fc009ef4ffa2f2749370/components/omnibox/browser/shortcuts_backend.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 16 2018

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

commit 3446489292f64bb46e918835693aca9894e972a4
Author: Christian Dullweber <dullweber@chromium.org>
Date: Wed May 16 07:52:17 2018

Migrate HistoryServiceObserver::OnUrlsDeleted

Change HistoryServiceObservers to use the new OnUrlsDeleted method.

Bug:  839798 
Change-Id: Ibc62d194a7905e2fe357c098e77dd5db486e89ff
Reviewed-on: https://chromium-review.googlesource.com/1059250
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559009}
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/chrome/browser/extensions/api/history/history_api.cc
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/chrome/browser/extensions/api/history/history_api.h
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/ntp_snippets/content_suggestions_service.cc
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/ntp_snippets/content_suggestions_service.h
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/ntp_snippets/content_suggestions_service_unittest.cc
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/safe_browsing/password_protection/password_protection_service.cc
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/safe_browsing/password_protection/password_protection_service.h
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/ukm/observers/history_delete_observer.cc
[modify] https://crrev.com/3446489292f64bb46e918835693aca9894e972a4/components/ukm/observers/history_delete_observer.h

Project Member

Comment 6 by bugdroid1@chromium.org, May 17 2018

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

commit ace896ebf017438c73f0bf668e5c52fa6ec29c8f
Author: Christian Dullweber <dullweber@chromium.org>
Date: Thu May 17 14:08:20 2018

Use new OnURLsDeleted method for Subresourcefilter and improve deletion

Migrate SubresourceFilterContentSettingsManager to the new
OnUrlsDeleted method and improve deletion of the setting that stores
whether the ui has been shown to only be deleted if all urls of
an origin have been have been removed.

Bug:  839798 
Change-Id: I74a9ec6206a07f5925eee8de2a0790976aa09221
Reviewed-on: https://chromium-review.googlesource.com/1063727
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559519}
[modify] https://crrev.com/ace896ebf017438c73f0bf668e5c52fa6ec29c8f/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.cc
[modify] https://crrev.com/ace896ebf017438c73f0bf668e5c52fa6ec29c8f/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
[modify] https://crrev.com/ace896ebf017438c73f0bf668e5c52fa6ec29c8f/chrome/browser/subresource_filter/subresource_filter_content_settings_manager_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 22 2018

Project Member

Comment 8 by bugdroid1@chromium.org, May 22 2018

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

commit d640995deb179cac745b4520167b854153a96893
Author: Christian Dullweber <dullweber@chromium.org>
Date: Tue May 22 10:17:40 2018

Remove old HistoryServiceObserver::OnUrlsDeleted method

All observers have been migrated to the new method, so the old one can
be removed.

Bug:  839798 
Change-Id: I2c0e89d460d53549cbb5253cc02712250458f0d9
Reviewed-on: https://chromium-review.googlesource.com/1064234
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560525}
[modify] https://crrev.com/d640995deb179cac745b4520167b854153a96893/components/history/core/browser/history_service.cc
[modify] https://crrev.com/d640995deb179cac745b4520167b854153a96893/components/history/core/browser/history_service_observer.h

Status: Fixed (was: Started)
done

Sign in to add a comment