Closed Bug 1962880 Opened 1 year ago Closed 11 months ago

Empty "Downloads" folder should not be created when opening the browser

Categories

(Core :: Gecko Profiler, defect)

defect

Tracking

()

RESOLVED FIXED
141 Branch
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 --- unaffected
firefox-esr140 --- fixed
firefox139 --- wontfix
firefox140 --- wontfix
firefox141 --- fixed

People

(Reporter: ilia20018, Assigned: canova, NeedInfo)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Arch Linux. Firefox version unspecified (I'm using a firefox's fork which has its' own versioning), but bug 474718 confirms this bug exists for 16(!) years already and is still not fixed.
Happens every time the browser starts. If it is already started reappears every 2-3 minutes.
Reproducible: Always

Steps to Reproduce:

  1. Have no "Downloads" folder in your home directory.
  2. Open browser.
  3. Now you have it.

Expected Results:
Nothing, I already have another folder to download files into.

Additional information I didn't think to look at first:

User Agent Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0
OS Linux 6.14.2-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 10 Apr 2025 18:43:59 +0000

bug 1828169 is about this happening if you open the settings page. bug 474718 was fixed 14 years ago, so it definitely has not been a problem for 16 years continuously.

As noted in 1828169, I can't reproduce this happening on startup.

Can you use the Firefox profiler with IO markers turned on (or xperf or similar) to get a stack of what is creating the folder on startup and/or every few minutes?

What is your default download directory set to within Firefox? Does this happen on a clean profile if you set the download folder to some existing, other folder, and restart the browser (without opening / reopening settings) ? That is, is it possible this is related to an extension you have installed in your main profile?

Flags: needinfo?(ilia20018)
See Also: → 1828169

I can confirm, that the folder is created at: (1) browser startup; (2) first opening of Settings after startup (following opens are inconsistent, sometimes do create, sometimes not); (3) at random times (in the range 1-5 mins, not more than 10 mins, i guess) even when idle.

I tried to use profiler. I have disabled everything except IO (otherwise there was a lot of (I guess irrelevant) info).
I have done:

  1. Deleted the folder;
  2. Immediately turned on profiler;
  3. Went to settings;
  4. Checked the folder did in fact created;
  5. Turned off profiler.

The results are here: https://share.firefox.dev/3ZdvGGu .

I did not find a way to start a profiler for the startup. I have never used this tool before and it was not intuitively understandable, at least.

The folder it creates is: ~/Downloads.
The custom default directory I have set is: ~/00_System/Downloads (that's temporary, but you get the idea). It is existing and already has downloads). And it basically works - it is downloading everything there, but for some reason still creates empty default folder. It's just I want to unclutter the root of the home directory, but this constantly creating folder destroys the order I've set, which is frustrating as I'm kind of (unofficially) OCD. XD
So I just can't live with that, even though I understand most people will not even really notice it and it will not have high priority.

Now that you have mentioned it, I see, that I did, in fact, forgot about extensions at all and I think about one of them, that (though unlikely) may lead to that behavior. I will definitely test it out and provide an update as soon as I'll have it.

Anyway, regardless of end result of that thread, thanks for your time! I appreciate your help.

Flags: needinfo?(ilia20018)

UPD #1:
Behavior is preserved in Troubleshoot Mode (clean profile with all extensions and everything disabled).
Here's the profiler results: https://share.firefox.dev/3ZbjGoZ .
Again, I've unchecked everything in the profiler except "All File IO". If that is incorrect from my side - I'll redo them.

UPD #2:
As I have said - I am using a modded fork of Firefox (Waterfox, if you know it).
So for the purpose of science I've installed clear default Firefox and tried it on that.
The results are the same, so the problem is definitely not in any extensions or in the modded client.
It still creates it on a startup and when going to the settings. I've not checked idle, as it's random and inconsistent in timing. And I don't know how to profile the startup. So again the first Settings opening is recorded.

And here I've left all defaults in the profiler checked on and All File IO turned on by me.
https://share.firefox.dev/4dbP5gY

I guess I can't provide any more useful info without help to set profiler for the startup =D
But I assume it should be the same thing, just called in the different time.

Hm, unfortunately it doesn't look like that profile captures any IO.

As an example, https://share.firefox.dev/43emKlq is a profile from my mac that shows a process starting and reading the omni.ja files that ship with Firefox.

You can remove other threads but most likely the parent process main thread would need to be included along with any of the FileIO markers. You can delete profiles after uploading them if you find they have info you'd rather not share, but it's likely that if you enable sanitization, all the markers are getting removed. If you hover/select the markers in the marker table or marker graph, you can see the stack on the right hand side. If you're uncomfortable sharing the profile including paths, you could still perhaps copy the relevant stack from how the folder is created on startup?

Also, based on comment 0, I was under the impression that something was creating the downloads directory for you other than the settings page (which we have already diagnosed in bug 1828169, just not fixed yet...). So what I'm trying to figure out is, is fixing the settings page not enough, and if so, what else is creating the download folder on systems configured like this. In particular, it sounds like this happens on startup for you?

So (information from) a startup profile would be helpful - https://profiler.firefox.com/docs/#/./guide-startup-shutdown documents how to get a startup profile. Note that you'd likely want to also use MOZ_PROFILER_STARTUP_FEATURES to enable the fileio and fileioall features.

Flags: needinfo?(ilia20018)

I have somehow missed a chunk of your instructions and seem to have done it wrong at first, but I left it here just in case:
Ok, thanks for the info. I've checked all the defaults on plus main threads fileio and all fileio. While I'm a bit known to the debugger at that scale I'm just overwhelmed with it, so I've just uploaded it as is. Here's the work build - https://share.firefox.dev/43sn3dE And here for the cleanliness the pure Firefox - https://share.firefox.dev/4mb5kix I hope this helps.

OH, I have definitely somehow missed the part about the profile including paths. So I'll redo it again, but only on pure Firefox, as I've already written a long text =D
https://share.firefox.dev/4dqLmMz

If I've missed something - I can do anything needed

Flags: needinfo?(ilia20018)

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

For more information, please visit BugBot documentation.

Flags: needinfo?(mak)

Nazim or Florian, can you please help? The reporter has been very diligent and already shared something like 5 different profiles. As far as I can tell they all have the relevant markers turned on in the settings, but 0 of those markers are actually visible in the marker table. Of course I want to see the one that is creating this directory, but even besides that we must be touching the disk to start the browser, but there are 0 markers, which makes absolutely no sense to me.

What we need to make progress here is something that contains stacks pointing to why the directory would be created at startup, because it's not obvious to me. We could try to optimistically fix the issue identified in bug 1828169 and hope it fixes this too, but I'm not very confident of that without some evidence that it is really the same root cause... If the profiler cannot give us those stacks, is there a different tool that can?

Flags: needinfo?(mak)
Flags: needinfo?(florian)
Flags: needinfo?(canaltinova)

(In reply to :Gijs (he/him) from comment #9)

Nazim or Florian, can you please help?

I still have no idea of why the profiles didn't contain any mainthreadio markers, but about what's creating the folder:

diff --git a/xpcom/io/nsDirectoryService.cpp b/xpcom/io/nsDirectoryService.cpp
--- a/xpcom/io/nsDirectoryService.cpp
+++ b/xpcom/io/nsDirectoryService.cpp
@@ -15,6 +15,7 @@
 #include "nsThreadUtils.h"
 
 #include "mozilla/SimpleEnumerator.h"
+#include "mozilla/ProfilerMarkers.h"
 #include "nsICategoryManager.h"
 #include "nsISimpleEnumerator.h"
 
@@ -191,6 +192,8 @@ nsDirectoryService::Get(const char* aPro
   MOZ_ASSERT(NS_IsMainThread(), "Do not call dirsvc::get on non-main threads!");
 
   nsDependentCString key(aProp);
+  AUTO_PROFILER_MARKER_TEXT("nsDirectoryService::Get", OTHER,
+                            MarkerStack::Capture(), key);
 
   nsCOMPtr<nsIFile> cachedFile = mHashtable.Get(key);
 

and got this profile: https://share.firefox.dev/3SXwOKX

There's only call to the directory service for the downloads dir during startup, and it has this stack:

ScopedXPCOMStartup::Initialize(bool) [mozilla/toolkit/xre/nsAppRunner.cpp]
NS_InitXPCOM [mozilla/xpcom/build/XPCOMInit.cpp]
profiler_lookup_async_signal_dump_directory() [mozilla/tools/profiler/core/platform.cpp]
NS_GetSpecialDirectory(char const*, nsIFile**) [mozilla/obj-opt/dist/include/nsDirectoryServiceUtils.h]
nsDirectoryService::Get(char const*, nsID const&, void**) [mozilla/xpcom/io/nsDirectoryService.cpp]

So... the Downloads directory is accessed (and thus created) during early startup when initializing the profiler so the profiler can be started using signals (even when the main thread is blocked). This had to be done before the signal is received because the directory service is accessed through main thread only xpcom.

Flags: needinfo?(florian)
Keywords: regression
Regressed by: 1883903

Sorry for not looking at it earlier. The issue wasn't obvious to me from an initial look, so I was keeping in my backlog.

Oh wow, I was definitely not expecting a profiler bug from here.
Currently we use use NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR, ...) to retrieve the download directory. But apparently this is not correct. The other place where we use this function is here, in GetPreferredDownloadsDirectory. I think the correct method to use is GetPreferredDownloadsDirectory from the profiler, so we should replace that usage with this.

Flags: needinfo?(canaltinova)
Assignee: nobody → canaltinova
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Set release status flags based on info from the regressing bug 1883903

We were using NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR, ...)
before for getting the download directory for the profiler POSIX signal
profiling. But this ignores the user-set download directory and always
use the default downloads folder. And while doing so, it creates this
default folder even if the user set it to another one.

Now we are using GetPreferredDownloadsDirectory directly which looks
for the user-set download directory first and uses it if it find one.

We had to change the location we call profiler_lookup_async_signal_dump_directory
because in that location user-defined prefs weren't fully initialized
yet in NS_InitXPCOM. Moved it inside XREMain::XRE_mainRun. Note that
there is a small behavior change here. Previously that function was
being called in every process, and now it's being called only in the
parent process. This is actually the desired behavior because we can't
capture profiles by directly sending POSIX signals to child processes
due to sandboxing restrictions.

Pushed by canaltinova@gmail.com: https://github.com/mozilla-firefox/firefox/commit/df018ffa673e https://hg.mozilla.org/integration/autoland/rev/f6feafabe30d Get the download directory correctly for the profiler POSIX signal profiling r=florian,profiler-reviewers
Pushed by csabou@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/0a7901304d18 https://hg.mozilla.org/integration/autoland/rev/91f1c95b08c2 Revert "Bug 1962880 - Get the download directory correctly for the profiler POSIX signal profiling r=florian,profiler-reviewers" for causing gv-junit crashes on GetPreferredDownloadsDirectory.

Backed out for causing crashes on GetPreferredDownloadsDirectory

Push with failures

Failure log

Backout link

Flags: needinfo?(canaltinova)
Pushed by canaltinova@gmail.com: https://github.com/mozilla-firefox/firefox/commit/689daeebfad3 https://hg.mozilla.org/integration/autoland/rev/a50fa959f2ec Get the download directory correctly for the profiler POSIX signal profiling r=florian,profiler-reviewers

Thanks, Florian & Nazım!

Component: Downloads API → Gecko Profiler
Flags: needinfo?(canaltinova)
Product: Toolkit → Core
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 141 Branch

Please nominate this for ESR140 approval.

Flags: needinfo?(canaltinova)

We were using NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR, ...)
before for getting the download directory for the profiler POSIX signal
profiling. But this ignores the user-set download directory and always
use the default downloads folder. And while doing so, it creates this
default folder even if the user set it to another one.

Now we are using GetPreferredDownloadsDirectory directly which looks
for the user-set download directory first and uses it if it find one.

We had to change the location we call profiler_lookup_async_signal_dump_directory
because in that location user-defined prefs weren't fully initialized
yet in NS_InitXPCOM. Moved it inside XREMain::XRE_mainRun. Note that
there is a small behavior change here. Previously that function was
being called in every process, and now it's being called only in the
parent process. This is actually the desired behavior because we can't
capture profiles by directly sending POSIX signals to child processes
due to sandboxing restrictions.

Original Revision: https://phabricator.services.mozilla.com/D254342

Attachment #9500002 - Flags: approval-mozilla-esr140?

firefox-esr140 Uplift Approval Request

  • User impact if declined: Firefox will create empty downloads directory if user has changed the default download directory to something else
  • Code covered by automated testing: no
  • Fix verified in Nightly: yes
  • Needs manual QE test: yes
  • Steps to reproduce for manual QE testing: Go to about:preferences and change the default downloads directory to something else. Rename your downloads directory so it's not there anymore. Restart Firefox. A new default downloads directory shouldn't be created.
  • Risk associated with taking this patch: Very little
  • Explanation of risk level: It's not risky. It mainly moves the code around
  • String changes made/needed: -
  • Is Android affected?: no
Flags: qe-verify+
Flags: needinfo?(canaltinova)
Attachment #9500002 - Flags: approval-mozilla-esr140? → approval-mozilla-esr140+
QA Whiteboard: [uplift][qa-ver-needed-c142/b141]
QA Contact: cgeorgiu

Hi, Mr_El! Unfortunately, I'm not able to reproduce this on Win 11 and Ubuntu 24. I don't currently have Arch Linux installed to test it there as well.

Can you please help us verify if the bug is fixed on latest Nightly 141.0a1 and Esr 140.1.0?

Flags: needinfo?(ilia20018)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: