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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 15275004: bookmarks: Get rid of the dependency on history_notifications.h (Closed)

Created:
7 years, 7 months ago by tfarina
Modified:
7 years, 6 months ago
Reviewers:
sky
CC:
chromium-reviews, akalin, tfarina, browser-components-watch_chromium.org, Raghu Simha, haitaol1, tim (not reviewing), Jói
Visibility:
Public.

Description

bookmarks: Get rid of the dependency on history_notifications.h In order to do this we moved the FaviconChangeDetails struct into c/b/favicon, which bookmarks is already allowed to depend on, as well is history. BUG=144783 R=sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202587

Patch Set 1 : #

Patch Set 2 : sync+rebase #

Patch Set 3 : fixes - link #

Patch Set 4 : fix HistoryBackendTest.SetFaviconsDeleteBitmaps #

Patch Set 5 : mv BookmarkModelTestWithProfile.RemoveNotification to HistoryBackendTest #

Patch Set 6 : fix DEPS #

Patch Set 7 : fix HistoryBackendTest.SetFaviconsReplaceBitmapData #

Patch Set 8 : unit_tests fixed #

Patch Set 9 : fix android #

Patch Set 10 : fix leak #

Patch Set 11 : fix win #

Patch Set 12 : more android fixes #

Patch Set 13 : SendFaviconChangedNotifications #

Patch Set 14 : android fix #

Patch Set 15 : more android fixes #

Total comments: 3

Patch Set 16 : inherit from HistoryDetails #

Patch Set 17 : add files back #

Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -72 lines) Patch
M chrome/browser/bookmarks/DEPS View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/bookmarks/bookmark_model.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/bookmarks/bookmark_model_unittest.cc View 1 2 3 4 2 chunks +0 lines, -27 lines 0 comments Download
M chrome/browser/favicon/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
A chrome/browser/favicon/favicon_changed_details.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +21 lines, -0 lines 0 comments Download
A + chrome/browser/favicon/favicon_changed_details.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +3 lines, -5 lines 0 comments Download
M chrome/browser/history/android/android_provider_backend.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 5 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/history/android/android_provider_backend_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 4 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/history/history_backend.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/history/history_backend_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 5 chunks +35 lines, -1 line 0 comments Download
A chrome/browser/history/history_details.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +20 lines, -0 lines 0 comments Download
M chrome/browser/history/history_notifications.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +2 lines, -18 lines 0 comments Download
M chrome/browser/history/history_notifications.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/sync/glue/session_change_processor.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/common/chrome_notification_types.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
tfarina
7 years, 7 months ago (2013-05-18 00:18:23 UTC) #1
sky
https://codereview.chromium.org/15275004/diff/29001/chrome/browser/favicon/favicon_changed_details.h File chrome/browser/favicon/favicon_changed_details.h (right): https://codereview.chromium.org/15275004/diff/29001/chrome/browser/favicon/favicon_changed_details.h#newcode15 chrome/browser/favicon/favicon_changed_details.h:15: struct FaviconChangedDetails { It seems as though favicons is ...
7 years, 7 months ago (2013-05-20 14:48:13 UTC) #2
tfarina
https://codereview.chromium.org/15275004/diff/29001/chrome/browser/favicon/favicon_changed_details.h File chrome/browser/favicon/favicon_changed_details.h (right): https://codereview.chromium.org/15275004/diff/29001/chrome/browser/favicon/favicon_changed_details.h#newcode15 chrome/browser/favicon/favicon_changed_details.h:15: struct FaviconChangedDetails { On 2013/05/20 14:48:13, sky wrote: > ...
7 years, 7 months ago (2013-05-20 23:10:30 UTC) #3
tfarina
Scott, this is ready for another look. Please, could you take another look? Thanks,
7 years, 7 months ago (2013-05-26 15:09:46 UTC) #4
sky
LGTM
7 years, 6 months ago (2013-05-28 14:15:49 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tfarina@chromium.org/15275004/40001
7 years, 6 months ago (2013-05-28 15:13:48 UTC) #6
commit-bot: I haz the power
7 years, 6 months ago (2013-05-28 17:01:10 UTC) #7
Message was sent while issue was closed. 
Change committed as 202587

Powered by Google App Engine
This is Rietveld 408576698