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

Don't impose WebSockets limits on Extensions #21661

Closed
goodov opened this issue Mar 15, 2022 · 8 comments · Fixed by brave/brave-core#12597
Closed

Don't impose WebSockets limits on Extensions #21661

goodov opened this issue Mar 15, 2022 · 8 comments · Fixed by brave/brave-core#12597

Comments

@goodov
Copy link
Member

goodov commented Mar 15, 2022

Currently we limit WebSockets connections for Extensions as part of #19990 implementation. We don't need to do this.

See #19990 (comment)

@goodov goodov added OS/Android Fixes related to Android browser functionality OS/Desktop labels Mar 15, 2022
@goodov goodov self-assigned this Mar 15, 2022
@goodov goodov changed the title Don't impose WebSockets limits for Extensions Don't impose WebSockets limits on Extensions Mar 15, 2022
@goodov goodov added this to the 1.38.x - Nightly milestone Mar 15, 2022
@goodov goodov added the QA/Yes label Mar 15, 2022
@stephendonner
Copy link

@goodov 👋 mind rounding our a test plan for this? cc @brave/qa-team and setting QA/Blocked until we've got that; thanks!

@goodov
Copy link
Member Author

goodov commented Mar 29, 2022

@goodov 👋 mind rounding our a test plan for this? cc @brave/qa-team and setting QA/Blocked until we've got that; thanks!

This can be tested by running a simple extension which creates WebSocket connections and shows that there are no limits (it can create much more than 10 connections).

Extension for test: ws-extension-test.zip

@stephendonner
Copy link

Verified PASSED using

Brave 1.38.71 Chromium: 100.0.4896.46 (Official Build) beta (x86_64)
Revision 5ca33821b2211805855c77d334353d27c616a7ca-refs/branch-heads/4896@{#584}
OS macOS Version 11.6.5 (Build 20G527)

Steps:

  1. install 1.38.71
  2. launch Brave
  3. install https://github.com/brave/brave-browser/files/8370086/ws-extension-test.zip
  4. click the button
  5. note the # of successfully created WebSockets connections

Confirmed we hit well over 10; in my limited testrun, it took until 65 before we started having connection-establishing/maintaining errors.

25 WebSockets 64+ WebSockets
Screen Shot 2022-03-29 at 1 22 08 PM Screen Shot 2022-03-29 at 1 28 00 PM

@stephendonner
Copy link

@goodov asking on behalf of @brave/qa-team - are there really (easily) testable Android steps, here? 🙏

@goodov
Copy link
Member Author

goodov commented Mar 30, 2022

@goodov asking on behalf of @brave/qa-team - are there really (easily) testable Android steps, here? 🙏

There're no extensions, so this task is not applicable for Android.

@goodov goodov removed the OS/Android Fixes related to Android browser functionality label Mar 30, 2022
@LaurenWags
Copy link
Member

Linking confirmation that this is resolved from #19990 (comment)

@MadhaviSeelam MadhaviSeelam added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Apr 5, 2022
@MadhaviSeelam
Copy link

Verified Passed using

Brave 1.38.83 Chromium: 100.0.4896.79 (Official Build) beta (64-bit)
Revision 8fb749dcab8700c24213791969e59deb72fee36f-refs/branch-heads/4896@{#1015}
OS Windows 11 Version 21H2 (Build 22000.593)

Confirmed we hit well over 10.

25 web sockets 64+
25+sockets 64+ sockets

@MadhaviSeelam MadhaviSeelam added QA Pass-Win64 and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Apr 7, 2022
@btlechowski
Copy link

Verification passed on

Brave 1.38.83 Chromium: 100.0.4896.79 (Official Build) beta (64-bit)
Revision 8fb749dcab8700c24213791969e59deb72fee36f-refs/branch-heads/4896@{#1015}
OS Ubuntu 18.04 LTS

Confirmed I hit well over 10;

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment