Closed Bug 1258472 Opened 4 years ago Closed 4 years ago

[geckoview] Move Fennec-specific GeckoProfile initialization out of GeckoView

Categories

(Core Graveyard :: Embedding: APIs, defect)

defect
Not set

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: nalexander, Assigned: jchen)

References

Details

Attachments

(2 files)

After Bug 1258450, we have:


nalexander@chocho ~/M/gecko> java -cp mobile/android/build/classycle/classycle-1.4.1.jar classycle.dependency.DependencyChecker -mergeInnerClasses -dependencies=@mobile/android/base/geckoview.ddf objdir-droid/gradle/build/mobile/android/app/intermediates/packaged/local/debug/classes.jar
show onlyShortestPaths allResults
Set [lib] has 133 classes.
Set [app] has 715 classes.
check [lib] independentOf [app]
  Unexpected dependencies found:
  org.mozilla.gecko.GeckoProfile
    -> org.mozilla.gecko.GeckoApp
    -> org.mozilla.gecko.db.BrowserDB
    -> org.mozilla.gecko.distribution.Distribution
    -> org.mozilla.gecko.GuestSession
    -> org.mozilla.gecko.db.StubBrowserDB
    -> org.mozilla.gecko.TelemetryContract
    -> org.mozilla.gecko.Telemetry
    -> org.mozilla.gecko.db.LocalBrowserDB
    -> org.mozilla.gecko.preferences.DistroSharedPrefsImport
<snip>

There are two things Fennec-specific here.  One is that the GeckoProfile yields access to the BrowserDB factory and instances.  I think this can just be turned around: Rather than GeckoProfile.get().getDB(), call BrowserDB.getDB(GeckoProfile.get()).

The latter is that a bunch of Fennec-specific initialization with Telemetry and Distributions is tied into GeckoProfile creation.  I suggest we just make this explicit by having GeckoProfile hand itself to the ambient GeckoInterface, which can then complete whatever initialization it wants to do.

If we play this correctly, we may be able to get rid of https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/StubBrowserDB.java entirely.  This is because the GeckoView package won't reference Fennec DBs at all anymore!
Mass change of bugs in the "Embedding: GRE Core" component in preparation for archiving it. I believe that these are no longer relevant; but if they are, they should be reopened and moved into a component relevant to the code in question.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Product: Core → Core Graveyard
Reopening and moving to Core::Embedding: APIs, where we'll continue to triage and track GeckoView issues.
Status: RESOLVED → REOPENED
Component: Embedding: GRE Core → Embedding: APIs
Product: Core Graveyard → Core
Resolution: INCOMPLETE → ---
Depends on: 1291383
Depends on: 1291384
Move the first-run telemetry session from GeckoProfile to BrowserApp, so
there is no longer any dependency on Telemetry from inside GeckoProfile.
Attachment #8789839 - Flags: review?(s.kaspari)
Because guest mode is intimately tied to the profile, it'd be hard to
keep guest mode out of geckoview code entirely, But we also don't want
any dependency on GuestSession from geckoview code, so this patch moves
the part of GuestSession that manages guest mode state to GeckoProfile.
Attachment #8789840 - Flags: review?(s.kaspari)
Attachment #8789839 - Flags: review?(s.kaspari) → review?(nalexander)
Attachment #8789840 - Flags: review?(s.kaspari) → review?(nalexander)
Assignee: nobody → nchen
Comment on attachment 8789839 [details] [diff] [review]
1. Start first-run telemetry session outside of GeckoProfile (v1)

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

Much better.
Attachment #8789839 - Flags: review?(nalexander) → review+
Comment on attachment 8789840 [details] [diff] [review]
2. Move guest mode state management to GeckoProfile (v1)

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

Looks good.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java
@@ +98,5 @@
>      private File mProfileDir;
>  
>      private Boolean mInGuestMode;
>  
> +    public static boolean shouldUseGuestMode(final Context context) {

I feel like these should be synchronized, but this whole approach could theoretically race since prefs are not atomic, so whatever :)
Attachment #8789840 - Flags: review?(nalexander) → review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/77797e21fa19
1. Start first-run telemetry session outside of GeckoProfile; r=nalexander
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d2e0c859f9e
2. Move guest mode state management to GeckoProfile; r=nalexander
https://hg.mozilla.org/mozilla-central/rev/77797e21fa19
https://hg.mozilla.org/mozilla-central/rev/5d2e0c859f9e
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.