cannot view chrome:// xul urls under e10s

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: jlongster, Assigned: mconley)

Tracking

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
If I have the simplest xul page:

main.xul:

<?xml version="1.0" encoding="utf-8"?>
<!DOCTYPE window>
<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
  <body>
    Hello.
  </body>
</html>

And it's accessible under chrome://my-addon/content/main.xul, if I try to navigate straight to that URL in my browser under e10s (nightly), it just won't load. I don't see any errors anywhere. It simply does nothing, the UI doesn't change at at all.

If I disable e10s, the xul page properly comes up.
The "allow-remote-chrome" patch from bug 1059007 means that chrome:// URLs are now loaded in the content process, but this could be causing the broken behavior you see here.

Changeset: http://hg.mozilla.org/mozilla-central/rev/0d7122479a34
(Reporter)

Comment 2

4 years ago
Thanks! Pinging mconley since he's the one who committed.
Flags: needinfo?(mconley)
Committed, but didn't write it. :) That's OK though.

Hey jlongster - are you able to try your STR in a debug build? I'm interested in any console spew you might get when you make the request.
Flags: needinfo?(mconley) → needinfo?(jlong)
(Reporter)

Comment 4

4 years ago
Here's the output:

[Parent 78141] WARNING: NS_ENSURE_TRUE(resultIndex >= 0) failed: file /Users/james/projects/mozilla/gecko-dev/toolkit/components/autocomplete/nsAutoCompleteController.cpp, line 1552
[Parent 78141] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /Users/james/projects/mozilla/gecko-dev/dom/events/ContentEventHandler.cpp, line 110
++DOMWINDOW == 7 (0x10ffde000) [pid = 78146] [serial = 10] [outer = 0x111797400]
[Parent 78141] WARNING: No docshells for remote frames!: file /Users/james/projects/mozilla/gecko-dev/dom/base/nsFrameLoader.cpp, line 511

