search

mozilla / positron

a experimental, Electron-compatible runtime on top of Gecko
Other
562 stars 64 forks source link

make the mozbrowser iframe remote #76

Closed mykmelez closed 8 years ago

mykmelez commented 8 years ago

When the <iframe mozbrowser> in the shadow DOM of the <webview> element is remote, it doesn't render its contents. This appears to be a regression from the way we give the BrowserWindow document chrome privileges by assigning it the system principal in nsDocument::reset. We currently work around it by making the iframe not be remote.

That way of giving the BrowserWindow document chrome privileges is probably incorrect, and a fix for that might end up fixing this issue. But I'm filing it here to track it discretely, since it's potentially a separate issue.

We should make the mozbrowser iframe remote because it's more consistent with the behavior of Electron (which creates a renderer process for each webview) and has all the other benefits of e10s wrt. chrome UI responsiveness.

mykmelez commented 8 years ago

@jryans Do you have any ideas about the root cause of this by any chance?

jryans commented 8 years ago

Hmm, at moment I am not sure. I have actually seen the reverse: for my usage outside of Positron, it only renders when it is remote. How do web components interact with nsFrameLoader::Show? Is your iframe considered remote by the time that runs, or does it run before you had a chance to set it?

mykmelez commented 8 years ago

Hmm, at moment I am not sure. I have actually seen the reverse: for my usage outside of Positron, it only renders when it is remote. How do web components interact with nsFrameLoader::Show? Is your iframe considered remote by the time that runs, or does it run before you had a chance to set it?

nsFrameLoader::Show is called and calls IsRemoteFrame twice during startup of the basic browser app, and if the <webview> implementation is setting the 'remote' attribute (i.e. https://github.com/mozilla/positron/blob/639915958d8dc8a51fd5e5f1133bd3fb1c0df962/positron/electron/lib/renderer/web-view/web-view.js#L296-L298 is uncommented), then IsRemoteFrame returns true the first time and false the second time.

I also see the message "Content sandbox disabled due to sandbox level setting" in the terminal. And I get a download dialog for a file that turns out to be the HTML document at the URL that the <webview> is loading.

mykmelez commented 8 years ago

nsFrameLoader::Show is called and calls IsRemoteFrame twice during startup of the basic browser app, and if the implementation is setting the 'remote' attribute (i.e. https://github.com/mozilla/positron/blob/639915958d8dc8a51fd5e5f1133bd3fb1c0df962/positron/electron/lib/renderer/web-view/web-view.js#L296-L298 is uncommented), then IsRemoteFrame returns true the first time and false the second time.

I hooked up a debugger to nsFrameLoader::Show, and the second time it's called, mURIToLoad.mBaseURI is about:devtools-toolbox?type=window&id=3, which explains why it isn't a remote frame.

The first time, mURIToLoad.mBaseURI is file:///Users/myk/Projects/electron-sample-apps/webview/browser/browser.html, which is the URL of the BrowserWindow (in my local clone of the sample browser app). And mURIToLoad itself is about:blank, which is presumably the initial URL that loads in the mozbrowser iframe. So that frame is correctly set to load remotely.

mykmelez commented 8 years ago

Ah, interesting, it looks like this only happens on http://www.github.com/. And it's unrelated to the <webview>, as it happens if I insert an <iframe mozbrowser> directly into an app. Other sites, like https://mozilla.org/ and http://melez.com/, work as expected.

jryans commented 8 years ago

Maybe it's related to GitHub's usage of X-Frame-Options: deny and / or Content-Security-Policy? If mozbrowser is working correctly, these headers shouldn't affect you since it's supposed to be the root browsing context, but perhaps something is confused still, and it's caused these headers to give you trouble? Perhaps a test page that sends these headers could help diagnose the exact cause.