Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Enable having multiple browser tabs with a single node js and (headless) chrome instance #13

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

MichaelBuessemeyer
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer commented Dec 16, 2021

Hi @cmfcmf,

as you might have already heard I am currently working with you project. For my goals I want to have multiple instances of the browser morph open at the same time. While this is currently possible by having multiple instances of the node process and therefore also multiple chrome instance open at the same time, I wanted to do that with a single instance for the whole squeak image.

The benefits I hope to get are mainly performance reasons and that this might be the less RAM hungry approach (as each chrome instance is likely to eat a lot of RAM 🍽️ )

  • Each time a browser is opened first the node js script starts which then starts a chrome instance and then streams the screencast to squeak. Starting the node script and the browser takes quite some time. But once it is started the browser is quite smooth.
  • To tackle the issue of time to wait for the loading, I want to "imply" add the feature to open a new tab in the opened chrome instance of a single node script. Thus each node script supports multiple browser tabs and squeak / magic mouse only needs a single script (and chrome) instance to support multiple mmbrowsers.

For that I did / want to do the following:

  • To each message between the script and squeak I add an id assigned to the tab and the corresponding browser morph so that the script / squeak knows which tab / morph is addressed.
  • I want to stop the node js script once no browser / tab is running anymore.
  • I make the Node js process a systemwide singleton. If the squeak image requests the singleton process but non is running, a node js process is started automatically to open the requested page by the browser morph. Therefore the node script is started lazily and not running if not needed. If a script is already running a browser can simply as the process to open a new tab for it. The interface to start a new tab is completely independent from whether a node script is already running or not.
    • I achieve the singleton behaviour by adding a new class var to the process wrapper which holds the currently (lazily) existing instance of process wrapper for the whole image. A method on the class side of the process wrapper is given to return the lazy initialised singleton instance of the process wrapper. The browser morph thus always communicates with the running process by calling this access method. Thus if this way of interacting with the process wrapper is used, it is ensured that the process is running only once.

As you can see, there is still quite some stuff missing. So if you notice anything above that could be solved in a better way that my approach I described, please tell me :)

Note!:
This PR currently also includes changes not needed for the suggestion above as they are needed for my current project. These changes might not be a very clean but working solution. I will make sure to remove these changes before merging this pr. But I guess merging will not be anytime soon :)

  • An example for such a change is reactOpenOn: and connect: aUrl throwTimeoutError: aBoolean
  • And I noticed that my vscode formatter kicked in on the run.js which is why the change count is a little higher than expected 😅
    • Maybe adding a formatter to the project might help to reduce such conflicts. Is that ok for you if I add one?

@cmfcmf
Copy link
Owner

cmfcmf commented Dec 16, 2021

Hey Michael,

I think that it is super cool that you use MagicMouse and want to improve it!! I can imagine that it is not easy to navigate the poorly documented code I have written and the binary protocol used to communicate to the Node.js script. Feel free to reach out whenever you have any questions or want additional feedback!

For my goals I want to have multiple instances of the browser morph open at the same time. While this is currently possible by having multiple instances of the node process and therefore also multiple chrome instance open at the same time, I wanted to do that with a single instance for the whole squeak image.

I see, makes sense! The only potential problem I am seeing is that I am not sure whether Puppeteer supports video streaming of multiple tabs at the same time. Have you tried that yet? By the way, in case you haven't seen that yet, I had to patch Puppeteer to add support for video recoding via https://github.com/cmfcmf/MagicMouse/blob/d5e6c260877c204d22e1a69a6e75a82d91ea5fb9/patches/puppeteer-core+1.19.0.patch. We're also using a super old version of Puppeteer, but I am not sure if there would be any benefits from upgrading to the latest version (and re-applying the video recording patch to it).

The benefits I hope to get are mainly performance reasons and that this might be the less RAM hungry approach (as each chrome instance is likely to eat a lot of RAM 🍽️ )

