Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2949513006:
Show number of remaining files when copying directories. (Closed)
|
Created:
3 years, 6 months ago by tetsui Modified:
3 years, 5 months ago Reviewers:
fukino CC:
chromium-reviews, oka+watch_chromium.org, rginda+watch_chromium.org, mtomasz+watch_chromium.org, yamaguchi+watch_chromium.org, fukino+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionShow number of remaining files when copying directories.
When copying directories with many files inside, the number of remaining
files was shown based on the number of these directories, instead of
actual number of files inside.
BUG=491044
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2949513006
Cr-Commit-Position: refs/heads/master@{#482217}
Committed: https://chromium.googlesource.com/chromium/src/+/b5481973514988451d65106ffc315f89e70143c2
Patch Set 1 #
Total comments: 2
Patch Set 2 : Calculate numRemainingItems in a way similar to processedBytes. #
Total comments: 2
Patch Set 3 : Check before decrement numRemainingItems. #Messages
Total messages: 28 (19 generated)
Description was changed from ========== WIP: Show number of remaining files when copying directories. BUG=491044 TEST=manual ========== to ========== WIP: Show number of remaining files when copying directories. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by tetsui@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== WIP: Show number of remaining files when copying directories. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show number of remaining files when copying directories. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Show number of remaining files when copying directories. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show number of remaining files when copying directories. When copying directories with many files inside, the number of remaining files was shown based on the number of these directories, instead of actual number of files inside. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
tetsui@chromium.org changed reviewers: + fukino@chromium.org
Please take a look. Thanks!
https://codereview.chromium.org/2949513006/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/background/js/file_operation_util.js (right): https://codereview.chromium.org/2949513006/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/background/js/file_operation_util.js:627: for (var key in resolvedEntryMap) { not lgtm, since this changes asymptotic time complexity of updating the progress, and the overhead might slow down the copy operation.
The CQ bit was checked by tetsui@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...
https://codereview.chromium.org/2949513006/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/background/js/file_operation_util.js (right): https://codereview.chromium.org/2949513006/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/background/js/file_operation_util.js:627: for (var key in resolvedEntryMap) { On 2017/06/26 05:15:15, fukino wrote: > not lgtm, since this changes asymptotic time complexity of updating the > progress, and the overhead might slow down the copy operation. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2949513006/diff/20001/ui/file_manager/file_ma... File ui/file_manager/file_manager/background/js/file_operation_util.js (right): https://codereview.chromium.org/2949513006/diff/20001/ui/file_manager/file_ma... ui/file_manager/file_manager/background/js/file_operation_util.js:838: this.numRemainingItems--; IIRC, progress callback can be called multiple times for a single file copy, and it might not be called for a small file. Could you double check?
The CQ bit was checked by tetsui@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...
https://codereview.chromium.org/2949513006/diff/20001/ui/file_manager/file_ma... File ui/file_manager/file_manager/background/js/file_operation_util.js (right): https://codereview.chromium.org/2949513006/diff/20001/ui/file_manager/file_ma... ui/file_manager/file_manager/background/js/file_operation_util.js:838: this.numRemainingItems--; On 2017/06/26 07:34:09, fukino wrote: > IIRC, progress callback can be called multiple times for a single file copy, and > it might not be called for a small file. > Could you double check? Done.
lgtm
Thank you!
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 tetsui@chromium.org
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": 40001, "attempt_start_ts": 1498469104580870,
"parent_rev": "f6d730546e0180c4383d3d50028261afc22ce55e", "commit_rev":
"b5481973514988451d65106ffc315f89e70143c2"}
Message was sent while issue was closed.
Description was changed from ========== Show number of remaining files when copying directories. When copying directories with many files inside, the number of remaining files was shown based on the number of these directories, instead of actual number of files inside. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Show number of remaining files when copying directories. When copying directories with many files inside, the number of remaining files was shown based on the number of these directories, instead of actual number of files inside. BUG=491044 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2949513006 Cr-Commit-Position: refs/heads/master@{#482217} Committed: https://chromium.googlesource.com/chromium/src/+/b5481973514988451d65106ffc31... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b5481973514988451d65106ffc31... |
