New issue
Advanced search Search tips

Issue 675559 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

[Chrome][M55] Chrome should not download DM contents

Reported by seiyon.p...@gmail.com, Dec 19 2016

Issue description

Example URL:

Steps to reproduce the problem:
1. Go to http://116.228.149.59/DRM/qiuyu/drm/htm/Image.htm
2. Download any DM contents 

What is the expected behavior?
DownloadManager should download DM contents.

What went wrong?
Chrome downloads DM contents.

Did this work before? Yes M52

Chrome version: 55.0.2883.87  Channel: stable
OS Version: 7.0
Flash Version: Shockwave Flash 24.0 r0

DD file is downloaded using chrome network stack, after extacting the url from DD, the DRM file(DM file) is downloaded by DownloadManager.

But when directly downloading DM file, DM file is downloaded by Chrome not DownloadManager.
 

Comment 1 by mef@chromium.org, Dec 19 2016

Components: UI>Browser>Downloads
Labels: Needs-Feedback
Hi, networking bug sheriff here, could you clarify a few things?

- What does DM stand for?
- What does DD stand for?
- What is DM content? 
- Why do you expect content to be downloaded by DownloadManager and not Chrome?

Comment 2 by asanka@chromium.org, Dec 19 2016

Components: -Internals>Network
Status: Untriaged (was: Unconfirmed)
Adding this to downloads triage queue.

Dear Google
Could you check my inline comment on Comment 1?

- What does DM stand for?
DM is the file extention of the OMA DRM 1.0 Foward-lock(FL)/Combind Delilvery(CD) file.

- What does DD stand for?
DD is the file extention of the OMA Download Descriptor file.

- What is DM content?
DM content is the DRM protected content which is encrypted by OMA DRM FL/CD specification.

