Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Brave process not terminated after bookmarking a page and closing through titlebar #13277

Closed
btlechowski opened this issue Feb 23, 2018 · 10 comments · Fixed by #13758
Closed

Brave process not terminated after bookmarking a page and closing through titlebar #13277

btlechowski opened this issue Feb 23, 2018 · 10 comments · Fixed by #13758
Assignees
@petemill
Labels
0.20.x issue first seen in 0.20.x bug OS/unix-like/linux OS/Windows priority/P3 Major loss of function. QA/checked-Linux QA/checked-Win64 QA/test-plan-specified release-notes/include
Milestone
0.22.x Release 3 ...

Comments

@btlechowski
Copy link
Contributor

btlechowski commented Feb 23, 2018 •
edited by petemill
Loading

Steps to Reproduce

  1. Clean Install
  2. Open Brave
  3. Bookmark page through the star
  4. Close Brave through titlebar's close button (x)

Actual result:
BraveBeta processes are running

Expected result:
BraveBeta processes are NOT running

Reproduces how often:
100%

Brave Version

Brave: 0.21.12 
V8: 6.4.388.41 
rev: 246f9008be47dbdcc6dc9ded5690a847eee8a4e0 
Muon: 5.0.5 
OS Release: 6.1.7601 
Update Channel: Beta 
OS Architecture: x64 
OS Platform: Microsoft Windows 
Node.js: 7.9.0 
Brave Sync: v1.4.2 
libchromiumcontent: 64.0.3282.140
@btlechowski btlechowski added this to the 0.21.x (Beta Channel) milestone Feb 23, 2018
@kjozwiak
Copy link
Member

kjozwiak commented Feb 23, 2018 •
edited
Loading

I also managed to reproduce this on the following platforms:

  • Win 10 x64 VM - Reproduced
  • Ubuntu 17.10 x64 VM - Reproduced

Looks like macOS isn't affected. When you close the only remaining browser window on macOS using X, the application isn't completely terminated/killed like it is under Windows and Linux. Even without any brave windows, Brave will continue running until it's completely closed via Exit Brave.

Closing Brave under macOS using 'Exit Brave' correctly terminates all Brave processes.

Great catch @btlechowski 👍

@kjozwiak kjozwiak added the 0.20.x issue first seen in 0.20.x label Feb 23, 2018
@kjozwiak
Copy link
Member

Looks like this is also present in 0.20.42 096c7cb. I managed to reproduce it under Win 10 and Ubuntu 17.10.

Not a new regression, but I think we should probably get this fixed in this milestone as #13233 was also fixed. @petemill @bsclifton thoughts?

@petemill
Copy link
Member

petemill commented Feb 23, 2018 •
edited
Loading

I'll give it 1 try

@bsclifton
Copy link
Member

Per @petemill: this is related to browser window being created for measurements (related to calculating the bookmark toolbar width?)

@petemill
Copy link
Member

Removing release/blocking since it's already an issue in 0.20

@alexwykoff alexwykoff changed the title BraveBeta process not terminated after bookmarking a page and closing through titlebar Brave process not terminated after bookmarking a page and closing through titlebar Feb 27, 2018
@alexwykoff alexwykoff added the priority/P3 Major loss of function. label Feb 27, 2018
@alexwykoff alexwykoff modified the milestones: 0.22.x (Developer Channel), Backlog (Prioritized) Feb 27, 2018
@kjozwiak kjozwiak mentioned this issue Mar 12, 2018
Closed
@kjozwiak
Copy link
Member

Just a reminder, when this gets fixed, please take a look at #13422 which is most likely the same issue.

@jonathansampson
Copy link
Collaborator

jonathansampson commented Mar 30, 2018 •
edited
Loading

I believe I just ran into this same issue today:

  1. Started with a fresh brave profile
  2. Started brave and navigated to brave.com
  3. Bookmark brave.com, leaving it in the bookmarks toolbar
  4. Close brave via the top-right [x]

Observe in Task Manager that several Brave processes are still running.

