Empty "Downloads" folder should not be created when opening the browser
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
| 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)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-esr140+
|
Details | Review |
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:
- Have no "Downloads" folder in your home directory.
- Open browser.
- 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
Comment 2•1 year ago
|
||
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?
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:
- Deleted the folder;
- Immediately turned on profiler;
- Went to settings;
- Checked the folder did in fact created;
- 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.
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.
Comment 6•1 year ago
|
||
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.
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
Comment 8•1 year ago
|
||
The severity field is not set for this bug.
:mak, could you have a look please?
For more information, please visit BugBot documentation.
Comment 9•1 year ago
|
||
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?
Comment 10•11 months ago
|
||
(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:
nsDirectoryServicethrough https://searchfox.org/mozilla-central/rev/5e24bf00212b4f5c053c1f8d943becf1b5bfd53c/xpcom/io/nsDirectoryService.cpp#423-426 and https://searchfox.org/mozilla-central/rev/5e24bf00212b4f5c053c1f8d943becf1b5bfd53c/xpcom/io/SpecialSystemDirectory.cpp#406 can create the directory on Linux.- To figure out what called it, I added this profiler marker:
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.
| Assignee | ||
Comment 11•11 months ago
|
||
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.
| Assignee | ||
Updated•11 months ago
|
Comment 12•11 months ago
|
||
Set release status flags based on info from the regressing bug 1883903
Updated•11 months ago
|
| Assignee | ||
Comment 13•11 months ago
|
||
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.
Comment 14•11 months ago
|
||
Comment 15•11 months ago
|
||
Comment 16•11 months ago
•
|
||
Backed out for causing crashes on GetPreferredDownloadsDirectory
Comment 17•11 months ago
|
||
Comment 18•11 months ago
|
||
Thanks, Florian & Nazım!
Comment 19•11 months ago
|
||
| bugherder | ||
| Assignee | ||
Comment 21•10 months ago
|
||
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
Updated•10 months ago
|
Comment 22•10 months ago
|
||
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
| Assignee | ||
Updated•10 months ago
|
Updated•10 months ago
|
Updated•10 months ago
|
Comment 23•10 months ago
|
||
| uplift | ||
Updated•10 months ago
|
Updated•10 months ago
|
Comment 24•10 months ago
|
||
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?
Description
•