927451 - GeckoView profile folder is not created

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GRE Core, defect)

Description

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

Attachment: geckoview-force-profile

GeckoProfile.get(...) is not enough to cause the actual folder to be created. This patch does a profile.detDir() call that forces the folder to be created. It looks like GeckoProfile should provide some other sane way to do this, but it doesn't yet.

Comment 1

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

Note: With this patch, I can run GeckoView demos on my < 4.3 Android devices again. Android 4.3 still seems broken, but the browser.js code is working. It seems to be a layer view issue. More debugging needed in bug 921792.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 817887 [details] [diff] [review]
geckoview-force-profile

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

::: mobile/android/base/GeckoView.java
@@ +88,5 @@
>          ThreadUtils.setUiThread(Thread.currentThread(), new Handler());
>          initializeView(GeckoAppShell.getEventDispatcher());
>  
>          GeckoProfile profile = GeckoProfile.get(context);
> +        profile.getDir();

not a fan of relying on this having a mysterious side effect. I'd rather add an ensureDir() method to GeckoProfile or a bool to the constructor to indicate that it should be created if it doesn't exist.

Comment 3

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

Attachment: geckoview-force-profile v2

I went with "ensureExists" to better tell us what it's doing. Added Wes too so he knows what's happening.

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 817963 [details] [diff] [review]
geckoview-force-profile v2

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

::: mobile/android/base/GeckoView.java
@@ +88,5 @@
>          ThreadUtils.setUiThread(Thread.currentThread(), new Handler());
>          initializeView(GeckoAppShell.getEventDispatcher());
>  
>          GeckoProfile profile = GeckoProfile.get(context);
> +        profile.ensureExists();

I think I'd like this slightly better if it read something like:

GeckoProfile profile = GeckoProfile.get(context).forceCreate();

Comment 5

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

Attachment: geckoview-force-profile v3

Wes' requested changes were enough for me to get another review. Make sure the "fluent" stuff looks right in forceCreate()

Comment 6

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

Comment on attachment 818018 [details] [diff] [review]
geckoview-force-profile v3

Wes gave a thumbs up on IRC

Comment 7

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

https://hg.mozilla.org/integration/fx-team/rev/723aed267eb0

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/723aed267eb0