New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 6 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Dec 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 68200
issue 78147



Sign in to add a comment

Move download opening logic to chrome/

Project Member Reported by asanka@chromium.org, Dec 27 2012 Back to list

Issue description

The logic for determining download opening behavior is currently split between content/ and chrome/. Move it to chrome/.

DownloadItemImpl::OnDownloadRenamedToFinalName() asks the delegate whether it is going to open the download (delegate_->ShouldOpenDownload()). If not, DownloadItemImpl::Complete() asks the delegate again whether the download should be opened based on extension (delegate_->ShouldOpenFileBasedOnExtension()).

Ideally, the delegate should observe the download completion and then open it as necessary.

The delegate_->ShouldOpenDownload() call is currently used to delay completion of a download that will be opened by the extension installer. Delaying the completion this way allows the UI to display a spinner while the extension is being installed.
 

Comment 1 by asanka@chromium.org, Dec 27 2012

Blocking: chromium:68200

Comment 2 by asanka@chromium.org, Dec 27 2012

Blocking: chromium:78147

Comment 3 by asanka@chromium.org, Dec 27 2012

Blocking: chromium:147753
I was going to do a dump in this CL of the various alternative Open() architectures I've considered, specifically around straddling the chrome/content split, but I just dug into the chrome/content split around the platform open, and found out that Open() is a chrome level concept anyway.  So I think just hoisting the whole thing out of content makes a lot of sense.

The biggest problem I see around the hoisting is how much and whether to centralize the Open functionality.  If possible, it'd be a cleaner design to have "open" be a UI concept only (possibly disablable for downloads we want to show the user but not allow them to do anything with), and have other subsystems that want to do something with the downloaded file do it themselves after the download completes.  The actual completion of the download would then always occur before open, and if subsystems needed to delay apparent completion at the UI level, it would establish a separate contract with the UI.  But that design may not match our use cases, if there are any cases in which both consumer subsystems to downloads want to do something with the file and we want to allow users to open it.  

My notes on current use cases for open:
* The plugin installer uses "set open when complete"; I don't think there's any reason this couldn't be changed to "disable UI open" + "observe and initiate file operation when complete".
* The UI uses the "opened" bool to mark that the shelf does not need to stay open.  This functionality should be moved into the UI-specific data structure.
* We don't display windows platform download complete notifications if we're going to or have auto-opened.
* We want to be able to support installing extensions and user scripts by clicking on them (currently disabled, but that's a policy decision, not a mechanism decision).
* We need to be able to support "auto-open" for specific file types.
* We need to persist the "opened" state (not sure of the requirement for this).
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-Internals -Feature-Downloads Cr-Internals Cr-UI-Browser-Downloads
Cc: -rdsmith@chromium.org

Comment 7 by asanka@chromium.org, May 14 2013

Blocking: -chromium:147753
Cc: tfarina@chromium.org
 Issue 241155  has been merged into this issue.

Comment 9 by asanka@chromium.org, Nov 16 2015

Labels: -Pri-2 Pri-3
Still necessary.
Labels: OS-All
Components: -Internals
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 2

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: WontFix (was: Untriaged)
This will be rendered obsolete by some new refactors that we are working on.

Sign in to add a comment