Closed mykmelez closed 8 years ago
mykmelez
commented
8 years ago Ah, hmm, setting the mozbrowser and remote attributes immediately in the createdCallback for the browserplugin custom element (3f661b9) resolves that problem, although it then reveals another: an alert dialog appears that says, "This page has a content security policy that prevents it from being loaded in this way." And the URL still doesn't load.
mykmelez
commented
8 years ago Ah, hmm, setting the mozbrowser and remote attributes immediately in the createdCallback for the browserplugin custom element (3f661b9) resolves that problem, although it then reveals another: an alert dialog appears that says, "This page has a content security policy that prevents it from being loaded in this way." And the URL still doesn't load.
Ah, this is triggered by nsDocument::InitCSP, presumably because github.com's CSP doesn't permit framing, and that function thinks it's being loaded in a frame, since there's an mDocumentContainer docshell for the mozbrowser.
Perhaps we just need to check nsDocShell::GetIsMozBrowserOrApp before deciding whether or not to enforce the frame-ancestor policy, although it returns false in this case, as mFrameType is FRAME_TYPE_REGULAR for the docshell, which seems wrong. So perhaps it's being set incorrectly somewhere.
It could also be an issue that nsCSPContext::PermitsAncestry generates ancestorsArray by walking the docshell tree until it reaches a docshell whose document URI scheme is "chrome", which won't be the case here, as the URL of the document that contains the mozbrowser will be a file: URL like file:///Users/myk/Projects/positron/positron/test/hello-world/index.html.
jryans
commented
8 years ago Perhaps we just need to check nsDocShell::GetIsMozBrowserOrApp before deciding whether or not to enforce the frame-ancestor policy, although it returns false in this case, as mFrameType is FRAME_TYPE_REGULAR for the docshell, which seems wrong. So perhaps it's being set incorrectly somewhere.
You should get FRAME_TYPE_BROWSER if mozbrowser is set in time and you have permission to use it. It's set by nsFrameLoader::MaybeCreateDocShell. The attribute and permissions check happens in nsGenericHTMLFrameElement::GetReallyIsBrowserOrApp. In desktop Firefox, we have permission implicitly because the embedding page is has the system principal (nsContentUtils::IsSystemPrincipal returns true). Perhaps check what GetReallyIsBrowserOrApp thinks for your setup?
mykmelez
commented
8 years ago @brendandahl Ok, this is ready for review!
The Electron implementation is complex, but the basic idea is that each <webview> node has a "browser plugin" <object> node in its shadow DOM, a WebViewImpl object in the renderer process, and a WebContents "guest" in the main process.
The "browser plugin" node is the actual rendering frame, the equivalent of an <iframe mozbrowser> in Gecko. And the WebContents guest in the main process is responsible for handling all interaction between the "embedder" WebContents (the BrowserWindow that embeds the <webview> element) and the "guest" WebContents, which are supposed to be in separate processes.
There's a complex sequence of initialization steps when a <webview> is created (or parsed from a document being loaded into a BrowserWindow): a guest WebContents is created in the main process, then the guest is "attached" to the <webview> (and added to the web view manager, and possibly also the web frame manager—not the same!), and then various other things happen. Oh, and there's a proxy guest in the renderer process that forwards method calls to the actual WebContents guest in the main process via IPC. And, and…
A Positron implementation could be much simpler, since the main and renderer processes are actually the same process, and since the renderer process could access the <iframe mozbrowser> API directly (even after we make it "remote"). But in the interest of maintaining as much Electron compatibility as possible, I minimized changes to the implementation, preferring to support Electron's existing model as much as possible.
The only significant difference is that I made the renderer process pass the WebViewImpl instance back to the main process, so the guest can access the <iframe mozbrowser> directly in order to implement the WebContents API more simply. We could instead make all the WebContents API calls pass messages back to some object in the renderer process that then accesses the <iframe mozbrowser>, but that would be cumbersome and entail even more significant changes to Electron. Let's avoid that for now.
This is a partial implementation of the <webview> element. It's sufficient to run the webview/browser/ sample app in https://github.com/hokein/electron-sample-apps, and thus it achieves the M2: Run Basic Browser App milestone.