I forget who said it, but someone in #devtools said xul is not allowed in e10s, but that it should kick back to the main process and render if there. Maybe that hasn't been implemented though?
Flags: needinfo?(jlong)
(In reply to James Long (:jlongster) from comment #4)
> Here's the output:
> 
> [Parent 78141] WARNING: NS_ENSURE_TRUE(resultIndex >= 0) failed: file
> /Users/james/projects/mozilla/gecko-dev/toolkit/components/autocomplete/
> nsAutoCompleteController.cpp, line 1552
> [Parent 78141] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result
> 0x80040111: file
> /Users/james/projects/mozilla/gecko-dev/dom/events/ContentEventHandler.cpp,
> line 110
> ++DOMWINDOW == 7 (0x10ffde000) [pid = 78146] [serial = 10] [outer =
> 0x111797400]
> [Parent 78141] WARNING: No docshells for remote frames!: file
> /Users/james/projects/mozilla/gecko-dev/dom/base/nsFrameLoader.cpp, line 511
> 

Thanks!

> I forget who said it, but someone in #devtools said xul is not allowed in
> e10s, but that it should kick back to the main process and render if there.
> Maybe that hasn't been implemented though?

Ah, yes - I would not be surprised if we haven't included the XUL content handler in the content process. I also believe that we're not likely to include it in the future.

So, yes, rendering it in the parent process seems like the right idea. I wonder if perhaps the simplest solution (looking for a .xul suffix in E10SUtils.shouldBrowserBeRemote) would be sufficient. 

Hey felipe, can you think of any problems with that approach?
Flags: needinfo?(felipc)
Assignee: nobody → mconley
Blocks: 905436
tracking-e10s: --- → m4+

Updated

4 years ago
Blocks: 1100955
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #5)
> (In reply to James Long (:jlongster) from comment #4)
> > Here's the output:
> > 
> > [Parent 78141] WARNING: NS_ENSURE_TRUE(resultIndex >= 0) failed: file
> > /Users/james/projects/mozilla/gecko-dev/toolkit/components/autocomplete/
> > nsAutoCompleteController.cpp, line 1552
> > [Parent 78141] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result
> > 0x80040111: file
> > /Users/james/projects/mozilla/gecko-dev/dom/events/ContentEventHandler.cpp,
> > line 110
> > ++DOMWINDOW == 7 (0x10ffde000) [pid = 78146] [serial = 10] [outer =
> > 0x111797400]
> > [Parent 78141] WARNING: No docshells for remote frames!: file
> > /Users/james/projects/mozilla/gecko-dev/dom/base/nsFrameLoader.cpp, line 511
> > 
> 
> Thanks!
> 
> > I forget who said it, but someone in #devtools said xul is not allowed in
> > e10s, but that it should kick back to the main process and render if there.
> > Maybe that hasn't been implemented though?
> 
> Ah, yes - I would not be surprised if we haven't included the XUL content
> handler in the content process. I also believe that we're not likely to
> include it in the future.
> 
> So, yes, rendering it in the parent process seems like the right idea. I
> wonder if perhaps the simplest solution (looking for a .xul suffix in
> E10SUtils.shouldBrowserBeRemote) would be sufficient. 

I'd rather keep this function as straightforward as possible. It looks like we only made chrome load in the remote process to keep session store tests happy. Is there any reason those tests can't use http urls instead? And any other reason to load chrome in the remote process?

Going forward I was considering a chrome.manifest flag that would mark chrome namespaces that should be loaded in the remote process
We discussed during the last triage that if this ended up looking like more than a check for the .xul suffix on a chrome URI, then we'd kick it out of M4 (we only wanted it on M4 for the easy stats. :) )
tracking-e10s: m4+ → ?
Flags: needinfo?(felipc)
Bill, is there any reason to have chrome: pages run in the child process other than fixing sessionstore tests?
Flags: needinfo?(wmccloskey)
I don't know that it's just limited to session store tests. I imagine that lots of tests use chrome URLs. There are 138 uses of getRootDirectory in the tree, although that's not a very accurate measure of chrome URL use.

Also, I don't know how easy it would be to fix them. Some of the tests probably use a chrome URL because they want chrome privileges.

I don't see the harm in adding an extra test to shouldBrowserBeRemote.
Flags: needinfo?(wmccloskey)
Created attachment 8526298 [details] [diff] [review]
[e10s] Blacklist chrome:// XUL urls from loading in the content process. r=?
This patch uses the simple suffix-check approach, in case we want to go with it.
Every test we add makes answering the question "will this URI load in the remote process" more confusing for both us and extension developers. If tests are our only concern then my suggestion of being able to flag chrome namespaces to be loaded in content would make it possible to just load the entire tests chrome namespace. This is cleaner than having particular files loaded remotely.
tracking-e10s: ? → +
Duplicate of this bug: 1105242

Comment 14

4 years ago
Consider making use of a separate url protocol. Simple and straightforward to identify. (Can't comment on how difficult to change code it would be.)

Comment 15

4 years ago
Will this be fixed before e10s is moved to the beta channel? It's a blocking issue for my extension.
(In reply to Josep del Rio from comment #15)
> Will this be fixed before e10s is moved to the beta channel? It's a blocking
> issue for my extension.

I have a patch almost ready for bug 1083281 that will make chrome: URIs default to loading in the main process again.
Depends on: 1083281

Comment 17

4 years ago
(In reply to Dave Townsend [:mossop] from comment #16)
> I have a patch almost ready for bug 1083281 that will make chrome: URIs
> default to loading in the main process again.

Great! Thanks a lot for the information.

Comment 18

4 years ago
Not working till today...
Duplicate of this bug: 1123782
Hi Dave. I was thinking about this bug some more. The patch here won't work because some add-ons load chrome:// URLs that end in .html, and they need to load in the parent process. On the other hand, I think we need a way to load privileged pages in the content process for tests. The test in bug 1113967 is one example. I don't think the patch in bug 1083281 would help with that one.

What if we load all chrome: and resource: URLs in the parent process unless they end in "?remote=true"? Then any test that wanted to be remote could just append that to the URL.
Flags: needinfo?(dtownsend)
(In reply to Bill McCloskey (:billm) from comment #20)
> Hi Dave. I was thinking about this bug some more. The patch here won't work
> because some add-ons load chrome:// URLs that end in .html, and they need to
> load in the parent process. On the other hand, I think we need a way to load
> privileged pages in the content process for tests. The test in bug 1113967
> is one example. I don't think the patch in bug 1083281 would help with that
> one.
> 
> What if we load all chrome: and resource: URLs in the parent process unless
> they end in "?remote=true"? Then any test that wanted to be remote could
> just append that to the URL.

The patch I have for bug 1083281 will allow you to define in the manifest whether a chrome namespace loads remotely or not and sets up a chrome space for each for all tests so will allow what you're after.
Flags: needinfo?(dtownsend)

Comment 23

4 years ago
For me looks that Firefox 38.0a1 (2015-02-03) is solving the problem. isn`t it?
Now that bug 1083281 is fixed chrome URLs will again by default load in the chrome process.

If you need to load a chrome URL in the child process (where you can't use XUL) then you can add "remoterequired=true" to the content line in the manifest. Then any URLs from that chrome namespace will load in the content process.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.