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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 870143004: HistoryService::Init() receives the list of languages (Closed)

Created:
5 years, 10 months ago by sdefresne
Modified:
5 years, 10 months ago
CC:
browser-components-watch_chromium.org, chromium-reviews, grt+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@web-history-service
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

HistoryService::Init() receives the list of languages Pass the list of languages to HistoryService::Init() in order to remove a dependency on prefs::kAcceptLanguages and on Profile. BUG=453790 R=droger@chromium.org Committed: https://crrev.com/c6ca44530c782348b6d49f5221b5678708353bb8 Cr-Commit-Position: refs/heads/master@{#314779}

Patch Set 1 #

Total comments: 1

Patch Set 2 : Remove unnecessary "explicit" #

Patch Set 3 : Change Init() to receive the languages directly and remove the dependency on prefs::kAcceptLanguages #

Patch Set 4 : Rebase #

Total comments: 1

Patch Set 5 : Rebase #

Total comments: 1

Patch Set 6 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -24 lines) Patch
M chrome/browser/history/history_backend_unittest.cc View 1 2 3 chunks +4 lines, -1 line 0 comments Download
M chrome/browser/history/history_querying_unittest.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/history/history_service.h View 1 2 3 4 5 4 chunks +8 lines, -4 lines 0 comments Download
M chrome/browser/history/history_service.cc View 1 2 3 4 5 6 chunks +7 lines, -15 lines 0 comments Download
M chrome/browser/history/history_service_factory.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/history/history_unittest.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector_unittest.cc View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/test/base/testing_profile.cc View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 18 (5 generated)
sdefresne
5 years, 10 months ago (2015-01-30 16:08:56 UTC) #1
droger
lgtm
5 years, 10 months ago (2015-01-30 16:28:52 UTC) #2
droger
https://codereview.chromium.org/870143004/diff/1/chrome/browser/history/history_service.h File chrome/browser/history/history_service.h (right): https://codereview.chromium.org/870143004/diff/1/chrome/browser/history/history_service.h#newcode95 chrome/browser/history/history_service.h:95: explicit HistoryService(history::HistoryClient* client, Profile* profile); Can you remove |profile| ...
5 years, 10 months ago (2015-01-30 16:31:14 UTC) #3
sdefresne
On 2015/01/30 at 16:31:14, droger wrote: > https://codereview.chromium.org/870143004/diff/1/chrome/browser/history/history_service.h > File chrome/browser/history/history_service.h (right): > > https://codereview.chromium.org/870143004/diff/1/chrome/browser/history/history_service.h#newcode95 ...
5 years, 10 months ago (2015-01-30 21:14:40 UTC) #4
sdefresne
I changed the implementation of HistoryService::Init() to instead take the list of languages (since there ...
5 years, 10 months ago (2015-01-30 22:29:35 UTC) #5
droger
LGTM with comment nits. https://codereview.chromium.org/870143004/diff/60001/chrome/browser/history/history_service.cc File chrome/browser/history/history_service.cc (right): https://codereview.chromium.org/870143004/diff/60001/chrome/browser/history/history_service.cc#newcode984 chrome/browser/history/history_service.cc:984: // During unit testing, there ...
5 years, 10 months ago (2015-02-04 16:42:46 UTC) #6
sdefresne
On 2015/02/04 at 16:42:46, droger wrote: > LGTM with comment nits. > > https://codereview.chromium.org/870143004/diff/60001/chrome/browser/history/history_service.cc > ...
5 years, 10 months ago (2015-02-04 17:49:51 UTC) #8
sdefresne
jcivelli: OWNERS for //chrome/test/base/testing_profile.h grt: OWNERS for //chrome/browser/safe_browsing
5 years, 10 months ago (2015-02-04 17:51:59 UTC) #12
grt (UTC plus 2)
On 2015/02/04 17:51:59, sdefresne wrote: > jcivelli: OWNERS for //chrome/test/base/testing_profile.h > grt: OWNERS for //chrome/browser/safe_browsing ...
5 years, 10 months ago (2015-02-04 18:34:44 UTC) #13
Jay Civelli
LGTM for chrome/test/base/testing_profile.cc
5 years, 10 months ago (2015-02-04 20:33:50 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/870143004/100001
5 years, 10 months ago (2015-02-05 09:14:58 UTC) #16
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 10 months ago (2015-02-05 09:18:41 UTC) #17
commit-bot: I haz the power
5 years, 10 months ago (2015-02-05 09:20:04 UTC) #18
Message was sent while issue was closed. 
Patchset 6 (id:??) landed as
https://crrev.com/c6ca44530c782348b6d49f5221b5678708353bb8
Cr-Commit-Position: refs/heads/master@{#314779}

Powered by Google App Engine
This is Rietveld 408576698