1812356 - Crash when loading any resource://android/assets/page.html

Status: RESOLVED FIXED

(GeckoView :: General, defect, P2)

Description

[devel2ew]

Attachment: geckoview-example-master.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/109.0.0.0 Safari/537.36

Steps to reproduce:

Load embed page from assets crash and exit

Actual results:

pid: 15950, tid: 16260, name: Gecko >>> com.test.example <<<
2023-01-25 17:02:31.453 17182-17182 DEBUG pid-17182 A #00 pc 0000000000a32b00 /data/app/~~WYN2qtYKg8_gzzbocaaMGw==/com.test.example-_9jcuR1U-r4QGTntuLW84Q==/lib/arm64/libxul.so!libxul.so (BuildId: c255cbfc920df26b04237585d6a67486d2e33fc5)

Expected results:

Load html page from assets.

Comment 1

[devel2ew]

crash with 'org.mozilla.geckoview:geckoview-nightly:111.0.20230125094200' too.

Comment 2

[Zac McKenney [:zmckenney]]

What restrictions do we have on loading embedded sites from internal assets? I'm not sure if this is a supported behavior or not

Comment 3

[Jonathan Almeida [:jonalmeida]]

From internal assets, it should be fine to load assets. It's not clear from the crash what caused it.

Comment 4

[Skyler Hawthorne]

This issue has been making Firefox nigh unusable on Android for the last couple months.

Comment 5

[mabangwang]

Is there no kind person to solve the problem

Comment 7

[Jonathan Almeida [:jonalmeida]]

(In reply to Skyler Hawthorne from comment #4)

This issue has been making Firefox nigh unusable on Android for the last couple months.

(In reply to mabangwang from comment #5)

Is there no kind person to solve the problem

This bug report is for an app that uses GeckoView in it. If you are seeing a similar result, please file a new bug with steps to reproduce and a crash log so we can properly understand the cause of it. Thanks!

Comment 8

[jw]

Thanks you for looking at this. I'm not sure I understand your request for a new bug, at least one other bug (which I opened) has been closed and pointed at this one as a duplicate.

The steps to reproduce the crash are either:

a) use the project in attached to this bug

OR

b) do the following:
1) Follow the steps at https://firefox-source-docs.mozilla.org/mobile/android/geckoview/consumer/geckoview-quick-start.html to set up an Android project to use GeckoView
2) Add an index.html asset file
3) Set the GeckoView to load this file: session.loadUri("resource://android/assets/index.html");
4) Run the project and GeckoView will segfault.

More complete logs can be found in https://bugzilla.mozilla.org/show_bug.cgi?id=1817249

Please, shout up if there is any more information needed as this crash is trivial to reproduce.

