Closed Bug 1166309 Opened 9 years ago Closed 9 years ago

java.lang.UnsatisfiedLinkError: nativeAllocateDirectBuffer when using standalone GeckoView

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox40 fixed, firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(5 files, 1 obsolete file)

On startup, we use GeckoJarReader/NaiveZip to load about page favicons from the omnijar, and using NativeZip requires mozglue. Since we are moving mozglue loading to GeckoThread, we need to delay loading these favicons to Gecko:Ready.
Attachment #8612932 - Flags: review?(mark.finkle)
We use DirectBufferAllocator on startup, but we can delay that until later when a buffer is actually required.
Attachment #8612934 - Flags: review?(snorp)
Comment on attachment 8612929 [details] [diff] [review]
Move mozglue loading to GeckoThread (v1)

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

::: mobile/android/base/GeckoThread.java
@@ +89,5 @@
>      private String initGeckoEnvironment() {
>          final Locale locale = Locale.getDefault();
>  
>          final Context context = GeckoAppShell.getContext();
> +        GeckoLoader.loadMozGlue(context);

Do we have to do this here instead of lower down where we start loading the other libs?
Attachment #8612929 - Flags: review?(snorp) → review+
Attachment #8612934 - Flags: review?(snorp) → review+
I could have sworn we had a bug on file to not pull those icons out of the JAR, but the best I could find is Bug 943082.

IIRC we kept the JAR-based approach because it turns out to be fast enough, but if it's getting in the way (or ends up showing the icon late) then let's fix it by shipping those icons as Android resources. File a bug?

(And why does GeckoView care about favicons anyway?)
Flags: needinfo?(nchen)
So we'd like to move mozglue loading to GeckoThread, which requires that we not use mozglue until Gecko is loaded. Loading favicon from omnijar is one of two places where we are currently using mozglue (through GeckoJarReader) before Gecko is loaded.
Flags: needinfo?(nchen)
(In reply to Jim Chen [:jchen] [:darchons] from comment #6)
> So we'd like to move mozglue loading to GeckoThread, which requires that we
> not use mozglue until Gecko is loaded. Loading favicon from omnijar is one
> of two places where we are currently using mozglue (through GeckoJarReader)
> before Gecko is loaded.

Does moving mozglue to the GeckoThread mean we can't use the shared SQLite until Gecko loads? Does this idea mean we can't try to use parts of Gecko, like networking, by moving them into mozglue (or similar) unless Gecko is running?

You can probably guess the "until Gecko is running" part has me a little worried.
We also need GeckoJarReader:

* to extract search engines from SearchEngineManager
* to add default bookmarks on first run where those icons aren't Android drawables.

The latter also has me worried.
Comment on attachment 8612932 [details] [diff] [review]
Load about page favicons on Gecko ready (v1)

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

This might have unpleasant side-effects; those are deliberately in the non-evictable pre-loaded cache to avoid hitting the usual favicon flow. I don't know offhand what will happen if we request the favicon for one of those pages and the cache isn't yet populated.
Sorry my wording wasn't very clear. I guess it's more about having each module that depends on mozglue load mozglue themselves, rather than loading mozglue for everybody in GeckoApplication. SQLiteBridge already does this, and loads mozglue itself if needed.

As for DirectBufferAllocator and NativeZip, maybe the better way is to have them load mozglue themselves as well.
This is the other approach of making GeckoJarReader load mozglue if necessary, instead of relying on GeckoApplication loading it. It adds an extra Context argument to some GeckoJarReader methods.
Attachment #8612932 - Attachment is obsolete: true
Attachment #8612932 - Flags: review?(mark.finkle)
Attachment #8614088 - Flags: review?(mark.finkle)
Comment on attachment 8614088 [details] [diff] [review]
Make GeckoJarReader load mozglue if necessary (v1)

I was going to ask about dropping the "resources" arg from getBitmapDrawable and just use the "context" you're passing in now to access the resources inside the method, but I guess leaving as is gives us some flexibility.

We could always follow-up later, if we want to change that.

LGTM, but passing to Richard in case he has some opinions.
Attachment #8614088 - Flags: review?(rnewman)
Attachment #8614088 - Flags: review?(mark.finkle)
Attachment #8614088 - Flags: review+
Comment on attachment 8614088 [details] [diff] [review]
Make GeckoJarReader load mozglue if necessary (v1)

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

I have two concerns about this.

The first is: why put the load of mozglue into GeckoJarReader and not into NativeZip?

The second is: that just-in-case load of mozglue involves synchronization, and potentially loading the library.

It's very likely that the synchronization has a negligible runtime impact, but without measuring it's hard to tell. And of course in the worst-case scenario we have to load mozglue in the middle of an operation; presumably there's some chance of that happening, otherwise there's no point doing this work!

So what can we do to measure and mitigate any runtime impact from those two things?
(In reply to Richard Newman [:rnewman] from comment #13)
> Comment on attachment 8614088 [details] [diff] [review]
> Make GeckoJarReader load mozglue if necessary (v1)
> 
> Review of attachment 8614088 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I have two concerns about this.
> 
> The first is: why put the load of mozglue into GeckoJarReader and not into
> NativeZip?

I tend to think of NativeZip as being a component within mozglue that you shouldn't use at all until mozglue itself is properly initialized/loaded. So that gives the responsibility of loading mozglue to whoever uses NativeZip. This is consistent with, e.g., SQLiteBridgeContentProvider loading mozglue before using SQLiteBridge [1], although we don't seem to use this pattern everywhere.

Loading mozglue requires a Context, so having the user code load mozglue also gives some flexibility in determining when the best time to load mozglue is.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/SQLiteBridgeContentProvider.java?rev=920ded6a1f77#127

> The second is: that just-in-case load of mozglue involves synchronization,
> and potentially loading the library.
> 
> It's very likely that the synchronization has a negligible runtime impact,
> but without measuring it's hard to tell. And of course in the worst-case
> scenario we have to load mozglue in the middle of an operation; presumably
> there's some chance of that happening, otherwise there's no point doing this
> work!
> 
> So what can we do to measure and mitigate any runtime impact from those two
> things?

mozglue loading happens at startup (through one of GeckoThread, GeckoJarReader, etc.), so if there's no startup regression, I think we can be reasonably sure there won't be much runtime impact.
Attachment #8614088 - Flags: review?(rnewman) → review+
Last piece to make GeckoView work again. It's okay to move setGeckoInterface out of | if (!isGeckoActivity) | because if we don't have a GeckoInterface by the time we initialize GeckoView, it's definitely not a GeckoActivity.
Attachment #8616198 - Flags: review?(snorp)
Attachment #8616198 - Flags: review?(snorp) → review+
FYI, win on throbber stop especially noticeable in twitter and nytimes.

http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstop/local-twitter/norejected/2015-06-09/2015-06-09/notcached/errorbars/standarderror/notry

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=acd94173f7e1&tochange=f84de1ca1d11

either due to Bug 1166452 - Add an even-more-delayed startup event or Bug 1166309 - java.lang.UnsatisfiedLinkError: nativeAllocateDirectBuffer when using standalone GeckoView
Attached patch Patch for auroraSplinter Review
Approval Request Comment

[Feature/regressing bug #]: bug 1158309

[User impact if declined]: unable to use standalone GeckoView (crash on startup)

[Describe test coverage new/current, TreeHerder]: Locally, m-c

[Risks and why]: Very small; patch is tested on Nightly and fixes the crash.

[String/UUID change made/needed]: None
Attachment #8621195 - Flags: review+
Attachment #8621195 - Flags: approval-mozilla-aurora?
Comment on attachment 8621195 [details] [diff] [review]
Patch for aurora

Fix a crash, taking it.
Attachment #8621195 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.