Closed Bug 1438620 Opened 2 years ago Closed 2 years ago

Don't attempt to remove profiles that are in use by another instance

Categories

(Toolkit :: Startup and Profile System, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: mossop, Assigned: mossop)

References

Details

Attachments

(1 file)

The profile service doesn't do any checks to see if a profile is in use in a different instance of Firefox before attempting to remove a profile.
This also fixed bug 1415640
Blocks: 1415640
Comment on attachment 8951370 [details]
Bug 1438620: Don't offer to delete profiles that are currently in use.

https://reviewboard.mozilla.org/r/220628/#review227468

Looks good, thanks!

::: toolkit/content/aboutProfiles.js:73
(Diff revision 1)
> +    if (!isInUse) {
> +      try {
> +        let lock = profile.lock({});
> +        lock.unlock();
> +      } catch (e) {
> +        Components.utils.reportError(e);

Please remove this Cu.reportError call, or is there a reason to report to the console that another profile is currently in use? (also, now that Cu is defined in all chrome scopes, we shouldn't use Components.utils anymore)

::: toolkit/locales/en-US/chrome/global/aboutProfiles.properties:16
(Diff revision 1)
>  # represent critical user data. (e.g., this directory may not be included as
>  # part of a backup scheme.)
>  # In case localDIr and rootDir are equal, localDir is not shown.
>  localDir = Local Directory
>  currentProfile = This is the profile in use and it cannot be deleted.
> +inUseProfile = This profile is in use and it cannot be deleted.

These 2 strings are almost the same, should we make them differ more, to reduce potential confusion?

::: toolkit/profile/nsToolkitProfileService.cpp:266
(Diff revision 1)
>                // the temp dir first.
>                if (NS_SUCCEEDED(rv) && !equals) {
>                    localDir->Remove(true);
>                }
>  
> +              // Ideally we'd lock after deleting but since the lock is a file

'Ideally we'd lock' -> 'Ideally we'd *un*lock'
Attachment #8951370 - Flags: review?(florian) → review+
I pushed an updated string, what do you think?
Flags: needinfo?(florian)
(In reply to Dave Townsend [:mossop] from comment #5)
> I pushed an updated string, what do you think?

Looks good to me, thanks!
Flags: needinfo?(florian)
Pushed by dtownsend@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c06d68b9961d
Don't offer to delete profiles that are currently in use. r=florian
https://hg.mozilla.org/mozilla-central/rev/c06d68b9961d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Depends on: 1530615
You need to log in before you can comment on or make changes to this bug.