Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2863863002:
Fix duplicate space calculation on Android download home. (Closed)
DescriptionFix duplicate space calculation on Android download home.
Check duplicate file paths when calculate disk space shown in UI.
Current Android code doesn't generate duplicate path, but we still need
this fix for existing records.
I think we shouldn't filter duplicate entries however, since the download
history is not the same concept as a list of files on disk.
BUG=639911
Review-Url: https://codereview.chromium.org/2863863002
Cr-Commit-Position: refs/heads/master@{#469781}
Committed: https://chromium.googlesource.com/chromium/src/+/26f8a66eafef66b5c9c64d40028a0769291eb0e5
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix tests and rebase. #
Messages
Total messages: 24 (16 generated)
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. We shouldn't filter duplicate entries however, since the download history records user download actions, and is not a list of files like the file system. BUG=639911 ========== to ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. We shouldn't filter duplicate entries however, since the download history records user download actions, and is not a list of files like the file system. This CL won't fix issues that different BackendItems can have same file path or removing the file from adb shell. BUG=639911 ==========
Description was changed from ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. We shouldn't filter duplicate entries however, since the download history records user download actions, and is not a list of files like the file system. This CL won't fix issues that different BackendItems can have same file path or removing the file from adb shell. BUG=639911 ========== to ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. I think we shouldn't filter duplicate entries however, since the download history is not same concept as a list of files on disk. BUG=639911 ==========
xingliu@chromium.org changed reviewers: + dfalcantara@chromium.org, dtrainor@chromium.org
Hi, PTAL, thanks.
Description was changed from ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. I think we shouldn't filter duplicate entries however, since the download history is not same concept as a list of files on disk. BUG=639911 ========== to ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. I think we shouldn't filter duplicate entries however, since the download history is not the same concept as a list of files on disk. BUG=639911 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
Seems like you busted the tests, somehow. https://codereview.chromium.org/2863863002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java (right): https://codereview.chromium.org/2863863002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java:32: HashSet<String> filePaths = new HashSet<String>(); new HashSet<>();
lgtm % tests and dan's comments.
On 2017/05/05 20:15:44, David Trainor-ping if over 24h wrote: > lgtm % tests and dan's comments. The tests also failed on master branch without this patch, weird.
The CQ bit was checked by xingliu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thanks for the review, tests fixed. https://codereview.chromium.org/2863863002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java (right): https://codereview.chromium.org/2863863002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/download/ui/BackendItems.java:32: HashSet<String> filePaths = new HashSet<String>(); On 2017/05/05 17:21:46, slow (dfalcantara) wrote: > new HashSet<>(); Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by xingliu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dtrainor@chromium.org Link to the patchset: https://codereview.chromium.org/2863863002/#ps20001 (title: "Fix tests and rebase.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494023159396910,
"parent_rev": "9bca3ccdd3cc9c9ad76186bb5339ca7a1c8a5e39", "commit_rev":
"26f8a66eafef66b5c9c64d40028a0769291eb0e5"}
Message was sent while issue was closed.
Description was changed from ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. I think we shouldn't filter duplicate entries however, since the download history is not the same concept as a list of files on disk. BUG=639911 ========== to ========== Fix duplicate space calculation on Android download home. Check duplicate file paths when calculate disk space shown in UI. Current Android code doesn't generate duplicate path, but we still need this fix for existing records. I think we shouldn't filter duplicate entries however, since the download history is not the same concept as a list of files on disk. BUG=639911 Review-Url: https://codereview.chromium.org/2863863002 Cr-Commit-Position: refs/heads/master@{#469781} Committed: https://chromium.googlesource.com/chromium/src/+/26f8a66eafef66b5c9c64d40028a... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/26f8a66eafef66b5c9c64d40028a... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