One interesting thing that you might not be aware of that I learned recently regarding Chrome's startup behavior: If you start Chrome, and then start another instance of Chrome, the second instance will just exit and delegate the new tab creation to the first instance, so there should not be any difference to just opening another tab. HOWEVER, I am not sure if the same behavior applies when controlling Chrome via Puppeteer, so you could still see the performance improvements you mention in that case. Regardless of that, running only a single instance of the Node.js script and Puppeteer will definitely be an improvement!

Thus each node script supports multiple browser tabs and squeak / magic mouse only needs a single script (and chrome) instance to support multiple mmbrowsers.

Sounds good!

  • To each message between the script and squeak I add an id assigned to the tab and the corresponding browser morph so that the script / squeak knows which tab / morph is addressed.

👍

  • I want to stop the node js script once no browser / tab is running anymore.

👍 If you want to get extra fancy, you could also consider to start a 30 second (just a random number I came up with) timer in Squeak once the last browser closes and only then shut down the browser and Node.js script. This would mean that the browser does not immediately shut down completely if you have no browser open in Squeak or minimize a Squeak MMBrowser window for just a short time.

  • I make the Node js process a systemwide singleton. If the squeak image requests the singleton process but non is running, a node js process is started automatically to open the requested page by the browser morph. Therefore the node script is started lazily and not running if not needed. If a script is already running a browser can simply as the process to open a new tab for it. The interface to start a new tab is completely independent from whether a node script is already running or not.

👍

  • I achieve the singleton behaviour by adding a new class var to the process wrapper which holds the currently (lazily) existing instance of process wrapper for the whole image. A method on the class side of the process wrapper is given to return the lazy initialised singleton instance of the process wrapper. The browser morph thus always communicates with the running process by calling this access method. Thus if this way of interacting with the process wrapper is used, it is ensured that the process is running only once.

This sounds like a good sue case for the ProcessLocalVariable class!

Note!: This PR currently also includes changes not needed for the suggestion above as they are needed for my current project. These changes might not be a very clean but working solution. I will make sure to remove these changes before merging this pr. But I guess merging will not be anytime soon :)

No worries -- I am really excited to see a demo whenever it is ready!!

  • Maybe adding a formatter to the project might help to reduce such conflicts. Is that ok for you if I add one?

Sounds good! It would be amazing if you added the prettier formatter as part of a separate PR by running yarn add --dev prettier, adding separate "lint": "prettier --check ." and "format": "prettier --write ." scripts to package.json, and committing the now pretty-printed files after running yarn format.

@MichaelBuessemeyer
Copy link
Contributor Author

Hohoho @cmfcmf,

I wish you a happy christmas time 🎄.

Sorry for the late response. I had to a lot of other stuff 😅.

Thanks for your detailed answer.

I see, makes sense! The only potential problem I am seeing is that I am not sure whether Puppeteer supports video streaming of multiple tabs at the same time. Have you tried that yet? By the way, in case you haven't seen that yet, I had to patch Puppeteer to add support for video recoding via https://github.com/cmfcmf/MagicMouse/blob/d5e6c260877c204d22e1a69a6e75a82d91ea5fb9/patches/puppeteer-core+1.19.0.patch. We're also using a super old version of Puppeteer, but I am not sure if there would be any benefits from upgrading to the latest version (and re-applying the video recording patch to it).

That is actually a very good point you bring up here. I just tried out whether this is possible and luckily it is (with a little restriction).
I editied the testing script from here to the following (I paste the code here for "documentation purposes"):

"use strict";

const puppeteer = require("puppeteer-core");
const findChrome = require("chrome-finder");

function sleep(ms) {
  return new Promise((resolve) => setTimeout(resolve, ms));
}

