New issue
Advanced search Search tips

Issue 707147 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 2018
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug



Sign in to add a comment

Browser shutdown is not properly handled if two browsers are opened

Reported by ase...@yandex-team.ru, Mar 31 2017

Issue description

Chrome Version       : 57.0.2987.133
OS Version: OS X 10.12.3

What steps will reproduce the problem?
1. open browser, open url chrome://help
2. open another browser, open url chrome://version
3. shutdown browsers with Cmd + Q

What is the expected result?
1. "chrome_shutdown_ms.txt" file is created in "~Library/Application Support/Google/Chrome" directory
2. shutdown histograms are created on the next browser start (i.e. "Shutdown.window_close.time2")

What happens instead of that?
"chrome_shutdown_ms.txt" and expected histograms is not created

This is happened due to races of functions:
- BrowserList::RemoveBrowser
- Browser::OnWindowClosing

 

Comment 1 by lgrey@chromium.org, Mar 31 2017

Status: Available (was: Unconfirmed)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d

commit 121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d
Author: Aleksei Seren <aseren@yandex-team.ru>
Date: Tue Nov 28 20:11:18 2017

Fix shutdown histograms on browser closing.

Shutdown histograms were not properly written because of several issues:
1) There is an incorrect calculation of current open browsers which
have not yet started to close. This leads to the fact that
browser_shutdown::OnShutdownStarting() is not called in case of
several browsers shutdown.
2) During the closure of the browser we are trying to find if there is any
background Chrome applications with help of KeepAliveRegistry::IsKeepingAlive()
function call, which is actually tracking running Browsers also (i.e. it can
return true even if there is no background application). So it is needed to
introduce new function to track background applications only.

R=sky@chromium.org
BUG= 707147 
BUG= 707144 

Change-Id: If4f7c080965c95e2c0b810817e94a09f3a52ba51
Reviewed-on: https://chromium-review.googlesource.com/760356
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519810}
[add] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/chrome/browser/browser_shutdown_browsertest.cc
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/chrome/browser/ui/browser.cc
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/chrome/browser/ui/browser.h
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/chrome/browser/ui/browser_list.cc
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/chrome/browser/ui/browser_list.h
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/chrome/test/BUILD.gn
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/components/keep_alive_registry/keep_alive_registry.cc
[modify] https://crrev.com/121533e5f6bc5a80d8fbc7b9cf8ae19f8a21243d/components/keep_alive_registry/keep_alive_registry.h

Status: Fixed (was: Available)

Sign in to add a comment