Move download opening logic to chrome/
Project Member Reported by email@example.com, Dec 27 2012
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.
Dec 27 2012,
Dec 27 2012,
Dec 27 2012,
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).
Mar 10 2013,
Apr 30 2013,
May 14 2013,
May 17 2013,
Issue 241155 has been merged into this issue.
Nov 16 2015,
Nov 1 2016,
Nov 1 2016,
Nov 2 2017,
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
Dec 7 2017,
This will be rendered obsolete by some new refactors that we are working on.
Sign in to add a comment