1807360 - Black screen when submitting a large file

Status: ASSIGNED

(Fenix :: Browser Engine, defect)

Description

[Chris Peterson [:cpeterson]]

From github: https://github.com/mozilla-mobile/fenix/issues/8387.

Issue created based on ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=1609934

Steps to reproduce

1. Go to "https://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_input_type_file"
2. Tap on the "Browse" from the one file selection.
3. Choose a file that is of at least 1 Gb.

Expected behavior

The input type file tag has the name of the imported file displayed.

Actual behavior

Before having the file name displayed, a black screen is present.

Device information

• Android device: Google Pixel 3 XL (Android 10), Samsung Galaxy S8+ (Android 8.0.0)
• Fenix version: Firefox Preview Nightly 200213 (Build #20440605) GV: 74.0a1-20200210093912

Please view gif.

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:cpeterson, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[egk10]

I believe this is the same or closely related issue. I develop a PWA (https://pwa.kiwix.org) that opens very large files (up to 98GB) using the standard <input type=file> control (these files are offline archives of Web sites such as full English Wikipedia with images). On Chromium browsers for Android, there is no problem opening and reading these files even from a microSD card. In Firefox for Android, it is impossible to open any file larger than around 2GB (or less depending on the phone's memory). The symptom is precisely the black screen and lockup mentioned in this issue.

What I believe is happening is that for some reason Firefox on Android is attempting to copy the file to memory (or possibly its own internal storage) in order to access it. The reason I believe this, is that for files of around 1GB, there is a significant lockup for ~30 seconds, but then the file opens in the app and file access is extremely fast, unlike in Chromium browsers where file access is pretty slow (but usable).

To reproduce, download a large archive such as https://download.kiwix.org/zim/wikipedia/wikipedia_en_top_maxi_2023-07.zim (6.2GB) and attempt to open it in the app at https://pwa.kiwix.org in Firefox Android. Unless your phone has a massive amount of memory, this will totally lock up, whereas it opens fine (but slowly) on any Chromium browser.

To reproduce a file that can be opened, with a significant lockup before successfully opening it, try https://download.kiwix.org/zim/wikipedia/wikipedia_en_astronomy_maxi_2023-07.zim (846MB).

Many users (who are fans of FOSS software) ask me why I have to recommend using Chromium browsers on Android for this PWA rather than Firefox. Most such users would greatly prefer to use the app with Firefox. I hope there is some workaround, but I can find very few mentions of this issue.

Comment 3

[jackyzy823]

I'm not sure how Chromium-based browsers in android handling file upload.

What I believe is happening is that for some reason Firefox on Android is attempting to copy the file to memory (or possibly its own internal storage) in order to access it.

Yes. See comment from https://bugzilla.mozilla.org/show_bug.cgi?id=1856431#c4 . it will copy file to a temp uploads folder under app's cache folder one by one after file selected. This is a heavy IO thing and it happens on the UI thread i guess.

---

After changing like this (copying file in IO thread)

@@ -452,17 +455,21 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
         }
 
         val onSelectMultiple: (Context, Array<Uri>) -> Unit = { context, uris ->
-            val filesUris = uris.map {
-                it.toFileUri(context)
-            }.toTypedArray()
-            if (!prompt.isComplete) {
-                geckoResult.complete(prompt.confirm(context, filesUris))
+            CoroutineScope(Dispatchers.IO).launch {
+                val filesUris = uris.map {
+                    it.toFileUri(context)
+                }.toTypedArray()
+                if (!prompt.isComplete) {
+                    geckoResult.complete(prompt.confirm(context, filesUris))
+                }
             }
         }
 
         val onSelectSingle: (Context, Uri) -> Unit = { context, uri ->
             if (!prompt.isComplete) {
-                geckoResult.complete(prompt.confirm(context, uri.toFileUri(context)))
+                CoroutineScope(Dispatchers.IO).launch {
+                    geckoResult.complete(prompt.confirm(context, uri.toFileUri(context)))
+                }
             }
         }

Fenix using about 30s (first time about 1min) to open 6G zim file with following behavior ( After file selected , the button become from blue to light blue just as keeping clicking on the button for about 30s and page is responsive , not black screen, and then the homepage opened immediately )

Chrome using about 25s with following behavior (After file selected , go to loading page, then homepage show)

Comment 4

[egk10]

Thanks for the context. To be clear, the issue is not an actual file "upload", it's merely picking a file using the File API (i.e. selecting the file with the standard file picker on an <input type=files ...> element). In Chromium for Android, the picked File is made available pretty much instantly (not 25s) as an object that can be manipulated and accessed by JS (access to slices of data in the file is sluggish). In Firefox for Android, there is a long wait with a black screen (presumably because copying is happening) straight after picking, and the wait never finishes with very large files.

We need to open (locally) archives that are up to 98GB (full English Wikipedia with images), often from a microSD card, or from device storage if the user has enough space to store the archive there. It's clearly unfeasible for such large files to be copied.

(This behaviour does NOT happen if a file is in the Origin Private File System, presumably because the browser has direct access to that. The problem with OPFS is that it is limited to a max of 10GB on Firefox, and getting files into it is painfully slow, so it's only a solution for smaller archives.)

Comment 5

[jackyzy823]

Attachment: Bug 1807360 - Part 1: Support opening ContentUri as nsIFile. r?#geckoview-reviewers,#xpcom-reviewers

Comment 6

[jackyzy823]

Attachment: Bug 1807360 - Part 2: Pass ContentUri to GeckoView directly. r?#android-reviewers

Comment 7

[[:owlish] 🦉 PST]

Hi jackyzy823, have you tried reproducing the bug in GeckoView Example? Is it reproducible there?

Comment 8

[[:owlish] 🦉 PST]

Another question for you, jackyzy823: the original bug is about file picker (my understanding is, it wasn't about file upload), but this comment https://bugzilla.mozilla.org/show_bug.cgi?id=1807360#c3 seems to be discussing file upload? Aren't those two separate issues?

Comment 9

[jackyzy823]

Hi, :owlish

Under GeckoView Example, it is basically not possible to choose any file to upload. (for example bug 1591640 and bug 1515440 and bug 1620301)

Due to modern Android's security limitation, it is basically impossible to get file path from querying "_data" column , so GeckoView will just output a error in the log : "GeckoSession: Only file URIs are supported:" and do nothing.
Similar discussion happens in bug 1875732

So in fenix part, they did a trick : Copy the content of ContentUri to a temp file path that fenix has the control, then return that file path to GeckoView for further handling. The copy operation is very IO heavy especially on large file which mostly cause ANR. This trick have a lot of demerit. for example bug 1856431 (file path's limiation) and bug 1860472 (you need to clean up the temp file path) and bug 1872370 (same filename in different folder)
Reference back to bug 1515440 too.

So I'm here to purpose an alternative way handling file picking. 1) Pass ContentUri from Fenix via GeckoView and directly to Gecko. 2) Gecko open a file descriptor via a JNI call to Android's ContentResolver.openFileDescriptor API. 3) Do the same operations on this file descriptor as other file descriptors in all platforms.

---

File upload is just a further step of file picker. If file picker works, then file upload works. It is the same issue.

Comment 10

[Nika Layzell [:nika] (ni? for response)]

I don't think this is the approach we'd want to take here, as changing the low-level nsLocalFile type is a bit overkill - that type is used all over the place, and should generally be used with file paths. Unfortunately the "correct" solution here also doesn't seem to be simple. I don't have a full idea of what a correct solution would look like yet, so this will be a lengthy comment explaining my investigations into this and some thoughts on it:

---

The current approach of supporting content:// URIs as file paths in nsLocalFile is a bit invasive in the low-level components of the browser. In general a nsIFile is intended to be a bare file path, not a URI, so another approach would be to instead change the GeckoView-specific FilePicker code to handle Android URIs for a longer time, and to avoid needing to create a nsIFile for the file if possible. If we change the FilePrompt.confirm code to instead fill the "files" entry with a list of URLs rather than paths (skipping the old getFile code completely), we then need to change the code on the Gecko side to handle that.

The "files" entry is handled by FIlePickerDelegate in the JS code. This is an object implementing the nsIFilePicker interface, which is in-turn consumed by other Gecko APIs.

Looking at the FilePickerDelegate implementation, it is clear that on Android, we only support picking files, not directories, and some of the customization options from desktop are already limited, which simplifies things a bit. We have 3 major sets of APIs which we need to contend with:

1. file and files: These are the APIs which return nsIFile (or an enumerator over them), which is effectively a file path. We can't easily support callers of these APIs with content:// URIs, as there is no file path for those URIs. Unfortunately there's an important user of this API we'll need to change (discussed later), but for URIs which don't have a file path, we'll probably need to make these throw an exception. This has precedent, as in content processes, these methods throw an exception from the nsFilePickerProxy.
2. fileURL: We can perhaps return the content:// URI direcly from here - there appears to already be a GeckoViewContentProtocolHandler which handles directing loads of this scheme to the Java APIs. That being said, I see no C++ consumers of this API, and every caller appears to be in Firefox-desktop-specific code on first glance, so we could perhaps also get away with not supporting this method.
3. domFileOrDirectory and domFileOrDirectoryEnumerator: These return DOM File objects instead of bare file URIs, so are the APIs I believe we'd be able to support. Currently FilePickerProxy uses File.createFromFileName. It might be possible to add a new android-only API to create a File from a content:// URI (perhaps it'd create a StreamBlobImpl under the hood, with a GeckoViewContentInputStream?).

The main APIs for file pickers outside of browser/ generally appear to use DOM files under the hood, however there's one main exception which uses file and files, which is the code proxying nsIFilePicker to a content process. It appears to use the files API, then manually dispatch a runnable to convert the nsIFile to a BlobImpl. I imagine this was originally done to avoid doing main thread I/O, though I'm unclear on if this is still relevant (on Android, I don't think it would be as we create the blobs async before invoking the callback, but perhaps the other file picker implementations do main thread I/O when accessing the relevant member? I wasn't able to find any main-thread I/O in the implementation, which makes me think it's probably an old decision which is no longer relevant).

If we can switch FilePickerParent over to using the domFileOrDirectory APIs, then we could just implement that one interface, and avoid needing to create a nsIFile for a content:// URI at all. (I also considered FileDescriptorFile as an option for creating a normal nsIFile. This would require us making up a fake path for the file, which would be a bit unfortunate, so I'm inclined to avoid that approach unless we need to use it)

---

Leaving a ni? for :asuth who might have more insight into the blob stuff, and to :rkraesig who has been touching the file picker stuff recently and might also have some insight.

Comment 11

[Ray Kraesig [:rkraesig]]

:rkraesig who has been touching the file picker stuff recently and might also have some insight.

No joy, sorry. I've been isolated from most of this by nsBaseFilePicker, to which I have the luxury of merely returning a string, and whose implementation details have largely been irrelevant.

Comment 12

[jackyzy823]

Thanks for your advice :nika.

However I'd insist on my own opinion.

Some reasons

1. it only affect android's part, and do nothing with other POSIX part. I don't think it is invasive. (I mean in my opinion whatever a string could provide a fd should be treated as a path)
2. the GeckoViewContentProtocolHandler is used for Fenix opening PDF from third party application in Android. I used to want to reuse GeckoViewInputStream related code, but it's not easy to integrate.
   fileURL is interesting, but i couldn't find anywhere in dom/file which this attribute is used.
3. focus on dom File , (ie create a new blobImpl for android ContentUri) , well ,that makes sense, but it is way complex to implement.

FYI: What do Chromium for Android think about whether ContentUri could be treated as a file path.
https://source.chromium.org/chromium/chromium/src/+/main:base/files/file_path.h;l=494

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Nika Layzell [:nika] (ni? for response) from comment #10)

Leaving a ni? for :asuth who might have more insight into the blob stuff, and to :rkraesig who has been touching the file picker stuff recently and might also have some insight.
...
If we can switch FilePickerParent over to using the domFileOrDirectory APIs, then we could just implement that one interface, and avoid needing to create a nsIFile for a content:// URI at all. (I also considered FileDescriptorFile as an option for creating a normal nsIFile. This would require us making up a fake path for the file, which would be a bit unfortunate, so I'm inclined to avoid that approach unless we need to use it)

I agree that it would be desirable to go directly from the content URI to a DOM File/Blob. In particular, there's interesting potential to leverage the ContentResolve::registerContentObserver API to get an ContentObserver::onChange notification to do a better job with handling the snapshot state in terms of being able to invalidate the File on update for security/privacy reasons[1].

A meta problem is that the control/data flow around all of this is very confusing, so consolidating the multiple flows to just work in terms of Blobs/Files would be amazing.

The main APIs for file pickers outside of browser/ generally appear to use DOM files under the hood, however there's one main exception which uses file and files, which is the code proxying nsIFilePicker to a content process. It appears to use the files API, then manually dispatch a runnable to convert the nsIFile to a BlobImpl. I imagine this was originally done to avoid doing main thread I/O, though I'm unclear on if this is still relevant (on Android, I don't think it would be as we create the blobs async before invoking the callback, but perhaps the other file picker implementations do main thread I/O when accessing the relevant member? I wasn't able to find any main-thread I/O in the implementation, which makes me think it's probably an old decision which is no longer relevant).

As noted above, the implementation is confusing (we need searchfox dataflow support! ;), but I agree it's likely that the awkwardness here relates to the need for DOM Files/Blobs to have the size and content type synchronously available at the time of their minting. The comment in some initial code landing here in particular calls out the concept at the time of "mystery blobs" where we didn't know that info and caused no end of messed up problems. Mystery blobs are no longer a thing.

Comment 14

[Nika Layzell [:nika] (ni? for response)]

Leaving a ni? for myself to make sure I get back around to this soon.

Comment 15

[Nika Layzell [:nika] (ni? for response)]

Redirecting the bug to :asuth who says they'll look into the blob changes which might be required here.

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

I've translated Nika's proposal from comment 10 into bug 1899467 for the FilePickerParent changes and bug 1899472 for the Content URI Blob support and made them block this bug. Note that as discussed in both bugs, we believe it is likely possible to avoid needing to use GeckoViewContentProtocolHandler and/or GeckoViewInputStream since as long as we can get the file descriptor into C++, we can hand it to a nsFileStreamBase subclass to be the actual stream (noting that right now only NS_NewLocalFileOutputStream directly takes a file descriptor).