New issue
Advanced search Search tips

Issue 841559 link

Starred by 2 users
Status: Fixed
Owner:
xingliu@chromium.org
Closed: Jun 2018
Cc:
qin...@chromium.org
dtrainor@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows
Pri: 1
Type: Bug



Sign in to add a comment

Download location, missing sd card error handling.

Project Member Reported by xingliu@chromium.org, May 9 2018 
Chrome Version: 68

When user downloads to external SD card, the user may unmount the SD card, and needs to show UI to user.

The backend native code layer needs the following change:

1. Handle base::File::Write gracefully, that a Write call may only write a partial buffer and return a size less than expected when the SD card is unmounted. Currently we will hit a DCHECK and push the offset incorrectly, which may result in undefined data write to file at some point.

2. After pulling out the SD card, currently the code may generate a temp file through renaming logic on internal storage, we probably want to avoid this to happen.

On the UI layer, it needs:

1. Show an error notification.

2. Show DownloadLocationDialog with an error message on the main view.
Project Member

Comment 1 by bugdroid1@chromium.org, May 11 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab750274305770612416bf8f03f04cdac1890b09

commit ab750274305770612416bf8f03f04cdac1890b09
Author: Xing Liu <xingliu@chromium.org>
Date: Fri May 11 16:48:06 2018

Download location: Fix an issue in file writing.

When we write to a file, currently we assume the whole chunk is written.
However when users pull out the SD card or disk, base::File::Write will
return a size less than the chunk size, and the next call to
base::File::Write will return an error.

This CL fixed this issue so FILE_FAILED error can be correctly reported
when the SD card is unmounted.

This CL basically makes the core write function back to this revision:
https://chromium.googlesource.com/chromium/src/+/29db19d0c973a8990283ec4b81111db5bdb8d2de/content/browser/download/base_file.cc#101

Bug:  841559 ,792775
Change-Id: Icfba65833bf3aec07d30d8d3bed57a9abba7f8d1
Reviewed-on: https://chromium-review.googlesource.com/1053233
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557903}
[modify] https://crrev.com/ab750274305770612416bf8f03f04cdac1890b09/components/download/internal/common/base_file.cc
Project Member

Comment 2 by bugdroid1@chromium.org, May 23 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fcd12ababd1a66e8473d65fca05b569f09707b30

commit fcd12ababd1a66e8473d65fca05b569f09707b30
Author: Xing Liu <xingliu@chromium.org>
Date: Wed May 23 17:43:16 2018

Download location: Show correct error message when removing SD card.

When the user pulls out the SD card during download, currently we show
the error message file already exists.

This CL handles the case when download has no WebContents and can't
write to external storage, we now show error: "download failed because
storage location is not reachable." The download record will then be
removed in download home UI.

Bug:  841559 ,792775
Change-Id: I918d1484abf183770c49514675e1d1757f5ff887
Reviewed-on: https://chromium-review.googlesource.com/1067754
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561144}
[modify] https://crrev.com/fcd12ababd1a66e8473d65fca05b569f09707b30/chrome/browser/download/chrome_download_manager_delegate.cc

Comment 3 by pnangunoori@chromium.org, May 24 2018

Labels: TE-Hardware-Dependency

Comment 4 by xingliu@chromium.org, Jun 21 2018

Status: Fixed (was: Started)
►

Sign in to add a comment