don't share event loop between windows when possible

[akosyakov]

All windows on the same origin share an event loop as they can synchronously communicate

from http://hassansin.github.io/shared-event-loop-among-same-origin-windows

As a consequence debugging in one window blocks all other windows for the same origin.

The obvious candidate is mini-browser. But we should review other places where we use window.open and pass rel=noopener when it makes sense, i.e. we did not need to communicate with an opened window.

[DanTup]

I wish I'd found this earlier - I've been debugging this most of the morning 🙈 The issue is when we hit a breakpoint in the web app, it causes GitPod to lock up and everything hangs.

I tested adding 'noopener' as the third argument to window.open here and it solved my issue:

theia/packages/core/src/browser/window/default-window-service.ts

Line 44 in aff11be

openNewWindow(url: string): Window | undefined {

However since this prevents it from returning a Window, it may make sense to change the interface to return void to avoid confusion (as far as I can tell, nothing is using the return value?).

[akosyakov]

@DanTup Could you send a PR please? Changing a return type is fine. I could not find any clients using the returned window object.

[DanTup]

@akosyakov I made a start, but I'm not getting very good errors in my IDE so I'm not sure if this is everything. I'm also not sure whether changing the undefineds to void throughout like this is desired?

DanTup@ce9c034

Does this seem reasonable?

[akosyakov]

I'm also not sure whether changing the undefineds to void throughout like this is desired?

OpenerService api should NOT be changed, it is used to open anything, not only a window, e.g. editors, preview widgets and so on. In such cases a widget will be returned. But other places look fine to me.

[DanTup]

OpenerService api should be changed

I presume there's a missing "not" there? I'll revert that then and use undefined instead of void.

Does it seem ok removing the error-throwing code in default-window-service since it'll always get an undefined return value now?

btw, I don't know if I can reliably test all of this locally.. I tested the noopener change on GitPod by just using the dev console to replace window.open with a wrapper that did passed this arg.