- Why do you expect content to be downloaded by DownloadManager and not Chrome?
LGE add the download processing logic to support OMA DRM 1.0 in DownloadManager.
Actually this issue is already discussed why DRM content should be downloaded by DownloadManager. (https://bugs.chromium.org/p/chromium/issues/detail?id=618959)
Kindly reminder.
It is badly impacting the usability.
From Chrome 55, users suffer from DM content downloading failure.
Please prioritize this issue.
Gentle reminder

Comment 7 by dah...@chromium.org, Jan 19 2017

Owner: qin...@chromium.org
Status: Assigned (was: Untriaged)

Comment 8 by qin...@chromium.org, Jan 19 2017

I see the issue. we currently only support OMA download with DD files
I will fix that immediately.

Comment 9 by qin...@chromium.org, Jan 25 2017

BTW, if user long press the link and choose "download link" from context menu, it works fine for M55 and M56. 
So user can temporarily use this as a workaround.
Could you let us know which version of Chrome has a fix for it?
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 8 2017

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

commit 1dfa16b2126e9776a3e26f00b4c767b83f2645b5
Author: qinmin <qinmin@chromium.org>
Date: Wed Feb 08 21:59:50 2017

reintroduce InterceptDownloadResourceThrottle for some OMA DRM downloads

For OMA DRM downloads, if download descriptor file doesn't exist, Android DownloadManager should be used.
This CL reverts some of the changes in https://codereview.chromium.org/2341643008 to intercept such downloads.

BUG= 675559 

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

[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/BUILD.gn
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/chrome_download_delegate.cc
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/chrome_download_delegate.h
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/download_controller_base.cc
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/download_controller_base.h
[add] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/intercept_download_resource_throttle.cc
[add] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/intercept_download_resource_throttle.h
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/mock_download_controller.cc
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/android/download/mock_download_controller.h
[modify] https://crrev.com/1dfa16b2126e9776a3e26f00b4c767b83f2645b5/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc

Labels: -Pri-2 -Arch-x86_64 Arch-ARM M-57 Merge-Request-57 Pri-1
Status: Started (was: Assigned)
The fix is landed on trunk, requesting a merge into chrome 57 as M56 already reaches stable.
Project Member

Comment 13 by sheriffbot@chromium.org, Feb 9 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Hi Google, what does it mean by "CL to branch 2987 manually"?
"CL to branch 2987 for M56 manually" or "CL to branch 2987 for M56 manually"?
Hopefully I expect CL to branch 2987 for M56.
Sorry for typo in Comment 14

Hi Google, what does it mean by "CL to branch 2987 manually"?
"CL to branch 2987 for M56 manually" or "CL to branch 2987 for M57 manually"?
Hopefully I expect CL to branch 2987 for M56.
2987 is M57
Project Member

Comment 17 by bugdroid1@chromium.org, Feb 9 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f38b5828681175efbfac85432cd1527a68b1eff

commit 7f38b5828681175efbfac85432cd1527a68b1eff
Author: Min Qin <qinmin@chromium.org>
Date: Thu Feb 09 17:16:52 2017

reintroduce InterceptDownloadResourceThrottle for some OMA DRM downloads

For OMA DRM downloads, if download descriptor file doesn't exist, Android DownloadManager should be used.
This CL reverts some of the changes in https://codereview.chromium.org/2341643008 to intercept such downloads.

TBR=dtrainor@chromium.org,thakis@chromium.org
BUG= 675559 

Review-Url: https://codereview.chromium.org/2642683005
Cr-Commit-Position: refs/heads/master@{#449100}
(cherry picked from commit 1dfa16b2126e9776a3e26f00b4c767b83f2645b5)

Review-Url: https://codereview.chromium.org/2687023004 .
Cr-Commit-Position: refs/branch-heads/2987@{#408}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/android/java/src/org/chromium/chrome/browser/download/ChromeDownloadDelegate.java
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/BUILD.gn
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/chrome_download_delegate.cc
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/chrome_download_delegate.h
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/download_controller.h
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/download_controller_base.cc
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/download_controller_base.h
[add] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/intercept_download_resource_throttle.cc
[add] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/intercept_download_resource_throttle.h
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/mock_download_controller.cc
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/android/download/mock_download_controller.h
[modify] https://crrev.com/7f38b5828681175efbfac85432cd1527a68b1eff/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc

Status: Fixed (was: Started)
Hi Google, could you revisit this issue and check if it's fixed?
Chrome Beta 58.0.3029.21 has been released since March 16 and according to "Git log", the fix is in this build.
But the problem is that this issue is still reproducible on Chrome Beta 58.0.3029.21.
Hence it would be appreciated if you take a look.

Git log >>>
https://chromium.googlesource.com/chromium/src/+log/57.0.2987.0..58.0.3029.21?pretty=fuller&n=10000
Cc: dim...@chromium.org
Seems there is a new bug, The InterceptDownloadResourceThrottle are now bypassed, and so does the offline_pages::download::ResourceThrottle. 

Checking why this is happening now.
I suspect this is related to https://codereview.chromium.org/2526983002
Cc: mmenke@chromium.org
actually, this was caused  by something else. is_new_request is false when intercepting the download, so the throttler is bypassed.
Cc: -dim...@chromium.org -mmenke@chromium.org
Project Member

Comment 24 by bugdroid1@chromium.org, Mar 22 2017

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

commit 24c83fb4132db2d3ec271c7ae3ad22f6dcc9b5bc
Author: qinmin <qinmin@chromium.org>
Date: Wed Mar 22 07:28:05 2017

Fix an issue that OMA download is not intercepted on Android

We use InterceptDownloadResourceThrottle to intercept OMA download.
However, MimeSniffingResourceHandler::MaybeStartInterception always start
the download with is_new_request == false.
This CL fixes the issue by moving InterceptDownloadResourceThrottle out the if(is_new_request) block.
And append the throttle after AppendStandardResourceThrottles so we won't skip safe browsing
This CL also fixes an issue that displayed filename is null on the snackbar

BUG= 675559 

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

[modify] https://crrev.com/24c83fb4132db2d3ec271c7ae3ad22f6dcc9b5bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java
[modify] https://crrev.com/24c83fb4132db2d3ec271c7ae3ad22f6dcc9b5bc/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/24c83fb4132db2d3ec271c7ae3ad22f6dcc9b5bc/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
[modify] https://crrev.com/24c83fb4132db2d3ec271c7ae3ad22f6dcc9b5bc/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc

Labels: -Hotlist-Merge-Approved -M-57 -merge-merged-2987 M-58 Merge-Request-58
Status: Started (was: Fixed)
Project Member

Comment 26 by sheriffbot@chromium.org, Mar 22 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 27 Deleted

Project Member

Comment 28 by bugdroid1@chromium.org, Mar 23 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da81d57a761d511051498ab40e66c96a333b5e0b

commit da81d57a761d511051498ab40e66c96a333b5e0b
Author: Min Qin <qinmin@chromium.org>
Date: Thu Mar 23 16:33:10 2017

Fix an issue that OMA download is not intercepted on Android

We use InterceptDownloadResourceThrottle to intercept OMA download.
However, MimeSniffingResourceHandler::MaybeStartInterception always start
the download with is_new_request == false.
This CL fixes the issue by moving InterceptDownloadResourceThrottle out the if(is_new_request) block.
And append the throttle after AppendStandardResourceThrottles so we won't skip safe browsing
This CL also fixes an issue that displayed filename is null on the snackbar

TBR=mmenke@chromium.org,dtrainor@chromium.org
BUG= 675559 

Review-Url: https://codereview.chromium.org/2757403002
Cr-Commit-Position: refs/heads/master@{#458666}
(cherry picked from commit 24c83fb4132db2d3ec271c7ae3ad22f6dcc9b5bc)

Review-Url: https://codereview.chromium.org/2772853002 .
Cr-Commit-Position: refs/branch-heads/3029@{#387}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/da81d57a761d511051498ab40e66c96a333b5e0b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerDelegate.java
[modify] https://crrev.com/da81d57a761d511051498ab40e66c96a333b5e0b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
[modify] https://crrev.com/da81d57a761d511051498ab40e66c96a333b5e0b/chrome/android/javatests/src/org/chromium/chrome/browser/download/DownloadTest.java
[modify] https://crrev.com/da81d57a761d511051498ab40e66c96a333b5e0b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc

Huge thanks.
But sorry I found out another issue.

What is the expected behavior?
DM contents should be downloaded in the storage accessible by everyone.

What went wrong?
DM contents are downloaded in  /data/user/0/com.android.providers.downloads/files/ which is so private that only DownloadManager can access. 
Status: Fixed (was: Started)
The issue in #29 is tracked in crbug/708172, a fix is submitted and is merged to M58.

closing this for now.

Sign in to add a comment