1166309 - java.lang.UnsatisfiedLinkError: nativeAllocateDirectBuffer when using standalone GeckoView

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

See https://mail.mozilla.org/pipermail/mobile-firefox-dev/2015-May/001194.html

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Move mozglue loading to GeckoThread (v1)

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Load about page favicons on Gecko ready (v1)

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.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Move gfx mozglue dependency to after Gecko starts (v1)

We use DirectBufferAllocator on startup, but we can delay that until later when a buffer is actually required.

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

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?

Comment 5

[Richard Newman [:rnewman]]

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?)

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

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.

Comment 7

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

(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.

Comment 8

[Richard Newman [:rnewman]]

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 9

[Richard Newman [:rnewman]]

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.

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

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.

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Make GeckoJarReader load mozglue if necessary (v1)

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.

Comment 12

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

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.

Comment 13

[Richard Newman [:rnewman]]

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?

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

(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.

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Set interface and don't load mozglue in GeckoView (v1)

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.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7061b206595c
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3c9ab0ae6d1
https://hg.mozilla.org/integration/mozilla-inbound/rev/afe89573e4f9
https://hg.mozilla.org/integration/mozilla-inbound/rev/795f7d597ead

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7061b206595c
https://hg.mozilla.org/mozilla-central/rev/e3c9ab0ae6d1
https://hg.mozilla.org/mozilla-central/rev/afe89573e4f9
https://hg.mozilla.org/mozilla-central/rev/795f7d597ead

Comment 18

[Bob Clary [:bc] (inactive)]

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

Comment 19

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Patch for aurora

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

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8621195 [details] [diff] [review]
Patch for aurora

Fix a crash, taking it.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1ec10fb571da