mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix: window.open doesn't work correctly in child window (#25080)

Browse source 
If speculative render view host is deleted during naviagation to
page we try to open with call to window.open window is destroyed
right after it is created. It may happen when naviagation triggers
redirect from http to https. To void that we should emit event
current-render-view-deleted only when render view host which was
destroyed is the current webcontent's rvh.
This commit is contained in:
CezaryKulakowski 2020-10-08 01:11:19 +02:00 committed by GitHub
parent 3e627f7700
commit 73adeef877
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
shell/browser/api/electron_api_web_contents.cc
View file

@ -1066,22 +1066,13 @@ void WebContents::RenderFrameCreated(
rwh_impl->disable_hidden_ = !background_throttling_;
}
void WebContents::RenderViewHostChanged(content::RenderViewHost* old_host,
content::RenderViewHost* new_host) {
currently_committed_process_id_ = new_host->GetProcess()->GetID();
}
void WebContents::RenderViewDeleted(content::RenderViewHost* render_view_host) {
// This event is necessary for tracking any states with respect to
// intermediate render view hosts aka speculative render view hosts. Currently
// used by object-registry.js to ref count remote objects.
Emit("render-view-deleted", render_view_host->GetProcess()->GetID());
if (-1 == currently_committed_process_id_ ||
render_view_host->GetProcess()->GetID() ==
currently_committed_process_id_) {
currently_committed_process_id_ = -1;
if (web_contents()->GetRenderViewHost() == render_view_host) {
// When the RVH that has been deleted is the current RVH it means that the
// the web contents are being closed. This is communicated by this event.
// Currently tracked by guest-window-manager.js to destroy the

6
shell/browser/api/electron_api_web_contents.h
View file

@ -556,8 +556,6 @@ class WebContents : public gin::Wrappable<WebContents>,
const base::TimeTicks& proceed_time) override;
void RenderViewCreated(content::RenderViewHost* render_view_host) override;
void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
void RenderViewHostChanged(content::RenderViewHost* old_host,
content::RenderViewHost* new_host) override;
void RenderViewDeleted(content::RenderViewHost*) override;
void RenderProcessGone(base::TerminationStatus status) override;
void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
@ -696,10 +694,6 @@ class WebContents : public gin::Wrappable<WebContents>,
bool initially_shown_ = true;
// The ID of the process of the currently committed RenderViewHost.
// -1 means no speculative RVH has been committed yet.
int currently_committed_process_id_ = -1;
service_manager::BinderRegistryWithArgs<content::RenderFrameHost*> registry_;
mojo::ReceiverSet<mojom::ElectronBrowser, content::RenderFrameHost*>
receivers_;

25
spec-main/api-web-contents-spec.ts
View file

@ -1156,6 +1156,10 @@ describe('webContents module', () => {
res.end();
} else if (req.url === '/redirected') {
res.end('<html><script>window.localStorage</script></html>');
} else if (req.url === '/first-window-open') {
res.end(`<html><script>window.open('${serverUrl}/second-window-open', 'first child');</script></html>`);
} else if (req.url === '/second-window-open') {
res.end('<html><script>window.open(\'wrong://url\', \'second child\');</script></html>');
} else {
res.end();
}
@ -1192,6 +1196,27 @@ describe('webContents module', () => {
expect(currentRenderViewDeletedEmitted).to.be.false('current-render-view-deleted was emitted');
});
it('does not emit current-render-view-deleted when speculative RVHs are deleted and nativeWindowOpen is set to true', async () => {
const parentWindow = new BrowserWindow({ show: false, webPreferences: { nativeWindowOpen: true } });
let currentRenderViewDeletedEmitted = false;
let childWindow:BrowserWindow;
const destroyed = emittedOnce(parentWindow.webContents, 'destroyed');
const renderViewDeletedHandler = () => {
currentRenderViewDeletedEmitted = true;
};
app.once('browser-window-created', (event, window) => {
childWindow = window;
window.webContents.on('current-render-view-deleted' as any, renderViewDeletedHandler);
});
parentWindow.loadURL(`${serverUrl}/first-window-open`);
setTimeout(() => {
childWindow.webContents.removeListener('current-render-view-deleted' as any, renderViewDeletedHandler);
parentWindow.close();
}, 500);
await destroyed;
expect(currentRenderViewDeletedEmitted).to.be.false('child window was destroyed');
});
it('emits current-render-view-deleted if the current RVHs are deleted', async () => {
const w = new BrowserWindow({ show: false });
let currentRenderViewDeletedEmitted = false;