Resolve window errors when platform debugging

[jryans]

I was able to get around the issues from #48 by ensuring we always have a window, and during early startup, we can use the hidden window for that purpose. This allows us to revert the various _progressListener guards that were added recently.

Overall the tools are working much better now in the platform debugger. However, I did notice that typing in the console triggers autocompletion that seems to send things over Positron's IPC mechanism, but it fails for some reason. I did not work that part out yet, but added a small debugging aid to improve the error at least. Console evaluation should now be working, so you ignore these autocomplete errors for now.

Fixes #48. @mykmelez, want to take a look?

[mykmelez]

Overall the tools are working much better now in the platform debugger. However, I did notice that typing in the console triggers autocompletion that seems to send things over Positron's IPC mechanism, but it fails for some reason. I did not work that part out yet, but added a small debugging aid to improve the error at least. Console evaluation should now be working, so you ignore these autocomplete errors for now.

The reason for this is quite complex. For renderer processes, I implemented Node's process global via a WebIDL binding, because it needs to be available not only to modules but also the page loaded into the window.

That means it gets added to every DOMWindow, including the hidden window! However, it's lazily instantiated, so it doesn't get instantiated when the hidden window loads. Once you evaluate something in the devtools console, which is hooked up to the hidden window, however, the console does something (iterate properties?) that causes process to be instantiated, at which point:

1. the WebIDL binding creates an instance of the Process XPCOM component;
2. the Process instance calls ModuleLoader.getLoaderForWindow, which creates an instance of ModuleLoader for the hidden window;
3. the ModuleLoader instance loads Electron's renderer/init.js to initialize the Electron environment;
4. initialization includes this call in override.js to remote.getCurrentWindow();
5. getCurrentWindow in remote.js calls ipcRenderer.sendSync('ATOM_BROWSER_CURRENT_WINDOW');
6. that triggers IPC to the main process, where it's handled by BrowserWindow.prototype.receiveMessage, which returns undefined because the window from which the message was sent is not the window to which the particular instance of BrowserWindow belongs.

So it never reaches the ATOM_BROWSER_CURRENT_WINDOW handler in rpc-server.js, although I don't think that handler would successfully return a window if the message did make it that far, since there isn't a BrowserWindow (nor WebContents) for the hidden window.

There are a couple lessons here.

First, since ipcRenderer.sendSync only expects to receive a single return value, we should ensure that BrowserWindow.prototype.receiveMessage also returns a single value from the correct BrowserWindow instance. Currently, it'll return an array of values, one for each BrowserWindow instance, and atom_renderer_ipc always returns the first item in the array, even though the actual return value may be a subsequent item. (We get away with that for now because we've only been testing apps with one BrowserWindow.) Perhaps BrowserWindow.prototype.receiveMessage should be a static method that receives all IPC messages and routes them to the correct BrowserWindow instance.

Second, we should ensure that the hidden window doesn't try to initialize an Electron environment, since that environment should only exist in the windows that the Electron app opens. It might be possible to restrict the process global via the WebIDL binding with a Func attribute, although it looks like we'd have to implement it in C++, even though the Process component is in JS. Alternately, the Process component might be able to work around the problem by not created a ModuleLoader instance. Then process would still exist in the hidden window, but it wouldn't trigger Electron initialization.

[mykmelez]

First, since ipcRenderer.sendSync only expects to receive a single return value, we should ensure that BrowserWindow.prototype.receiveMessage also returns a single value from the correct BrowserWindow instance. Currently, it'll return an array of values, one for each BrowserWindow instance, and atom_renderer_ipc always returns the first item in the array, even though the actual return value may be a subsequent item. (We get away with that for now because we've only been testing apps with one BrowserWindow.) Perhaps BrowserWindow.prototype.receiveMessage should be a static method that receives all IPC messages and routes them to the correct BrowserWindow instance.

I've filed issue #55 on this problem, since it's unrelated to the problem you're solving here, and your changes merely expose this issue.

Second, we should ensure that the hidden window doesn't try to initialize an Electron environment, since that environment should only exist in the windows that the Electron app opens. It might be possible to restrict the process global via the WebIDL binding with a Func attribute, although it looks like we'd have to implement it in C++, even though the Process component is in JS. Alternately, the Process component might be able to work around the problem by not created a ModuleLoader instance. Then process would still exist in the hidden window, but it wouldn't trigger Electron initialization.

This one isn't entirely related either, but I'd like to at least work around it in this changeset, even if we don't completely fix it.

[mykmelez]

This one isn't entirely related either, but I'd like to at least work around it in this changeset, even if we don't completely fix it.

I implemented a prospective workaround in https://github.com/jryans/positron/pull/3.

[jryans]

@mykmelez, I've rolled in your fix and addressed the other review comments. What do you think?

[mykmelez]

@mykmelez, I've rolled in your fix and addressed the other review comments. What do you think?

Looks great!

[jryans]

This will be upstreamed as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1275942.