1643917 - Allow WebExtensions to clear downloads by hostname in browsingData API

Status: NEW

(WebExtensions :: General, enhancement, P5)

Description

[Anna]

Attachment: Allow WebExtensions to clear downloads by hostname

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Comment 1

[Tom S. (please needinfo tschuster)]

Thanks for submitting this patch. Please see https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html for how to submit a patch using phabricator. To ensure that this patch can be landed you will have to add some tests using hostnames. For example you could extend the existing test.

Comment 2

[Anna]

While working on a test I saw that the documentation for 'hostnames' says that

Items associated with subdomains of a given hostname will not be removed: you must explicitly list subdomains.

but that's not the case if we just call the ClearDataService DownloadsCleaner directly (due to Services.eTLD.hasRootDomain). So we don't get hostname support for downloads quite as free as I initally thought.

I have another patch ready that includes a test and honors that rule in the documentation but phabricator requires multi-factor authentication and I'm not willing to enable that.
If it's not possible to contribute to Firefox without a phabricator account, I'll just stick to experimental APIs for my own extensions and wait until someone else adds all the many missing browsingData API features.

Comment 3

[Tom S. (please needinfo tschuster)]

(In reply to Anna from comment #2)

While working on a test I saw that the documentation for 'hostnames' says that

Items associated with subdomains of a given hostname will not be removed: you must explicitly list subdomains.

but that's not the case if we just call the ClearDataService DownloadsCleaner directly (due to Services.eTLD.hasRootDomain). So we don't get hostname support for downloads quite as free as I initally thought.

Good point! I hadn't actually taken noticed of this documented behavior. I wonder how the other ClearDataService implementations behave.
ImageCacheCleaner at least creates http(s) URLs so I doubt those clean subdomains. Maybe the DownloadsCleaner implementation should be changed? Baku do you know what the expected behavior here is for deleteByHost ?

I have another patch ready that includes a test and honors that rule in the documentation but phabricator requires multi-factor authentication and I'm not willing to enable that.
If it's not possible to contribute to Firefox without a phabricator account, I'll just stick to experimental APIs for my own extensions and wait until someone else adds all the many missing browsingData API features.

If there is really no way for you to use phabricator you can attach patches here and someone else can attach them to phabricator with your author information preserved.

Comment 4

[Anna]

I wonder how the other ClearDataService implementations behave.

I'm sure they're covered by proper tests, right? ;)

Maybe the DownloadsCleaner implementation should be changed? Baku do you know what the expected behavior here is for deleteByHost ?

not Baku but I do know that the DownloadsCleaner's deleteByHost is called indirectly as part of "Forget this site" and I assume it's expected behavior there. [1]

Instead of changing the DownloadsCleaner implementation you could just semi-duplicate the host-deletion part into ext-browsingData.js and replace the Services.eTLD.hasRootDomain call with an explicit comparison, something like this:

const clearDownloads = options => {
  // Items associated with subdomains of a given hostname will *not* be removed: you must explicitly list subdomains.
  if (options.hostnames) {
    return Promise.all(
      options.hostnames.map(thishost => {
        return Downloads.getList(Downloads.ALL).then(aList => {
          aList.removeFinished(aDownload =>
            Services.io.newURI(aDownload.source.url).host === thishost
          );
        });
      })
    );
  }

  return clearData(options, Ci.nsIClearDataService.CLEAR_DOWNLOADS);
};

I'm not sure if the all the promise and return stuff is totally correct but you get the idea.

[1] https://searchfox.org/mozilla-central/source/toolkit/components/forgetaboutsite/ForgetAboutSite.jsm#15