GeckoView profile folder is not created

RESOLVED FIXED in mozilla27

Status

defect
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

unspecified
mozilla27
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch geckoview-force-profile (obsolete) — Splinter Review
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.
Attachment #817887 - Flags: review?(blassey.bugs)
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 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.
Attachment #817887 - Flags: review?(blassey.bugs) → review-
Posted patch geckoview-force-profile v2 (obsolete) — Splinter Review
I went with "ensureExists" to better tell us what it's doing. Added Wes too so he knows what's happening.
Assignee: nobody → mark.finkle
Attachment #817887 - Attachment is obsolete: true
Attachment #817963 - Flags: review?(wjohnston)
Attachment #817963 - Flags: review?(blassey.bugs)
Attachment #817963 - Flags: review?(blassey.bugs) → review+
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();
Attachment #817963 - Flags: review?(wjohnston) → review+
Wes' requested changes were enough for me to get another review. Make sure the "fluent" stuff looks right in forceCreate()
Attachment #817963 - Attachment is obsolete: true
Attachment #818018 - Flags: review?(wjohnston)
Comment on attachment 818018 [details] [diff] [review]
geckoview-force-profile v3

Wes gave a thumbs up on IRC
Attachment #818018 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/723aed267eb0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.