Removed NOTIFICATION_BROWSER_CLOSING notification

chrome/browser/ui/browser.cc

Index: chrome/browser/ui/browser.cc
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 832153b86e17201770479a2f4bfa17284c823b52..8682c00804f3af0a46307afe2410fd212ecc8eb6 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -746,6 +746,8 @@ void Browser::OnWindowClosing() {
   if (tab_restore_service && is_type_tabbed() && tab_strip_model_->count())
     tab_restore_service->BrowserClosing(live_tab_context());
 
+  BrowserList::NotifyBrowserCloseStarted(this);
+
   // TODO(sky): convert session/tab restore to use notification.
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_BROWSER_CLOSING,

[hashimoto, 2017/04/14 08:54:37]

How about moving this notification code to
BrowserList::NotifyBrowserCloseStarted?
The same goes for NOTIFICATION_BROWSER_CLOSE_CANCELLED in
fast_unload_controller.cc and unload_controller.cc.

[Alexey Seren, 2017/04/22 19:52:54]

Acknowledged.

@@ -2556,7 +2558,11 @@ bool Browser::ShouldHideUIForFullscreen() const {
 }
 
 bool Browser::ShouldStartShutdown() const {
-  return BrowserList::GetInstance()->size() <= 1;
+  DCHECK_GE(BrowserList::GetInstance()->size(),
+            BrowserList::GetInstance()->currently_closing_browsers_count());
+  // Start shutdown process only when closing the last browser.
+  return BrowserList::GetInstance()->size() ==
+         BrowserList::GetInstance()->currently_closing_browsers_count() + 1;

[hashimoto, 2017/04/14 08:54:37]

Sorry, I still don't understand why this change is needed.
This function is used only in Browser::OnWindowClosing(), and IIUC it eventually
returns true when closing the last Browser.

[Alexey Seren, 2017/04/22 19:52:54]

This fixes the races happened when two browsers are closing simultaneously. How
it can happen:
Suppose that we have already called Browser::OnWindowClosing() for the first
browser and lets suppose that Browser::OnWindowClosing() for the second browser
was called before the first browser was closed in BrowserList::RemoveBrowser().
In this case when calling OnWindowClosing() for the second browser we will have
BrowserList::GetInstance()->size() == 2.
So Browser::ShouldStartShutdown() will return false.

To fix this we need to keep track of currently closing browsers list.

[hashimoto, 2017/04/28 09:16:29]

Thank you for the explanation.
I don't think it's the right thing to do to return true from
ShouldStartShutdown() when the window close can be still cancelled.

ShouldStartShutdown's only user is Browser::OnWindowClosing().
Can't we stop calling browser_shutdown::OnShutdownStarting() in
Browser::OnWindowClosing(), and instead call
browser_shutdown::OnShutdownStarting() somewhere else when window close is
actually finished (or the last keep alive is released from the
KeepAliveRegistry)?

[Alexey Seren, 2017/05/03 17:19:44]

Actually we cannot cancel shutdown after we have entered
Browser::OnWindowClosing() once. This functionality was done by following CL:
https://codereview.chromium.org/2695233003
Ok, I will try to find right place for this.

 }
 
 bool Browser::MaybeCreateBackgroundContents(