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

sometimes there is a delay when closing Brave-Beta #12488

Closed
LaurenWags opened this issue Jan 3, 2018 · 3 comments · Fixed by #12582
Closed

sometimes there is a delay when closing Brave-Beta #12488

LaurenWags opened this issue Jan 3, 2018 · 3 comments · Fixed by #12582
Assignees
@petemill
Labels
0.20.x issue first seen in 0.20.x bug perf QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified regression release/blocking release-notes/exclude
Milestone
0.20.x (Release C...

Comments

@LaurenWags
Copy link
Member

LaurenWags commented Jan 3, 2018 •
edited
Loading

Description

Sometimes when quitting Brave-Beta there is a delay in closing (5-10s). This is reproducible on a clean profile without a large amount of data.

Steps to Reproduce

  1. Clean profile on 0.20.14
  2. Open Bookmark Manager
  3. Add a folder under Bookmarks Toolbar in the tree (Folder A)
  4. Add a sub-folder (Folder AA)
  5. Add a sub-subfolder (Folder AAA)
  6. Quit Brave.

Actual result:
5-10s delay in closing Brave (closer to 10s):

brave-beta-closing-delay

Expected result:
No delay in closing.

Reproduces how often:
Easily with these steps. Other steps probably produce this result as well.

Brave Version

about:brave info:
Brave | 0.20.14
V8 | 6.3.292.48
rev | 5d75826
Muon | 4.5.33

Reproducible on current live release:
no, STR do not reproduce on 0.29.123

Additional Information

Found on MacOS, Reproduced on Linux and Win 10 x64 by @kjozwiak
@LaurenWags LaurenWags added 0.20.x issue first seen in 0.20.x bug perf regression labels Jan 3, 2018
@LaurenWags LaurenWags added this to the 0.20.x (Beta Channel) milestone Jan 3, 2018
@bsclifton bsclifton self-assigned this Jan 3, 2018
@bsclifton
Copy link
Member

bsclifton commented Jan 3, 2018 •
edited
Loading

wow- great find (confirmed problem locally)! 😄 Will start trying to find the root cause...

@bsclifton bsclifton removed their assignment Jan 5, 2018
@petemill
Copy link
Member

petemill commented Jan 9, 2018

Timeout for waiting for windows to respond with their state on shutdown is 10 seconds, so probably hitting that

@petemill
Copy link
Member

petemill commented Jan 9, 2018

Looks like an extra hidden window is being created when those settings are changed

petemill added a commit that referenced this issue Jan 10, 2018
@petemill
Only ask Renderer Windows for saved state, on shutdown
c123074 
Create a new windows API method to retrieve renderer windows.
Avoid circular dependency in sessionStoreShutdown / windows.js by exposing a 'new-window-state' event that the session store can subscribe to in order to set state.

Fix #12488 since state was being asked from all windows, even non-renderer windows, which causes a timeout waiting for a state response.
@petemill petemill mentioned this issue Jan 10, 2018
Merged
10 tasks
bsclifton pushed a commit that referenced this issue Jan 12, 2018
@petemill @bsclifton
Only ask Renderer Windows for saved state, on shutdown
58acfe4 
Create a new windows API method to retrieve renderer windows.
Avoid circular dependency in sessionStoreShutdown / windows.js by exposing a 'new-window-state' event that the session store can subscribe to in order to set state.

Fix #12488 since state was being asked from all windows, even non-renderer windows, which causes a timeout waiting for a state response.
This was referenced Jan 22, 2018
Closed
Closed
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 perf QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified regression release/blocking release-notes/exclude
Projects
None yet
Milestone
0.20.x (Release Channel)
Development

Successfully merging a pull request may close this issue.

Only ask Renderer Windows for saved state, on shutdown brave/browser-laptop
7 participants
@petemill @alexwykoff @kjozwiak @luixxiul @bsclifton @srirambv @LaurenWags