Closed Bug 1259653 Opened 4 years ago Closed 4 years ago

Make GeckoService aware of profiles

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(5 files, 2 obsolete files)

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.
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.
Attachment #8735579 - Flags: review?(margaret.leibovic)
Make sure GeckoThread adds the appropriate profile arguments even for
custom profiles.
Attachment #8735584 - Flags: review?(snorp)
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.
Attachment #8735585 - Flags: review?(snorp)
In GeckoService, wait until we get an Intent before starting GeckoThread
using the profile information contained in the Intent.
Attachment #8735586 - Flags: review?(snorp)
(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?
(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.
Attachment #8735579 - Flags: review?(margaret.leibovic) → review+
Attachment #8735584 - Flags: review?(snorp) → review+
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
Attachment #8735585 - Flags: review?(snorp) → review+
Attachment #8735586 - Flags: review?(snorp) → review+
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.
Attachment #8737274 - Flags: review?(margaret.leibovic)
Attachment #8735579 - Attachment is obsolete: true
Attached patch Update testGeckoProfile (v1) (obsolete) — Splinter Review
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.
Attachment #8737276 - Flags: review?(margaret.leibovic)
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.
Attachment #8737856 - Flags: review?(gbrown)
Attachment #8737276 - Attachment is obsolete: true
Attachment #8737276 - Flags: review?(margaret.leibovic)
Attachment #8737856 - Flags: review?(gbrown) → review+
Attachment #8737274 - Flags: review?(margaret.leibovic) → review+
See Also: → 1497148
You need to log in before you can comment on or make changes to this bug.