1237576 - DownloadContentService: canWrite() and hasEnoughDiskSpace() can erroneously return false

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

On Android 6 with targetSdkVersion=23 and without storage permission canWrite() and hasEnoughDiskSpace() return false (temporaryFile and destinationFile).

Both files are stored in the application specific temporary and data directories. Those directories are writable without a permission. Nevertheless File.canWrite() and getUsableSpace() return values as if we do not have the needed permissions.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

This seems to be actually independent from the permission:
* File.canWrite() returns false if the file does not exist (even though we /could/ write to this destination)
* hasEnoughDiskSpace() returns 0 for a directory that does not exist yet

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1237576 - DownloadAction.getDestinationFile(): Create directory if needed. r?rnewman

Review commit: https://reviewboard.mozilla.org/r/30355/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30355/

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1237576 - DownloadAction: Remove canWrite() check. r?rnewman

We can't use File.canWrite() on files that do not exist yet. After all we are
going to create the file in a folder that we just created. So it is very
unlikely that writing to that folder is going to fail.

Review commit: https://reviewboard.mozilla.org/r/30357/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30357/

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

> * hasEnoughDiskSpace() returns 0 for a directory that does not exist yet

If needed the directory is now created.

> * File.canWrite() returns false if the file does not exist (even though we
> /could/ write to this destination)

I removed this from the code: The only safe way to tell if the file is writable is to create it. But I do not want to create empty files or create and delete them. And as we are now creating the folder first: It's unlikely that we can't create a file in the folder we just created.

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 8706493 [details]
MozReview Request: Bug 1237576 - DownloadAction.getDestinationFile(): Create directory if needed. r?rnewman

https://reviewboard.mozilla.org/r/30355/#review27081

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8706495 [details]
MozReview Request: Bug 1237576 - DownloadAction: Remove canWrite() check. r?rnewman

https://reviewboard.mozilla.org/r/30357/#review27079

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/c49219ebe24741df0615bac9bb7d368793b26b0a
Bug 1237576 - DownloadAction.getDestinationFile(): Create directory if needed. r=rnewman

https://hg.mozilla.org/integration/fx-team/rev/35c1ed58949d991030af52b8af23c879c8ccb364
Bug 1237576 - DownloadAction: Remove canWrite() check. r=rnewman

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c49219ebe247
https://hg.mozilla.org/mozilla-central/rev/35c1ed58949d