1504968 - Fix missing triggeringPrincipal from android code

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P2)

Description

[Jonathan Kingston [:jkt] he/him]

In Bug 1490257 we made a carve out for the asserts for android. This is mainly due to browser.js in android browsers not passing triggering principals in most places.

This bug is so we follow up on that and require this everywhere.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Some notes: It seems we don't have the right principal on all of android, within Bug 1513241 I encountered some of those problems, see also [1].

Within this Bug we should update loadURIOptions.webidl and change the principal setting to:

required Principal? triggeringPrincipal;

If we do that currently, we face problems like [1], in detail:

0 loadURI/<() ["chrome://global/content/elements/browser-custom-element.js":809:6]
1 _wrapURIChangeCall() ["chrome://global/content/elements/browser-custom-element.js":749:8]
2 loadURI() ["chrome://global/content/elements/browser-custom-element.js":808:4]
3 create() ["chrome://browser/content/browser.js":3855:8]
4 Tab() ["chrome://browser/content/browser.js":3642:2]
5 addTab() ["chrome://browser/content/browser.js":1216:17]
6 onEvent() ["chrome://browser/content/browser.js":1965:10]
Handled load error: TypeError: Missing required 'triggeringPrincipal' member of LoadURIOptions

It seems that |this.browser.contentPrincipal| within mobile/android/chrome/content/browser.js is null. So probably we should start digging there.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=61ab3e53d35dba0a5db9c20394a7000cd301bef7

Comment 2

[Johannes Pfrang [:johnp]]

Could it be that this just works now in Fenix? I set security.strict_security_checks.enabled to true and don't see such an error in the remote debugging browser console. Furthermore I can switch to different geckoview.xhtml tabs and see a non-null, seemingly correct this.browser.contentPrincipal. I suppose this is due to GeckoView? Since Android Nightly switched to Fenix by now, this could maybe be fixed and ride the trains?

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Johannes Pfrang [:johnp] from comment #2)

Could it be that this just works now in Fenix? I set security.strict_security_checks.enabled to true and don't see such an error in the remote debugging browser console. Furthermore I can switch to different geckoview.xhtml tabs and see a non-null, seemingly correct this.browser.contentPrincipal. I suppose this is due to GeckoView? Since Android Nightly switched to Fenix by this could maybe be fixed and ride the trains?

Yes, that's entirely possible that that just works using Fenix. I would be happy to remove "security.strict_security_checks.enabled". If you want me to, I'll take on that bug. Would be really happy if that is really true and we have the right triggeringPrincipal on Android now.

Let me know!

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #3)

(In reply to Johannes Pfrang [:johnp] from comment #2)

Could it be that this just works now in Fenix? I set security.strict_security_checks.enabled to true and don't see such an error in the remote debugging browser console. Furthermore I can switch to different geckoview.xhtml tabs and see a non-null, seemingly correct this.browser.contentPrincipal. I suppose this is due to GeckoView? Since Android Nightly switched to Fenix by this could maybe be fixed and ride the trains?

Yes, that's entirely possible that that just works using Fenix. I would be happy to remove "security.strict_security_checks.enabled". If you want me to, I'll take on that bug. Would be really happy if that is really true and we have the right triggeringPrincipal on Android now.

Let me know!

I'll just try that - if we could enforce strict top-level principal checks, that would make me just feel so much better!

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1504968: Remove pref security.strict_security_checks.enabled and enforce strict top-level principal checking on Android. r=baku,snorp

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Let's hope that really works everywhere:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=73f847e0871e1c4b631266c50637afc9c1c4ed78

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/739832ead584
Remove pref security.strict_security_checks.enabled and enforce strict top-level principal checking on Android. r=snorp,baku

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/739832ead584