In short, the last working version was v108. Since v109, GeckoView is no longer useable as a WebView component on Android for any line-of-business app that requires the loading of assets from within the Android app itself (it still functions to browse the web, but really, this isn't much use). I'd imagine almost every Android app using GeckoView is currently stuck on v108.

We moved to GeckoView to give us the stability of a known WebView implementation that our app can be built on, but will reluctantly have to look to move back to the stock Android/Chromium WebView if this issue isn't resolved, as it's been outstanding for months.

Please, can this be made a P1/S1 as it is completely fatal for use of GeckoView on Android in a line-of-business app. If you need more information, we are happy to provide.

Thanks again.

Comment 9

[Jonathan Almeida [:jonalmeida]]

The request for a new bug was for the the users who I quoted in my comment above

(In reply to jw from comment #8)

We moved to GeckoView to give us the stability of a known WebView implementation that our app can be built on, but will reluctantly have to look to move back to the stock Android/Chromium WebView if this issue isn't resolved, as it's been outstanding for months.

Please, can this be made a P1/S1 as it is completely fatal for use of GeckoView on Android in a line-of-business app. If you need more information, we are happy to provide.

Can you still see this bug with the latest nightly version of GeckoView? This bug does not reproduce for us. You can see how we load files from resources for our error pages here and still works with nightly 113 and beta 112 for myself.

My hypothesis is that this a configuration issue - could you share a sample project that reproduces the bug?

Comment 10

[Jonathan Almeida [:jonalmeida]]

I see from bug 1817249 comment 3 that you have already tried with version 111. Which makes me believe that this is still a configuration issue.

Here is GeckoView Example which is a simple browser app that you can use to compare your application with to look for any differences.

Comment 11

[jw]

(In reply to Jonathan Almeida [:jonalmeida] from comment #10)

I see from bug 1817249 comment 3 that you have already tried with version 111. Which makes me believe that this is still a configuration issue.

Here is GeckoView Example which is a simple browser app that you can use to compare your application with to look for any differences.

implementation "org.mozilla.geckoview:geckoview-nightly:113.0.20230316125715" still fails for me

I'll have a go building the example and see if we get the same behaviour, though, this bug isn't manifest if you get the GeckoView to show the error page (which I believe is how you've tried to recreate it so far). So, we have:
session.loadUri("resource://android/assets/index.html"); => this segfaults immediately
session.loadUri("google.com"); => this works just fine
session.loadUri("http://some.test.address.that.doesnt.exist.com/"); => this displays the error page and works just fine

It is possible for you to spin up the example and just force it to load a URI that is an asset and see if you get the same issue? So, hard code it to do:
session.loadUri("resource://android/assets/error.html");

Thanks.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S4. However, the following bug duplicate has higher severity:

• Bug 1817249: S2

:cpeterson, could you consider increasing the severity of this bug to S2?

For more information, please visit auto_nag documentation.

Comment 13

[jw]

I've reproduced this using the GeckoView Example listed above (https://hg.mozilla.org/mozilla-central/file/tip/mobile/android/geckoview_example).

The simplest way to do this is to build and install the example, click on the URL bar at the bottom, type in "resource://android/assets/error.html" and press Done. The app will crash.

This can also be done programmatically, by amending the example to call:
session.loadUri("resource://android/assets/error.html");

Comment 14

[Chris Peterson [:cpeterson]]

Thanks for verifying this crash in the latest GeckoView Example. I'll update this bug's status flags.

Comment 15

[jw]

Could this please me made a S2?

From the documentation an S2 is:
(Serious) Major functionality/product severely impaired or a high impact issue and a satisfactory workaround does not exist

I believe this meets this criteria, the product is severely impaired (i.e. unusable) on Android for any line-of-business application and there is no workaround. The only thing you can currently use GeckoView for is building a web browser for viewing external webpages (I can't see why anyone would do this when you can just download Firefox). Therefore, GeckoView it is no longer useful as a WebView alternative on Android.

Comment 16

[Makoto Kato [:m_kato]]

This is assertion by nsDocShellLoadState::ValidateWithOriginalState. Since resouce:// URI is internal resource, nsStandardURL::EqualsInternal always return false due to QI's failure.

resource:// URI is intenral resource URI in Gecko, so we shouldn't allow it by GeckoSession.loadUri.

Comment 17

[jw]

(In reply to Makoto Kato [:m_kato] from comment #16)

This is assertion by nsDocShellLoadState::ValidateWithOriginalState. Since resouce:// URI is internal resource, nsStandardURL::EqualsInternal always return false due to QI's failure.

resource:// URI is intenral resource URI in Gecko, so we shouldn't allow it by GeckoSession.loadUri.

Thanks for the update. Is still an intended behaviour change? The ability to load from a "resource://" URI was explicitly added (about 7 years ago) to make the GeckoView useful for Android applications and has been working till v108 towards the end of last year. I can't see anything in any release notes about this change (as it's a breaking change, I would have expected something).

Please, can this be reverted? As the ability to load files from the assets within an Android application is pretty fundamental for a WebView component.

Or is there another mechanism that should be used since v109+ to load local resource files?

Comment 18

[Makoto Kato [:m_kato]]

Regression range is here.
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a69845ef7e6e5a16056ebdbfdf550e473da1cf17&tochange=73b3372a88c5894eec2125711abcaa11f0e87552

Comment 19

[Makoto Kato [:m_kato]]

I guess that root cause is that this nsIURI is nsStandardURL, not SubstitutingJARURI. If we allow resource://android/assets by GeckoSession.loadUri, I have to investigate why.

Comment 20

[Makoto Kato [:m_kato]]

Ah, I may make sense. When passing SubstitutingJARURI via IPC, SubstitutingJARURI is serialized to other URI types such as nsStandardURL in https://searchfox.org/mozilla-central/rev/54c533e94ae786056a43231f230c7d9b0773cb80/netwerk/protocol/res/SubstitutingJARURI.h#149.

Comment 21

[Makoto Kato [:m_kato]]

Nika, this seems to be regression by bug 1538028. When URI is SubstitutingJARURI, it is serialized to nsStandardURL. Then nsDocShellLoadState::ValidateWithOriginalState detects as URI mismatched (then finally, chrome process crashes).

Should we add SubstitutingJARURI has NS_THIS_STANDARDURL_IMPL_CID interface for nsStandardURL::Equals, or add SubstitutingJARURIParams in URIParams for IPC?

Comment 23

[Nika Layzell [:nika] (ni? for response)]

(In reply to Makoto Kato [:m_kato] from comment #21)

Nika, this seems to be regression by bug 1538028. When URI is SubstitutingJARURI, it is serialized to nsStandardURL. Then nsDocShellLoadState::ValidateWithOriginalState detects as URI mismatched (then finally, chrome process crashes).

Should we add SubstitutingJARURI has NS_THIS_STANDARDURL_IMPL_CID interface for nsStandardURL::Equals, or add SubstitutingJARURIParams in URIParams for IPC?

I don't love that we need to enumerate every type of URI in the URIParams type in order to preserve it, but given that is how things work right now I think the most elegant solution is probably to add a case for it to URIParams. Right now we're explicitly throwing away the SubstitutingJARURI wrapper type when serializing (https://searchfox.org/mozilla-central/rev/7939a5150dcd96915bccf1c819433ad489a5edc9/netwerk/protocol/res/SubstitutingJARURI.h#149-152), which is unfortunate if it's important for some semantic reason or another (such as being able to access the nsIJARURI interface)

We shouldn't be implementing the NS_THIS_STANDARDURL_IMPL_CID, because code using that is expecting the concrete type to be actually nsStandardURL (https://searchfox.org/mozilla-central/rev/7939a5150dcd96915bccf1c819433ad489a5edc9/netwerk/base/nsStandardURL.cpp#2349-2351), meaning that the QI would likely behave incorrectly.

Comment 24

[Makoto Kato [:m_kato]]

Thanks. SubstitutingJARURI should implement Mutator to serialize to same type via IPC.

Comment 25

[mabangwang]

I see you've been discussing it all along. When can this bug be resolved? It's urgent

Comment 26

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/jobs?repo=try&revision=4a8eaa97f684ea40724b08800926476d6654faf2

Comment 27

[Makoto Kato [:m_kato]]

Attachment: Bug 1812356 - Part 1. Add SubstitutingJARURIParams to serialize SubstitutingJARURI. r=#necko-reviewers!,nika!

GeckoView supports resource://android/ URL to access package root. But when
using this URL, nsDocShellLoadState::ValidateWithOriginalState returns URI
mismatch because SubstitutingJARURI is serialized to other type.

So SubstitutingJARURI should be serialized to same type.

To serialize this via IPC, we return Mutator type. But this doesn't allow
to modify properties except for IPC serialization. But some Set* can return
NS_OK when having same value for ContentPrincipal::GetSiteOriginNoSuffix.

Comment 28

[Makoto Kato [:m_kato]]

Attachment: Bug 1812356 - Part 2. Add geckoview-junit test for resource://android/. r=#geckoview-reviewers

Depends on D175765

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Too late for the Fx113 RC at this point, but I'd still consider fixing this in a dot release if the stars align to do so.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b23d2491f2b7
Part 1. Add SubstitutingJARURIParams to serialize SubstitutingJARURI. r=nika,necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/34c9c6244dde
Part 2. Add geckoview-junit test for resource://android/. r=geckoview-reviewers,ohall

Comment 31

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b23d2491f2b7
https://hg.mozilla.org/mozilla-central/rev/34c9c6244dde

Comment 32

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:m_kato, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox113 to wontfix.

For more information, please visit BugBot documentation.