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

Issue 2502113002: [Download Home] More cleaning (Closed)

Created:
4 years, 1 month ago by gone
Modified:
4 years, 1 month ago
Reviewers:
Theresa
CC:
chromium-reviews, asanka, agrieve+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Download Home] More cleaning * More uniformly handle how each of the lists of downloaded items from the three backends are manipulated. * Add a new BackendItems class to encapsulate more of the list manipulating behavior. * Pull out the LoadingStateDelegate into its own file. * Stop differentiating between DownloadItemWrappers and OfflineItemWrappers unless it's necessary. BUG=654630 Committed: https://crrev.com/b5e76d744d772f1ae8d6fe9b929c0a2257c258ef Cr-Commit-Position: refs/heads/master@{#432918}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Comment #

Messages

Total messages: 14 (6 generated)
gone
Mostly moving code, but there's some adjusted logic in here to try and consolidate everything. ...
4 years, 1 month ago (2016-11-15 22:56:32 UTC) #2
Theresa
lgtm https://codereview.chromium.org/2502113002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java File chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java (right): https://codereview.chromium.org/2502113002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java#newcode282 chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:282: void readdItemsToAdapter(List<DownloadHistoryItemWrapper> items) { nit: I initially read ...
4 years, 1 month ago (2016-11-17 00:42:33 UTC) #3
gone
https://codereview.chromium.org/2502113002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java File chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java (right): https://codereview.chromium.org/2502113002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java#newcode282 chrome/android/java/src/org/chromium/chrome/browser/download/ui/DownloadHistoryAdapter.java:282: void readdItemsToAdapter(List<DownloadHistoryItemWrapper> items) { On 2016/11/17 00:42:33, Theresa Wellington ...
4 years, 1 month ago (2016-11-17 01:03:54 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2502113002/20001
4 years, 1 month ago (2016-11-17 01:06:03 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_TIMED_OUT, no build URL) android_clang_dbg_recipe on ...
4 years, 1 month ago (2016-11-17 03:09:48 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2502113002/20001
4 years, 1 month ago (2016-11-17 17:52:03 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-11-17 18:26:40 UTC) #12
commit-bot: I haz the power
4 years, 1 month ago (2016-11-17 18:32:57 UTC) #14
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/b5e76d744d772f1ae8d6fe9b929c0a2257c258ef
Cr-Commit-Position: refs/heads/master@{#432918}

Powered by Google App Engine
This is Rietveld 408576698