809942 - ToolkitProfile service can't change default profile

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

mfinkle has been trying to update his profiles addon:

http://people.mozilla.com/%7Emfinkle/fennec/addons/

but using the toolkit profile service doesn't seem to be working anymore. We should be honoring things that set Default in profiles.ini. Its possible that we're using a cache or something that overwrites the changes. Need more investigation.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: WIP patch

This patch adds a public method to get the default profile name. The default profile is defined as "Default=1" in profiles.ini, not just "default".

The patch then uses the method to implement App.getDefaultProfileName

This seems to fix the problem. Without using this, the "default" profile was always ebing used.

I tested this with WebApps too and they seem to be working with the patch.

Comment 2

[Wesley Johnston (:wesj)]

Comment on attachment 682497 [details] [diff] [review]
WIP patch

Review of attachment 682497 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is fine. Just worried about performance impact.

::: mobile/android/base/App.java.in
@@ +25,5 @@
>          return "@MOZ_CHILD_PROCESS_NAME@";
>      }
>  
>      protected String getDefaultProfileName() {
> +        String profile = GeckoProfile.findDefaultProfile(this);

So this does File I/O now. I wonder if we can cache the default (which would causes this to be incorrect if the default was changed by JS and Fennec not restarted yet). Can we do something like look at the timestamp on the file and use that determine whether to use the cache?

::: mobile/android/base/GeckoProfile.java
@@ +57,5 @@
> +        synchronized (sProfileCache) {
> +            GeckoProfile profile = sProfileCache.get(profileName);
> +            if (profile != null)
> +                return profile;
> +        }

Oh wow. Good catch.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #2)
> Comment on attachment 682497 [details] [diff] [review]
> WIP patch
> 
> Review of attachment 682497 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think this is fine. Just worried about performance impact.
> 
> ::: mobile/android/base/App.java.in
> @@ +25,5 @@
> >          return "@MOZ_CHILD_PROCESS_NAME@";
> >      }
> >  
> >      protected String getDefaultProfileName() {
> > +        String profile = GeckoProfile.findDefaultProfile(this);
> 
> So this does File I/O now. I wonder if we can cache the default (which would
> causes this to be incorrect if the default was changed by JS and Fennec not
> restarted yet). Can we do something like look at the timestamp on the file
> and use that determine whether to use the cache?

I think it swaps the I/O so there should only be the same number of I/O. That said, I am adding a sDefaultProfileName cache to GetProfile, which will enforce this better.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch removes the logging and adds the sDefaultProfileName cache

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 684127 [details] [diff] [review]
patch

Review of attachment 684127 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoProfile.java
@@ +119,5 @@
>          if (!TextUtils.isEmpty(profilePath)) {
>              File dir = new File(profilePath);
>              if (dir.exists() && dir.isDirectory()) {
>                  if (mDir != null) {
> +                    Log.i(LOGTAG, "profile dir changed from " + mDir + " to " + dir);

I wonder if we should remove this. Seems like a potential privacy leak.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Removed the Log.i

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5b4caaea44f

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b5b4caaea44f