Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(38)

Issue 891763002: Remove dependency of HistoryService on WebHistoryServiceFactory (Closed)

Created:
5 years, 10 months ago by sdefresne
Modified:
5 years, 10 months ago
Reviewers:
Bernhard Bauer, droger
CC:
browser-components-watch_chromium.org, chromium-reviews, markusheintz_
Base URL:
https://chromium.googlesource.com/chromium/src.git@schedule-and-forget
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove dependency of HistoryService on WebHistoryServiceFactory Change HistoryService::ExpireLocalAndRemoteHistoryBetween() to receive the history::WebHistoryService instance to use in order to remove a dependency of HistoryService on Profile. It is not possible to pass the WebHistoryService instance to the HistoryService constructor and add a dependency between the two factories due to http://crbug.com/171406 (as WebHistoryService depends on a KeyedService calling Profile::GetRequestContext()). BUG=453790 R=droger@chromium.org Review URL: https://codereview.chromium.org/891763002 Cr-Commit-Position: refs/heads/master@{#314152}

Patch Set 1 #

Patch Set 2 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -12 lines) Patch
M chrome/browser/browsing_data/browsing_data_remover.cc View 1 3 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/history/history_service.h View 2 chunks +8 lines, -5 lines 0 comments Download
M chrome/browser/history/history_service.cc View 1 4 chunks +3 lines, -6 lines 0 comments Download

Messages

Total messages: 13 (4 generated)
sdefresne
5 years, 10 months ago (2015-01-30 16:08:30 UTC) #1
droger
lgtm
5 years, 10 months ago (2015-01-30 16:32:40 UTC) #2
sdefresne
bauerb: please review chrome/browser/browsing_data/browsing_data_remover.cc
5 years, 10 months ago (2015-01-30 22:35:15 UTC) #4
Bernhard Bauer
lgtm
5 years, 10 months ago (2015-02-02 14:41:56 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/891763002/20001
5 years, 10 months ago (2015-02-02 15:19:44 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 10 months ago (2015-02-02 15:51:14 UTC) #8
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/173da8215285a28458517ae5423f4ff23c8223cd Cr-Commit-Position: refs/heads/master@{#314152}
5 years, 10 months ago (2015-02-02 15:52:17 UTC) #9
Vitaly Buka (NO REVIEWS)
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/898473003/ by vitalybuka@chromium.org. ...
5 years, 10 months ago (2015-02-02 23:09:50 UTC) #10
Vitaly Buka (NO REVIEWS)
5 years, 10 months ago (2015-02-02 23:27:11 UTC) #11
On 2015/02/02 23:09:50, Vitaly Buka wrote:
> A revert of this CL (patchset #2 id:20001) has been created in
> https://codereview.chromium.org/898473003/ by mailto:vitalybuka@chromium.org.
> 
> The reason for reverting is: BUG=454524.

This CL was reverted only because of conflicts with
https://codereview.chromium.org/889553003/

Powered by Google App Engine
This is Rietveld 408576698