Skip to content

webbrowser: implement Webbrowser / ChromiumWebbrowser #5386

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 3, 2023
Merged

webbrowser: implement Webbrowser / ChromiumWebbrowser #5386

merged 5 commits into from
Jul 3, 2023

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Jun 17, 2023 •
edited
Loading

Part 2/4 of #5380

Opening this as a draft was well until I have the rest finished.

This

  1. Adds a utility executable resolver function with support for custom values, default names and fallback paths.
  2. Adds a utility function for finding an available port on a specific network interface
    • No tests, because I don't think mocking everthing here makes sense, and neither does running this without mocking
  3. Adds the trio dependency, as well as pytest-trio and trio-typing
    • trio-typing looks like it's going to get merged into trio soon
    • pytest-trio is used for initializing the runloop in async tests, similar to pytest-asyncio.
      Having implementations for both asyncio and trio seems to cause minor issues with trio's runloop in the tests, and on Windows it triggers a warning when asyncio was run prior to its own runloop. This is related to signal.set_wakeup_fd(). This can be resolved in the future by re-implementing the utils.processoutput based on trio instead of asyncio. The other asyncio stuff is purely concurrency related and not I/O related, so it's irrelevant which runloop we're using. The warning can be ignored for now.
  4. Adds the Webbrowser base class with the basic launch implementation
    • I initially had custom process cancellation logic added, when the process doesn't end on SIGTERM on POSIX systems. The default behavior of trio waits for 5 seconds and then sends SIGKILL. My plan was to wait a bit less and log a warning instead of triggering a warning via the warnings system, but I ran into issues with the custom implementation while testing. Some weird internal issues with trio and the stdlib subprocess module. Doesn't matter though.
  5. Adds the ChromiumWebbrowser class
    • This includes a list of all known Chromium/Chrome executable names (with Chromium having higher priority)
    • With fallback paths for Windows and macOS (because of PATH env var nonsense)
    • I carefully selected launch parameters for disabling lots of unneeded features and behaviors which we don't need or which are bad for our purposes. This is all annotated and linked. There's probably more that could be added.
    • Headless mode is currently not added, but this can be done with a simple class argument. I can't get a valid client-integrity token on Twitch with headless mode, regardless of what others are saying, so I don't see the point right now.
    • Chromium-based browsers I tested (CLI arguments for custom paths are not included in this PR):
      • Chromium (Arch): worked
      • Google Chrome (Windows): worked
      • Brave (Windows): worked
      • Vivaldi (Windows): didn't work, despite a successful CDP session, didn't check further
      • Opera (Windows): didn't work, didn't check further

@bastimeyer bastimeyer changed the title utils.path: add resolve_executable() webbrowser: implement Webbrowser / ChromiumWebbrowser Jun 17, 2023
@bastimeyer bastimeyer mentioned this pull request Jun 17, 2023
Closed
4 tasks
@bastimeyer bastimeyer force-pushed the webbrowser/init branch 3 times, most recently from fa07ad1 to 8312d3b Compare June 18, 2023 13:07
@bastimeyer bastimeyer mentioned this pull request Jun 19, 2023
Merged
bastimeyer
bastimeyer commented Jun 21, 2023
View reviewed changes
@bastimeyer
Copy link
Member Author

Rebased to current head of master, because this PR is the base branch for other PRs until it got merged into master.

Also disabled that one flaky test on Windows because I'm already annoyed by it.

@bastimeyer bastimeyer force-pushed the webbrowser/init branch 2 times, most recently from c739035 to 8d1c786 Compare June 29, 2023 00:36
@bastimeyer bastimeyer mentioned this pull request Jul 2, 2023
Merged
bastimeyer added 3 commits July 3, 2023 17:30
@bastimeyer
build: add trio dependency
d19c71e 
as well as pytest-trio and trio-typing
@bastimeyer
utils.path: add resolve_executable()
d668052 
Implement utility function for resolving executables,
with default names and fallback paths
@bastimeyer
utils.socket: add find_free_port_ipv{4,6}()
b04a22d
@bastimeyer bastimeyer marked this pull request as ready for review July 3, 2023 15:31
bastimeyer added 2 commits July 3, 2023 17:42
@bastimeyer
webbrowser: implement Webbrowser class
4fe311a 
The base class for launching a web browser and interacting with it.

Uses trio for async I/O.
@bastimeyer
webbrowser: implement ChromiumWebbrowser class
13bfa16 
Extends the base Webbrowser class with known Chromium/Chrome
executable names and fallback paths on Windows and macOS, as well as
an extensive list of launch arguments for disabling unneeded features
and behaviors when running and controlling it via
the Chrome Devtools Protocol (CDP).
@bastimeyer
Copy link
Member Author

Rebased to master, fixed order of commits (dependencies first), added missing trio dependency note to docs and added API stability comment to the __init__ module of the streamlink.webbrowser package.

@bastimeyer bastimeyer merged commit adda016 into streamlink:master Jul 3, 2023
@bastimeyer bastimeyer deleted the webbrowser/init branch July 3, 2023 15:50
@bastimeyer
Copy link
Member Author

Thinking about it, it would probably make sense adding MSEdge to the executables list and fallback path list on Windows, even with a higher priority, since it's installed by default and can't be removed on this OS. The old non-Chromium Edge is dead, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
API: webbrowser
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@bastimeyer