Brave: 0.22.12 
V8: 6.5.254.41 
rev: 516cf8a43a9aba83b95d8a2c8f7b72f8f338a360 
Muon: 5.1.2 
OS Release: 10.0.16299 
Update Channel: Release 
OS Architecture: x64 
OS Platform: Microsoft Windows 
Node.js: 7.9.0 
Brave Sync: v1.4.2 
libchromiumcontent: 65.0.3325.181

petemill added a commit that referenced this issue Apr 7, 2018
@petemill
Calculate bookmarks bar overflow using css
9bd1cb7 
Fix #13695

Remove usage of textCalc as it was the last usage of tabs.executeScriptInBackground which is not compatible with upcoming muon v6.x since as it incorrectly used executeScriptInTab which should only work on a WebContents which is a Tab, not a Window. Muon does not have a generic WebContents.executeJavascript method.

Also added bonus of not requiring creating new Windows in order to calculate text width and having to manage when they are closed, etc which should address the following issues, if that was the cause:
Fix #13422
Fix #13277
@petemill petemill mentioned this issue Apr 7, 2018
Merged
10 tasks
petemill added a commit that referenced this issue Apr 7, 2018
@petemill
Calculate bookmarks bar overflow using css
8b1767b 
Fix #13695

Remove usage of textCalc as it was the last usage of tabs.executeScriptInBackground which is not compatible with upcoming muon v6.x since as it incorrectly used executeScriptInTab which should only work on a WebContents which is a Tab, not a Window. Muon does not have a generic WebContents.executeJavascript method.

Also added bonus of not requiring creating new Windows in order to calculate text width and having to manage when they are closed, etc which should address the following issues, if that was the cause:
Fix #13422
Fix #13277
petemill added a commit that referenced this issue Apr 9, 2018
@petemill
Calculate bookmarks bar overflow using css
52982e6 
Fix #13695

Remove usage of textCalc as it was the last usage of tabs.executeScriptInBackground which is not compatible with upcoming muon v6.x since as it incorrectly used executeScriptInTab which should only work on a WebContents which is a Tab, not a Window. Muon does not have a generic WebContents.executeJavascript method.

Also added bonus of not requiring creating new Windows in order to calculate text width and having to manage when they are closed, etc which should address the following issues, if that was the cause:
Fix #13422
Fix #13277
petemill added a commit that referenced this issue Apr 9, 2018
@petemill
Calculate bookmarks bar overflow using css
122aa70 
Fix #13695

Remove usage of textCalc as it was the last usage of tabs.executeScriptInBackground which is not compatible with upcoming muon v6.x since as it incorrectly used executeScriptInTab which should only work on a WebContents which is a Tab, not a Window. Muon does not have a generic WebContents.executeJavascript method.

Also added bonus of not requiring creating new Windows in order to calculate text width and having to manage when they are closed, etc which should address the following issues, if that was the cause:
Fix #13422
Fix #13277
@kjozwiak
Copy link
Member

@petemill should this particular issue be moved into the 0.22.x Release 3 as well? Looks like #13758 fix is in that milestone as well.

@bsclifton bsclifton modified the milestones: Backlog (Prioritized), 0.22.x Release 3 (Beta channel) Apr 11, 2018
@btlechowski
Copy link
Contributor Author

Verified Win7 x64 v0.22.108

@kjozwiak
Copy link
Member

Verified on Mint 18.3 x64 using the following build:

  • 0.22.711 31d6bccb220485dc44f0ce0c65ed797f0160c8a7
  • Muon: 6.0.9
  • libchromiumcontent: 66.0.3359.139

@kjozwiak kjozwiak mentioned this issue May 16, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@petemill petemill

Labels
0.20.x issue first seen in 0.20.x bug OS/unix-like/linux OS/Windows priority/P3 Major loss of function. QA/checked-Linux QA/checked-Win64 QA/test-plan-specified release-notes/include
Projects
None yet
Milestone
0.22.x Release 3 (Release Channel)
Development

Successfully merging a pull request may close this issue.

Calculate bookmarks bar overflow using css brave/browser-laptop
7 participants
@petemill @jonathansampson @alexwykoff @kjozwiak @bsclifton @LaurenWags @btlechowski