(async () => {
  const browser = await puppeteer.launch({
    ignoreDefaultArgs: true,
    args: [
      "--enable-automation",
      "--headless",
      // '--start-fullscreen',
      "--force-device-scale-factor=1",
    ],
    executablePath: findChrome(),
  });
  const startPage = async (browser, id) => {
    const page = await browser.newPage();
    await page.goto("https://www.youtube.com/watch?v=dQw4w9WgXcQ");

    let frameCount = 0;
    page.on("screencastframe", async (frame) => {
      await page.screencastFrameAck(frame.sessionId);

      console.log(`got image from id ${id}`);
      frameCount++;

      if (frameCount > 15) await browser.close();
    });
    console.log("Starting tab with id ", id);
    await page.startScreencast({ format: "jpeg", everyNthFrame: 1 });
  };
  startPage(browser, 1);
  startPage(browser, 2);
  await sleep(2000);
})();

When In run this in none-headless-mode the screencast only works for the last opened tab. I guess the reason is that chrome only sends a frame update when the tab is actually rendered. But as the second tab opens up, the first isn't visible bile anymore and therefore not rendered. Thus there are no updates from the screencast. However the screencast works for the second / last opened tab.
If chrome is run in the ´headless-mode`, the screencasts seem to work perfectly asynchronous / in parallel. Here is the console output of the code above (on MacOS 12, node 14 and chrome 96)

Starting tab with id  1
got image from id 1
got image from id 1
got image from id 1
got image from id 1
got image from id 1
got image from id 1
got image from id 1
Starting tab with id  2
got image from id 1
got image from id 1
got image from id 2
got image from id 1
got image from id 2
got image from id 1
got image from id 2
got image from id 1
got image from id 2
got image from id 1
got image from id 2
got image from id 1
got image from id 1
got image from id 2
got image from id 1

I think updating puppeteer is not needed as the current version works perfectly fine for me :)

One interesting thing that you might not be aware of that I learned recently regarding Chrome's startup behavior: If you start Chrome, and then start another instance of Chrome, the second instance will just exit and delegate the new tab creation to the first instance, so there should not be any difference to just opening another tab.

Using the code above slightly modified I was able to test this. The result is that I can see two running processes of headless chrome instances in my htop (Which then each start a few more). Thus this behaviour seems not to kick in in case of using puppeteer with a headless chrome. I did not test this in none-headless-mode.
The code for the experiment is here:

"use strict";
const puppeteer = require("puppeteer-core");
const findChrome = require("chrome-finder");

function sleep(ms) {
  return new Promise((resolve) => setTimeout(resolve, ms));
}

const startBrowser = async () => {
  const browser = await puppeteer.launch({
    ignoreDefaultArgs: true,
    args: [
      "--enable-automation",
      "--headless",
      "--force-device-scale-factor=1",
    ],
    executablePath: findChrome(),
  });
  const startPage = async (browser, id) => {
    const page = await browser.newPage();
    await page.goto("https://www.youtube.com/watch?v=dQw4w9WgXcQ");

    let frameCount = 0;
    page.on("screencastframe", async (frame) => {
      await page.screencastFrameAck(frame.sessionId);

      console.log(`got image from id ${id}`);
      frameCount++;

      if (frameCount > 50) await page.close();
    });
    console.log("Starting tab with id ", id);
    await page.startScreencast({ format: "jpeg", everyNthFrame: 1 });
  };
  startPage(browser, 1);
  startPage(browser, 2);
  await sleep(20000);
  await browser.close();
};
Promise.all([startBrowser(), startBrowser()]);

👍 If you want to get extra fancy, you could also consider to start a 30 second (just a random number I came up with) timer in Squeak once the last browser closes and only then shut down the browser and Node.js script. This would mean that the browser does not immediately shut down completely if you have no browser open in Squeak or minimize a Squeak MMBrowser window for just a short time.

In my opinion a minimized browser would still count as opened. The timer idea sound nice 👍.

This sounds like a good sue case for the ProcessLocalVariable class!

Thanks for the suggestion. I'll take a look at the class.

@cmfcmf
Copy link
Owner

cmfcmf commented Jan 12, 2022

Sorry for the late response. I had to a lot of other stuff 😅.

No worries, I didn't really expect you to work over the holidays! And as you see, I also sometimes take a few days to respond :)

If chrome is run in the ´headless-mode`, the screencasts seem to work perfectly asynchronous / in parallel. Here is the console output of the code above (on MacOS 12, node 14 and chrome 96)

