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

Issue 1904613005: [Offline pages] Removing bookmarks dependency from offline pages component (Closed)

Created:
4 years, 8 months ago by fgorski
Modified:
4 years, 8 months ago
Reviewers:
sky, jianli
CC:
chromium-reviews, cbentzel+watch_chromium.org, blundell+watchlist_chromium.org, droger+watchlist_chromium.org, fgorski+watch_chromium.org, sdefresne+watchlist_chromium.org, browser-components-watch_chromium.org, noyau+watch_chromium.org, tfarina, dimich+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Offline pages] Removing bookmarks dependency from offline pages component * Removing offline_page_bookmark_bridge and related tests * Updating constants' names * Updating constants' names in tests * Removing bookmarks from build files and DEPS Given that we no longer are bound to bookmarks or bookmarks UI there is no point in tracking existence thereof. When bookmark is deleted the offline copy is still around, until it expires. Once it does we clean it up. BUG=603940 Committed: https://crrev.com/0b9ea192c1b2b54fededc722305892d1c2fcb2b9 Cr-Commit-Position: refs/heads/master@{#388995}

Patch Set 1 #

Patch Set 2 : Fix unit tests compilation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -448 lines) Patch
M chrome/browser/android/offline_pages/offline_page_tab_helper_unittest.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/android/offline_pages/offline_page_utils.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/offline_pages/offline_page_utils_unittest.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/bookmarks/bookmark_model_factory.cc View 3 chunks +0 lines, -20 lines 0 comments Download
M chrome/browser/net/net_error_tab_helper.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/offline_pages.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M components/offline_pages/BUILD.gn View 2 chunks +0 lines, -4 lines 0 comments Download
M components/offline_pages/DEPS View 1 chunk +0 lines, -1 line 0 comments Download
D components/offline_pages/offline_page_bookmark_bridge.h View 1 chunk +0 lines, -46 lines 0 comments Download
D components/offline_pages/offline_page_bookmark_bridge.cc View 1 chunk +0 lines, -85 lines 0 comments Download
M components/offline_pages/offline_page_metadata_store_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/offline_pages/offline_page_metadata_store_impl_unittest.cc View 7 chunks +10 lines, -9 lines 0 comments Download
M components/offline_pages/offline_page_model.h View 1 chunk +2 lines, -2 lines 0 comments Download
M components/offline_pages/offline_page_model.cc View 2 chunks +1 line, -3 lines 0 comments Download
M components/offline_pages/offline_page_model_unittest.cc View 42 chunks +73 lines, -270 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 18 (9 generated)
fgorski
PTAL.
4 years, 8 months ago (2016-04-21 17:44:00 UTC) #3
jianli
lgtm
4 years, 8 months ago (2016-04-21 20:45:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904613005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904613005/20001
4 years, 8 months ago (2016-04-21 21:57:37 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/171672)
4 years, 8 months ago (2016-04-21 22:05:56 UTC) #9
fgorski
sky@ can you please take a look? This patch is de-cluttering bookmarks and offline pages ...
4 years, 8 months ago (2016-04-21 22:09:22 UTC) #11
sky
LGTM
4 years, 8 months ago (2016-04-21 23:38:42 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1904613005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1904613005/20001
4 years, 8 months ago (2016-04-22 01:13:22 UTC) #14
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2016-04-22 01:18:28 UTC) #16
commit-bot: I haz the power
4 years, 8 months ago (2016-04-22 19:43:14 UTC) #18
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/0b9ea192c1b2b54fededc722305892d1c2fcb2b9
Cr-Commit-Position: refs/heads/master@{#388995}

Powered by Google App Engine
This is Rietveld 408576698