New issue
Advanced search Search tips

Issue 654048 link

Starred by 4 users
Status: Verified
Owner:
qin...@chromium.org
Closed: Nov 2016
Cc:
qin...@chromium.org
asanka@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 652907



Sign in to add a comment

Hitting "back" on Android download warning causes future downloads to hang

Project Member Reported by nparker@chromium.org, Oct 7 2016 
Version: Canary 55.0.2883.0.  This also repros on beta, with a slightly different flow.

Repro:
 1) Visit http://parkerly.com/sbt
 2) Click "downloads" (need something in history)
 3) Click "foo.spl"
 4) When it shows a warning, click BACK.  You'll go to the prev page and the warning will disappear.
 5) Click "cancel" in the system download notification. It'll disappear.
 5) Click "downloads" and then "foo.spl" again.

Result:
  No warning will appear, but the download notification will still show, stuck in a pending state.

Second issue:
  If I try to download foo.spl a third time after this while the download notification is still pending, it'll ask to replace the existing file (though I don't see an existing one), and if I say yes, it downloads without a warning.

Comment 1 by qin...@chromium.org, Oct 12 2016

Owner: qin...@chromium.org

Comment 2 by nparker@chromium.org, Oct 13 2016

Blocking: 652907
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25 2016 

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

commit ad5764a975c7b084009ba19ed7c95a393b8b3b23
Author: qinmin <qinmin@chromium.org>
Date: Tue Oct 25 18:54:45 2016

Use native implementation of dangerous download infobar

Currently the dangerous download infobar is implemented in java.
This is because some of the download is intercepted by
Android DownloadManager previously.
When all the download are handled by native, we should use native
infobar implementation.
The only exception is OMA DRM download, but those files are not
considered dangerous.

BUG= 654048 

Review-Url: https://codereview.chromium.org/2437623003
Cr-Commit-Position: refs/heads/master@{#427428}

[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/BUILD.gn
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/chrome_download_delegate.cc
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/chrome_download_delegate.h
[add] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/dangerous_download_infobar_delegate.cc
[add] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/dangerous_download_infobar_delegate.h
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/download_controller_base.h
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/mock_download_controller.cc
[modify] https://crrev.com/ad5764a975c7b084009ba19ed7c95a393b8b3b23/chrome/browser/android/download/mock_download_controller.h

Comment 4 by qin...@chromium.org, Oct 25 2016

Status: Fixed (was: Untriaged)

Comment 5 by kgna...@chromium.org, Nov 1 2016

Status: Verified (was: Fixed)
Works as per expected behavior, Issue tested on Latest M56 Build.

Comment 6 by qin...@chromium.org, Nov 10 2016 

Issue 663866 has been merged into this issue.

Comment 7 by qin...@chromium.org, Nov 10 2016

Labels: -Pri-2 M-55 ReleaseBlock-Stable Merge-Request-55 Pri-1
Status: Started (was: Verified)
The CL fixes 2 issues, one mentioned here, one mentioned in crbug.com/663866, the latter is more important to fix.
Requesting merge

Comment 8 by dimu@chromium.org, Nov 10 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 11 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7620c145dcabaef1925b113a46637fcdc30cb4f

commit b7620c145dcabaef1925b113a46637fcdc30cb4f
Author: Min Qin <qinmin@chromium.org>
Date: Fri Nov 11 18:30:14 2016

Use native implementation of dangerous download infobar

Currently the dangerous download infobar is implemented in java.
This is because some of the download is intercepted by
Android DownloadManager previously.
When all the download are handled by native, we should use native
infobar implementation.
The only exception is OMA DRM download, but those files are not
considered dangerous.

TBR=dfalcantara@chromium.org
BUG= 654048 

Review-Url: https://codereview.chromium.org/2437623003
Cr-Commit-Position: refs/heads/master@{#427428}
(cherry picked from commit ad5764a975c7b084009ba19ed7c95a393b8b3b23)

Review URL: https://codereview.chromium.org/2496983003 .

Cr-Commit-Position: refs/branch-heads/2883@{#541}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/BUILD.gn
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/chrome_download_delegate.cc
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/chrome_download_delegate.h
[add] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/dangerous_download_infobar_delegate.cc
[add] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/dangerous_download_infobar_delegate.h
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/download_controller_base.h
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/mock_download_controller.cc
[modify] https://crrev.com/b7620c145dcabaef1925b113a46637fcdc30cb4f/chrome/browser/android/download/mock_download_controller.h

Comment 10 by qin...@chromium.org, Nov 14 2016

Status: Fixed (was: Started)

Comment 11 by kgna...@chromium.org, Nov 16 2016

Status: Verified (was: Fixed)
Works as per expected behavior, Issue tested on Latest M55 Beta Build.
►

Sign in to add a comment