I see, good to know! I suppose it could be possible to open multiple windows instead of tabs in non-headless mode. I wonder whether that would allow to record the screen of multiple tabs in parallel. However, as you might have guessed, the non-headless mode really only is most useful for debugging, so that should be fine regardless.

Using the code above slightly modified I was able to test this. The result is that I can see two running processes of headless chrome instances in my htop (Which then each start a few more). Thus this behaviour seems not to kick in in case of using puppeteer with a headless chrome. I did not test this in none-headless-mode.

Good to know!

@cmfcmf
Copy link
Owner

cmfcmf commented Jan 12, 2022

await page.goto("https://www.youtube.com/watch?v=dQw4w9WgXcQ");

Nice example page by the way 👍

combine yarn commands; remove unused dependecies; adjust eslint rules
@MichaelBuessemeyer
Copy link
Contributor Author

@cmfcmf I just cleaned up this pr. If you got some time to spare, I would be glad if you could review it.

How it works:
It should work just like described above: There is only always one instance of an MMProcessWrapper running simultaneously. This process communicates with the node process and each message between these processes has an id attached which corresponds to a tab in the chrome process. Everything that was previously supported should still be supported. Additionally, it is possible to open a new tab via a message from the MMProcessWrapper instance and to take a screenshot from a webpage with a given extent. The screenshot-taking mechanism opens the given webpage in a new tab, waits for a second for it to load, and then takes a screenshot from that page. After that, the tab is closed again and the screenshot is sent over to Squeak.
I needed the screenshot functionality for the seminar. If you think it is not worth keeping as it is kinda out of order, just tell me and I'll remove it.

The high-level code changes of this pr:

  • Every command sent between Squeak and Node has an id attached indicating the matching between the Chrome Tab and the MMBrowserMorph.
  • Every node process-related method from MMBrowserMorph was moved to MMProcessWrapper.
  • The run.js script was adapted to handle multiple tabs and communicated with Squeak by adding a tab Id to every message send/received.

What I tested on MacOS and what you could do to test this pr:

  • First, save your image ;)
  • Execute MMBrowserMorph open. This should start a node process in the background and after some time google should pop up in the MMBrowserMorph that just opened up by that command.
  • Next, execute the same command again MMBrowserMorph open. This should now not start a new node instance but use the already started instance. Therefore google should show up very fast compared to when the first MMBrowserMorph was opened.
  • Play around a little with the browser (eg. google smth). The browsers should work as expected.
  • Inspect the running instance of MMProcessWrapper via executing MMProcessWrapper getOSDependentSingleton by pressing ctrl + i. Under browserMorphMapping there should be all instances of MMBrowserMorph listed.
  • Close the two MMBrowserMorph instances. The MMProcessWrapper should now automatically shut down the node process which is indicated via its isRunning instance variable.

Currently know issues / bugs:

  • As before, sometimes the stdin to the MMProcessWrapper instance gets stuck somehow. This prevents all browser instances from updating. This issue can be prevented by opening another browser because as long as some new images are streamed to the stdin, the stdin should not get stuck and therefore the browser should all be responsive again.
  • The debug to transcript option seems to be bugged in my version (on MacOS). The code somehow always gets stuck at trying to print something to the Transcript upon receiving some stderr output from the node process.
  • The clean-up upon closing Squeak does not work for me (MacOS). Upon shutDown: I try to close the currently running MMProcessWrapper instance, because otherwise when restarting Squeak this process still assumes a running node process. This let's error pop up right after starting Squeak which is kinda annoying. To repair this issue after starting Squeak again just execute MMProcessWrapper renewOSDependentSingleton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants