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

Brave extension keeps closed tabs' shields data in memory #23559

Closed
petemill opened this issue Jun 20, 2022 · 5 comments · Fixed by brave/brave-core#13872
Closed

Brave extension keeps closed tabs' shields data in memory #23559

petemill opened this issue Jun 20, 2022 · 5 comments · Fixed by brave/brave-core#13872

Comments

@petemill
Copy link
Member

petemill commented Jun 20, 2022

This will allow memory to grow for this specific process (brave extension), unchecked.

STR:

  • Open and close many tabs
  • Observe brave extension memory growing for each opened tab (even if it is now closed). However, it's an extremely minimal amount. If you wanted to check you'd have to open and close hundreds of tabs.

The code this touches isn't even used. It's old code to support the previous version of the shields UI (which used the shields extension), which is useable via the flag but doesn't affect this bug, which has always been present in brave-core from what I can tell. It will be completely removed in #23285

@petemill petemill added priority/P2 A bad problem. We might uplift this to the next planned release. perf OS/Desktop labels Jun 20, 2022
@petemill petemill self-assigned this Jun 20, 2022
@petemill
Copy link
Member Author

petemill commented Jun 20, 2022

@kjozwiak
Copy link
Member

The above will require 1.41.85 or higher for 1.41.x verification 👍

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

MadhaviSeelam commented Jul 1, 2022

Verification PASSED using

Brave | 1.41.93 Chromium: 103.0.5060.114 (Official Build) (64-bit)
-- | --
Revision | a1c2360c5b02a6d4d6ab33796ad8a268a6128226-refs/branch-heads/5060@{#1124}
OS | Windows 11 Version 21H2 (Build 22000.739)

Modified and verified with original testplan from brave/brave-core#13872 (comment)

Confirmed with Brave 1.36.116 Chromium: 99.0.4844.74 (Official Build) (64-bit)

  • Repeated closing/re-opening and cycling through the tabs (x50 times)
  • Extension: Brave process got memory usage of ~65-75MB

Screenshot 2022-07-05 134519

Steps:

  • install 1.41.93
  • launch Brave
  • open the Task manager via More tools
  • load mail.google.com in 3 tabs
  • load mail.yandex.com in one of the tabs
  • load github.com in one of the tabs
  • open https://www.cnn.com and open another ~12 tabs by opening random links on https://www.cnn.com
  • ensure that you have another Brave window opened without any tabs other than the active brave://newtab
  • close the window with CNN, gmail, yandex, github tabs
  • restore the same window with the CNN, gmail, yandex, github tabs via Settings -> History -> Recently closed
  • cycle through each of the tabs and then close the window with all the tabs once again
  • repeat the above re: closing/re-opening and cycling through the tabs (x45 times)
  • Confirmed Extension: Brave process consistently got memory usage of < 50MB

1.41.93

1 10 20 30 40 45
31532 36812 40444 26972 37184 45500

image

@stephendonner
Copy link

Verified PASSED using

Brave 1.41.91 Chromium: 103.0.5060.114 (Official Build) beta (x86_64)
Revision a1c2360c5b02a6d4d6ab33796ad8a268a6128226-refs/branch-heads/5060@{#1124}
OS macOS Version 11.6.7 (Build 20G630)

Steps

Followed @MadhaviSeelam's adaptation (above) of the original testplan: brave/brave-core#13872 (comment)

Reproduced the original issue using 1.31.115.. The only modification I did to #23559 (comment) was to run 20x, instead of 45x+.

Max Extension: Brave memory usage (in MB):

1.41.91 1.36.115
46.4 61.5
1.41.91 1.36.115
Screen Shot 2022-07-05 at 5 12 25 PM Screen Shot 2022-07-05 at 5 15 46 PM

@stephendonner
Copy link

stephendonner commented Jul 6, 2022

Verified PASSED using

Brave 1.41.93 Chromium: 103.0.5060.114 (Official Build) (64-bit)
Revision a1c2360c5b02a6d4d6ab33796ad8a268a6128226-refs/branch-heads/5060@{#1124}
OS Linux

Steps

Followed @MadhaviSeelam's adaptation (above) of the original testplan: brave/brave-core#13872 (comment)

Reproduced the original issue using 1.36.115.. The only modification I did to #23559 (comment) was to run 20x, instead of 45x+.

Max Extension: Brave memory usage (in kilobytes & MB):

1.41.91 1.36.115
40, 512K (39.6MB) 52, 256K (51.0MB)
1.41.91 1.36.115
Screen Shot 2022-07-05 at 6 43 13 PM Screen Shot 2022-07-05 at 6 33 23 PM

@stephendonner stephendonner added QA Pass-Linux and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants