Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebTorrent: Add "Save All Files..." feature #1230

Closed
btlechowski opened this issue Sep 21, 2018 · 6 comments · Fixed by brave/brave-core#3146
Closed

WebTorrent: Add "Save All Files..." feature #1230

btlechowski opened this issue Sep 21, 2018 · 6 comments · Fixed by brave/brave-core#3146
Assignees

Comments

@btlechowski
Copy link

btlechowski commented Sep 21, 2018

Steps to Reproduce

  1. Go to https://webtorrent.io/free-torrents
  2. Download Big Buck Bunny magnet link
  3. Download torrent content
  4. Save the files

Actual result:

There is no ability to save all files at once. User has to save each file separately.

Expected result:

Ability to save the files in bulk

Brave version (chrome://version info)

Brave 0.55.5 Chromium: 70.0.3538.16 (Official Build) dev (64-bit)
Revision 16ed95b41bb05e565b11fb66ac33c660b721f778-refs/branch-heads/3538@{#306}
OS Windows 7

cc @yrliou

Related issues: #289 #855

@btlechowski btlechowski added suggestion feature/webtorrent Label for webtorrent related issues labels Sep 21, 2018
@bbondy bbondy added this to the 1.x Backlog milestone Sep 21, 2018
@srirambv
Copy link
Contributor

@btlechowski btlechowski added this to Untriaged in Tor and Private Windows via automation Jan 10, 2019
@tildelowengrimm tildelowengrimm removed this from Untriaged in Tor and Private Windows Jan 24, 2019
@rebron rebron added this to Untriaged Backlog in General Jan 28, 2019
@rebron rebron added the priority/P5 Not scheduled. Don't anticipate work on this any time soon. label Jan 29, 2019
@rebron rebron moved this from Untriaged Backlog to P5 Backlog in General Jan 29, 2019
@rebron rebron removed this from the 1.x Backlog milestone Feb 7, 2019
@feross feross added this to To do in WebTorrent Jul 23, 2019
@feross feross self-assigned this Jul 23, 2019
@feross feross moved this from To do to In progress in WebTorrent Jul 30, 2019
@feross feross changed the title Allow in torrent to save all files in bulk WebTorrent: Add "Save All Files..." feature Jul 31, 2019
@feross
Copy link

feross commented Jul 31, 2019

I just did some investigation into how hard it would be to support "Save All Files...". The main constraint is that WebTorrent is running inside a Chrome extension, so we don't have direct access to the filesystem.

First approach – a.click()

The naive approach is to simply trigger a file download for each file in the torrent like this:

const saveAllFiles = () => {
  if (!torrent.serverURL || !torrent.files) return
  torrent.files.forEach((file, ix) => {
    const url = torrent.serverURL + '/' + ix + '/' + file.name
    const a = document.createElement('a')
    a.href = url
    a.download = file.name
    a.click()
  })
}

Two problems with this approach:

  1. The browser pops up a warning that "This site is trying to download multiple files" and blocks all but the first file download until the user gives permission. Bad UX to ask for approval a second time, since the user already said they want to save all files.

  2. If the user has the "Ask where to save each file before downloading" setting enabled (which I believe is the default), then the browser will repeatedly open "Save File" dialogs, one per file in the torrent. Really bad UX.


Second approach – chrome.downloads

The next idea is to use the chrome.downloads API available to Chrome extensions to trigger the file downloads. This requires granting the WebTorrent extension the "downloads" permission. The code to download files looks like this:

const saveAllFiles = () => {
  if (!torrent.serverURL || !torrent.files) return
  torrent.files.forEach((file, ix) => {
    const url = torrent.serverURL + '/' + ix + '/' + file.name
    chrome.downloads.download({
      url: url,
      saveAs: false,
      conflictAction: "uniquify"
    })
  })
}

This addresses problem 1 from the first approach. There is no longer a "This site is trying to download multiple files" dialog. Excellent!

However, for some reason The saveAs: false option does not work. The user is still prompted for a location to save each file, repeatedly. And there doesn't appear to be a way to use the API to trigger the download of a full folder of files. It's not acceptable for the browser to repeatedly open "Save File" dialogs.

Third approach – chrome.fileSystem.chooseEntry

The next idea is to use the chrome.fileSystem API to allow the user to select the folder where they want to save the torrent files. This API is supposed to give us writable access to the directory selected by the user. This would solve both problems with the first approach and seems like our best bet.

This API is intended to only be exposed to Chrome Apps (which are deprecated), and not exposed to extensions. I was able to get around this restriction by creating a new _permission_features.json file and allowing extensions that come from a whitelist that includes the WebTorrent extension:

{
  "fileSystem": {
    "channel": "stable",
    "extension_types": ["extension", "platform_app"],
    "whitelist": ["3D9518A72EB02667A773B69DBA9E72E0F4A37423"]
  },
  "fileSystem.directory": {
    "channel": "stable",
    "extension_types": ["extension", "platform_app"],
    "whitelist": ["3D9518A72EB02667A773B69DBA9E72E0F4A37423"]
  },
  "fileSystem.write": {
    "channel": "stable",
    "extension_types": ["extension", "platform_app"],
    "whitelist": ["3D9518A72EB02667A773B69DBA9E72E0F4A37423"]
  }
}

The extensions build file needed to be changed too:

json_features("permission_features") {
  feature_type = "PermissionFeature"
  method_name = "AddBravePermissionFeatures"
  sources = [
    "//chrome/common/extensions/api/_permission_features.json",
    "//extensions/common/api/_permission_features.json",
    "_permission_features.json",  // <---------- THIS LINE WAS ADDED
  ]
}

Then we can write code like this:

chrome.fileSystem.chooseEntry({
  type: 'openDirectory'
}, onChooseEntry)

function onChooseEntry (entry: any, fileEntries: any) {
  if (chrome.runtime.lastError) {
    console.error(chrome.runtime.lastError)
    // return
  }
  console.log(entry)
  console.log(fileEntries)
  // onSaveAllFiles(entry)
}

But there's something broken with this approach.

Chrome seems to believe that chrome.fileSystem.chooseEntry is being called from the WebTorrent background page and outputs this error: Invalid calling page. This function can't be called from a background page.. The function is definitely being called from the WebTorrent HTML page. I spent several hours attempting to debug this and wasn't successful. If we can solve this error, this seems like the most promising approach. Ideas welcome!

Fourth approach – chrome.fileSystem.requestFileSystem

This API is supposed to give write access to a filesystem. Probably a bad idea to expose to the WebTorrent extension, but figured I'd try this approach for good measure too.

chrome.fileSystem.getVolumeList(onGetVolumeList)

function onGetVolumeList (volumes: any) {
  if (chrome.runtime.lastError) {
    console.error(chrome.runtime.lastError)
    // return
  }
  console.log(volumes)

  // @ts-ignore
  chrome.fileSystem.requestFileSystem({
    volumeId: volumes[0],
    writable: true
  }, onRequestFileSystem)
}

function onRequestFileSystem (filesystem: any) {
  if (chrome.runtime.lastError) {
    console.error(chrome.runtime.lastError)
    // return
  }
  console.log(filesystem)
}

The problem with this approach is that this API is only available on ChromeOS in Kiosk Mode, and lots of the code required to make this API work is behind conditional C++ macros, so it would be a huge pain to get it working. It's not a simple change to some permission json files, like in the third approach. Bummer.

Possible ways to proceed

  1. Create some kind of privileged process/thread with the ability to write the filesystem and get the WebTorrent extension to communicate to it over IPC. Unclear how hard it will be. Adds lots of moving parts. Important to get security right.

  2. Just use the chrome.downloads API from approach one and save all the files as a single .zip file for the user. This is not ideal, but at least it gives the user some way to accomplish the goal of getting all the torrent files out of Brave and into their filesystem. We can ship this as a temporary solution until we figure out a better way to save all files in a folder. This wouldn't be very hard for me to implement.

  3. Other ideas? Please share!

The work-in-progress (non-working) code from this investigation is on the webtorrent-save-all branch.

@yrliou
Copy link
Member

yrliou commented Jul 31, 2019

The saveAs parameter probably get ignored when user enables the following setting in brave://settings:
Screen Shot 2019-07-31 at 3 53 59 PM

Use chrome.downloads API to save as a single .zip file sounds like a reasonable approach to me, personally I would vote for 2, 1 sounds like a complex route which might not be worth it to pursue just for solving this problem.

cc @rossmoody to see if it's fine to serve a single .zip from UX perspective.

@rossmoody
Copy link
Contributor

seems like a win in my opinion in the interim.

@feross
Copy link

feross commented Aug 1, 2019

Thanks for the feedback @rossmoody, okay I'll send a PR to implement this.

@feross feross moved this from In progress to PR sent in WebTorrent Aug 10, 2019
feross added a commit to brave/brave-core that referenced this issue Aug 10, 2019
Currently there is no ability to save all files at once. User has to save each file separately. This commit fixes that.

Fixes: brave/brave-browser#1230
feross added a commit to brave/brave-core that referenced this issue Aug 13, 2019
Currently there is no ability to save all files at once. User has to save each file separately. This commit fixes that.

Fixes: brave/brave-browser#1230
General automation moved this from P5 Backlog to Completed Aug 13, 2019
WebTorrent automation moved this from PR sent to Done Aug 13, 2019
@yrliou yrliou added this to the 0.71.x - Nightly milestone Aug 13, 2019
@btlechowski
Copy link
Author

btlechowski commented Oct 9, 2019

Verification passed on

Brave 0.70.110 Chromium: 77.0.3865.90 (Official Build) beta (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#3146

Disabled "Save All Files..." when torrent is downloading
image

Enabled "Save All Files..." when torrent is downloaded
image

Verified that Save file modal is shown
image

Verified content of the zip file
image

Verified passed with

Brave 0.70.111 Chromium: 77.0.3865.90 (Official Build) beta (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS macOS Version 10.13.6 (Build 17G5019)

Disabled "Save All Files..." when torrent is downloading
Screen Shot 2019-10-09 at 12 11 30 PM

Enabled "Save All Files..." when torrent is downloaded & Save file modal is shown when link is clicked
Screen Shot 2019-10-09 at 12 12 14 PM

Verified content of the zip file
Screen Shot 2019-10-09 at 12 13 30 PM

Verification passed on

Brave 0.70.111 Chromium: 77.0.3865.90 (Official Build) beta (64-bit)
Revision 58c425ba843df2918d9d4b409331972646c393dd-refs/branch-heads/3865@{#830}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Enabled "Save All Files..." when torrent is downloaded & Save file modal is shown when link is clicked

image

image

@rebron rebron removed this from Completed in General Oct 29, 2019
@rebron rebron removed this from Done in WebTorrent Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants