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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 642623004: Rename VisitDatabaseObserver to HistoryServiceObserver (Closed)

Created:
6 years, 2 months ago by sdefresne
Modified:
6 years, 2 months ago
Reviewers:
Shishir, *blundell
CC:
chromium-reviews, davidben+watch_chromium.org, cbentzel+watch_chromium.org, tburkard+watch_chromium.org, browser-components-watch_chromium.org, gavinp+prer_chromium.org, jkarlin+watch_chromium.org, brettw
Base URL:
https://chromium.googlesource.com/chromium/src.git@373326
Project:
chromium
Visibility:
Public.

Description

Rename VisitDatabaseObserver to HistoryServiceObserver In preparation of migration of HistoryService from notification to observers, rename VisitaDatabaseObserver to HistoryServiceObserver and move it into the history component. TBR=shishir@chromium.orf BUG=373326 Committed: https://crrev.com/1ef49e0231d7b3dd8f8f618681ce5a1de0f918a4 Cr-Commit-Position: refs/heads/master@{#299450}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rebase the patch on master #

Patch Set 3 : Address comments #

Patch Set 4 : Cleanup patch #

Patch Set 5 : Further cleanup of the patch #

Patch Set 6 : Rebase #

Total comments: 2

Patch Set 7 : Address comments & rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -69 lines) Patch
M chrome/browser/history/android/android_provider_backend_unittest.cc View 1 2 3 4 5 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/history/history_backend.h View 1 2 3 4 5 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/history/history_backend.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/history/history_backend_unittest.cc View 1 2 3 4 5 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/history/history_service.h View 1 2 3 4 5 5 chunks +13 lines, -9 lines 0 comments Download
M chrome/browser/history/history_service.cc View 1 2 3 4 5 6 5 chunks +20 lines, -24 lines 0 comments Download
M chrome/browser/history/history_unittest.cc View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/prerender/prerender_local_predictor.h View 1 2 3 4 4 chunks +7 lines, -5 lines 0 comments Download
M chrome/browser/prerender/prerender_local_predictor.cc View 1 2 3 4 3 chunks +9 lines, -8 lines 0 comments Download
M components/history.gypi View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M components/history/core/browser/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
A components/history/core/browser/history_service_observer.h View 1 2 3 4 1 chunk +30 lines, -0 lines 0 comments Download
M components/history/core/browser/history_types.h View 1 chunk +0 lines, -7 lines 0 comments Download
M components/history/core/browser/history_types.cc View 1 chunk +0 lines, -4 lines 0 comments Download

Messages

Total messages: 13 (3 generated)
sdefresne
blundell: As discussed this is a followup to https://codereview.chromium.org/631253002/ to merge HistoryServiceObserver and VisitDatabaseObserver. shishir: ...
6 years, 2 months ago (2014-10-09 10:10:57 UTC) #2
blundell
LGTM, thanks https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h File chrome/browser/history/history_backend.h (right): https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h#newcode134 chrome/browser/history/history_backend.h:134: // Notify HistoryService that VisitDatabase was changed. ...
6 years, 2 months ago (2014-10-09 12:02:23 UTC) #3
sdefresne
https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h File chrome/browser/history/history_backend.h (right): https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h#newcode134 chrome/browser/history/history_backend.h:134: // Notify HistoryService that VisitDatabase was changed. The even ...
6 years, 2 months ago (2014-10-13 17:27:03 UTC) #4
sdefresne
On 2014/10/13 17:27:03, sdefresne wrote: > https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h > File chrome/browser/history/history_backend.h (right): > > https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h#newcode134 > ...
6 years, 2 months ago (2014-10-13 17:28:39 UTC) #5
sdefresne
On 2014/10/13 17:28:39, sdefresne wrote: > On 2014/10/13 17:27:03, sdefresne wrote: > > > https://codereview.chromium.org/642623004/diff/1/chrome/browser/history/history_backend.h ...
6 years, 2 months ago (2014-10-13 17:28:53 UTC) #6
blundell
LGTM https://codereview.chromium.org/642623004/diff/230001/chrome/browser/history/history_service.cc File chrome/browser/history/history_service.cc (right): https://codereview.chromium.org/642623004/diff/230001/chrome/browser/history/history_service.cc#newcode336 chrome/browser/history/history_service.cc:336: void HistoryService::AddObserver(history::HistoryServiceObserver* observer) { DCHECK called on valid ...
6 years, 2 months ago (2014-10-14 06:53:33 UTC) #7
sdefresne
https://codereview.chromium.org/642623004/diff/230001/chrome/browser/history/history_service.cc File chrome/browser/history/history_service.cc (right): https://codereview.chromium.org/642623004/diff/230001/chrome/browser/history/history_service.cc#newcode336 chrome/browser/history/history_service.cc:336: void HistoryService::AddObserver(history::HistoryServiceObserver* observer) { On 2014/10/14 06:53:32, blundell wrote: ...
6 years, 2 months ago (2014-10-14 08:20:58 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/642623004/400001
6 years, 2 months ago (2014-10-14 08:22:06 UTC) #11
commit-bot: I haz the power
Committed patchset #7 (id:400001)
6 years, 2 months ago (2014-10-14 09:21:11 UTC) #12
commit-bot: I haz the power
6 years, 2 months ago (2014-10-14 09:22:33 UTC) #13
Message was sent while issue was closed. 
Patchset 7 (id:??) landed as
https://crrev.com/1ef49e0231d7b3dd8f8f618681ce5a1de0f918a4
Cr-Commit-Position: refs/heads/master@{#299450}

Powered by Google App Engine
This is Rietveld 408576698