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 2017
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

issue 68200
issue 78147

Sign in to add a comment

Issue 167702: Move download opening logic to chrome/

Reported by, Dec 27 2012 Project Member

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, Dec 27 2012

Blocking: chromium:68200

Comment 2 by, Dec 27 2012

Blocking: chromium:78147

Comment 3 by, Dec 27 2012

Blocking: chromium:147753

Comment 4 by, Dec 28 2012

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).

Comment 5 by, Mar 10 2013

Project Member
Labels: -Area-Internals -Feature-Downloads Cr-Internals Cr-UI-Browser-Downloads

Comment 6 by, Apr 30 2013


Comment 7 by, May 14 2013

Blocking: -chromium:147753

Comment 8 by, May 17 2013

 Issue 241155  has been merged into this issue.

Comment 9 by, Nov 16 2015

Labels: -Pri-2 Pri-3
Still necessary.

Comment 10 by, Nov 1 2016

Labels: OS-All

Comment 11 by, Nov 1 2016

Components: -Internals

Comment 12 by, Nov 2 2017

Project Member
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 - Your friendly Sheriffbot

Comment 13 by, Dec 7 2017

Status: WontFix (was: Untriaged)
This will be rendered obsolete by some new refactors that we are working on.

Sign in to add a comment