1161435 - netErrorURL should be resolved from start-manifest url

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Runtime, defect)

Comment 1

[Paul Rouget [:paul]]

Attachment: version 1

Comment 2

[Paul Rouget [:paul]]

Attachment: version 2

Comment 3

[Paul Rouget [:paul]]

Fabrice, we want to be able to define the network error URL as relative to the start-manifest URL.

The start-manifest URL is only accessible in the parent process.

Here are 2 versions of the same idea. One where the netErrorURL is resolved in the parent process, one where only the startManifestURL is communicated to the child process, where the full URL is resolved.

The 2nd approach might be a tiny bit better because later we might want to do something similar again (querying the startManifestURL from the parent process).

Maybe we could also store the manifest URL in a pref or a setting.

Comment 4

[Paul Rouget [:paul]]

Apparently, <script src="foo.js"> is not found. I'm suspecting gecko to resolve foo.js from the original URI, not the new one. I don't know how to work around that.

Comment 5

[[:fabrice] Fabrice Desré]

The start manifest url is in a pref, it's b2g.system_manifest_url. You should be able to just use that instead of remoting to the parent.

I faced the same url resolution issue when I did a quick hack for that. It works with app:// urls but not http(s) ones probably because of the way we treat the originalUri in the app:// protocol handler.

Comment 6

[Paul Rouget [:paul]]

(In reply to Fabrice Desré [:fabrice] from comment #5)
> The start manifest url is in a pref, it's b2g.system_manifest_url. You
> should be able to just use that instead of remoting to the parent.

It is not when you use --start-manifest.

Comment 7

[[:fabrice] Fabrice Desré]

(In reply to Paul Rouget [:paul] from comment #6)
> (In reply to Fabrice Desré [:fabrice] from comment #5)
> > The start manifest url is in a pref, it's b2g.system_manifest_url. You
> > should be able to just use that instead of remoting to the parent.
> 
> It is not when you use --start-manifest.

Then we should fix that.

Comment 8

[Paul Rouget [:paul]]

(In reply to Fabrice Desré [:fabrice] from comment #7)
> (In reply to Paul Rouget [:paul] from comment #6)
> > (In reply to Fabrice Desré [:fabrice] from comment #5)
> > > The start manifest url is in a pref, it's b2g.system_manifest_url. You
> > > should be able to just use that instead of remoting to the parent.
> > 
> > It is not when you use --start-manifest.
> 
> Then we should fix that.

That's what I'm saying in comment 3. But we can't just override b2g.system_manifest_url, so should we use a new pref name? Like b2g.effective_system_manifest_url or something…

Comment 9

[Paul Rouget [:paul]]

Wait, I didn't realize there was 2 prefs. system_manifest_url and start_manifest_url.

Comment 10

[Paul Rouget [:paul]]

Attachment: v2

That's much better :)

Comment 11

[[:fabrice] Fabrice Desré]

Comment on attachment 8602578 [details] [diff] [review]
v2

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

::: b2g/components/B2GAboutRedirector.js
@@ +20,5 @@
> +  // new URL("error.html", "http://foo.com/index.html") -> http://foo.com/error.html
> +  // new URL("http://bar.com/error.html", "http://foo.com/index.html") -> http://bar.com/error.html
> +  // new URL("http://bar.com/error.html", undefined) -> http://bar.com/error.html
> +  // new URL("error.html", undefined) -> throw "error.html is not a valid URL."
> +  return new URL(netErrorURL, systemManifestURL).href;

nit: since you import Services.jsm, you could use Services.io.newURI(netErrorURL, null, systemManifestURL).spec and not have to importGlobalProperties();

Comment 12

[Paul Rouget [:paul]]

Attachment: v3

Addressed Fabrice's comment.

Comment 13

[Paul Rouget [:paul]]

Attachment: v3 without graphene changes. r=fabrice

I'd like to land this in m-c, and land the graphene part in larch from bug 1134106.

Comment 14

[Paul Rouget [:paul]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b59341f21cc

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/60854c327e1c

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/60854c327e1c