1259653 - Make GeckoService aware of profiles

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

When processing an Intent, GeckoService should start GeckoThread using the profile that initiated the Intent.

And if GeckoThread is already running and the Intent profile does not match the current profile, GeckoService should ignore the Intent because there is not much we could do in that situation. Maybe in the future, we could save these Intents and replay them when possible.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Treat custom profiles differently in GeckoProfile (v1)

Make sure we treat profiles under a custom directory differently. To
simplify things, custom profiles must already exist, and we don't
attempt to cache instances of GeckoProfile containing custom profiles.

Custom profiles are an edge use case (must be passed in via Intent
arguments) so I think it's reasonable to have this behavior change.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Make sure GeckoThread handles custom profiles (v1)

Make sure GeckoThread adds the appropriate profile arguments even for
custom profiles.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Support initializing GeckoThread with a specific profile (v1)

Add GeckoThread.initWithProfile that make it easy to target a particular
profile. The method succeeds when GeckoThread has not been initialized
or is already using the specified profile. It fails when the current
profile does not match the specified profile.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Respect the profile when starting GeckoService (v1)

In GeckoService, wait until we get an Intent before starting GeckoThread
using the profile information contained in the Intent.

Comment 5

[:Margaret Leibovic]

(In reply to Jim Chen [:jchen] [:darchons] from comment #1)
> Created attachment 8735579 [details] [diff] [review]
> Treat custom profiles differently in GeckoProfile (v1)
> 
> Make sure we treat profiles under a custom directory differently. To
> simplify things, custom profiles must already exist, and we don't
> attempt to cache instances of GeckoProfile containing custom profiles.
> 
> Custom profiles are an edge use case (must be passed in via Intent
> arguments) so I think it's reasonable to have this behavior change.

What are the consumers of this edge case right now?

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to :Margaret Leibovic from comment #5)
> (In reply to Jim Chen [:jchen] [:darchons] from comment #1)
> > Created attachment 8735579 [details] [diff] [review]
> > Treat custom profiles differently in GeckoProfile (v1)
> > 
> > Make sure we treat profiles under a custom directory differently. To
> > simplify things, custom profiles must already exist, and we don't
> > attempt to cache instances of GeckoProfile containing custom profiles.
> > 
> > Custom profiles are an edge use case (must be passed in via Intent
> > arguments) so I think it's reasonable to have this behavior change.
> 
> What are the consumers of this edge case right now?

I can only think of testing infra as a regular consumer (tests are run inside a custom profile). Technically anybody could pass in a profile directory through the "-profile" argument when launching the GeckoApp activity, but I don't know if anybody does that.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8735585 [details] [diff] [review]
Support initializing GeckoThread with a specific profile (v1)

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

::: mobile/android/base/java/org/mozilla/gecko/GeckoView.java
@@ +232,5 @@
>  
>              final GeckoProfile profile = GeckoProfile.get(context);
>           }
>  
> +        GeckoThread.init(/* profile */ null, /* args */ null,

Might be useful to just add a no-arg overload for init() to avoid having to pass all these

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Treat custom profiles differently in GeckoProfile (v2)

Asking for review again, because I merged GeckoProfile.get(Context) into the
other get method, so that there's only one place for all the logic of getting a
profile, instead of two separate methods.

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Update testGeckoProfile (v1)

This bug changed the meaning of profiles with empty names (""). It used
to mean the same thing as a null name, which represents the default
profile. However, the new behavior is that an empty name indicates a
custom profile. So the tests involving empty names are removed from
testGeckoProfile.

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Update Robocop tests;

This bug changed the meaning of profiles with empty names (""). It used
to mean the same thing as a null name, which represents the default
profile. However, the new behavior is that an empty name indicates a
custom profile. So the tests involving empty names are removed from
testGeckoProfile.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6968de072c7
https://hg.mozilla.org/integration/mozilla-inbound/rev/cecaa7c35791
https://hg.mozilla.org/integration/mozilla-inbound/rev/954498882fb7
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e648c021a95
https://hg.mozilla.org/integration/mozilla-inbound/rev/4743abb7f7ff

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f6968de072c7
https://hg.mozilla.org/mozilla-central/rev/cecaa7c35791
https://hg.mozilla.org/mozilla-central/rev/954498882fb7
https://hg.mozilla.org/mozilla-central/rev/0e648c021a95
https://hg.mozilla.org/mozilla-central/rev/4743abb7f7ff