Do not show menus during shutdown.

While menu is being shown, it is preventing Chrome from exiting.
However, if a menu is opening while Chrome is already shutting
down, this currently causes a CHECK failure (see linked bug).

The CL is adding shutdown detection logic to ChromeViewsDelegate
and avoids showing a menu if a shutdown is detected. Note that
no callbacks (such as OnMenuClosed) are called.

Bug: 960925
Test: none
Change-Id: Ie282f353b94a5c79a29ba5f770808d552d8e2980
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1601419
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659304}

chrome/browser/browser_process_impl.cc

@@ -379,8 +379,6 @@ void BrowserProcessImpl::StartTearDown() {
   tearing_down_ = true;
   DCHECK(IsShuttingDown());
 
-  KeepAliveRegistry::GetInstance()->SetIsShuttingDown();
-
   // We need to destroy the MetricsServicesManager, IntranetRedirectDetector,
   // NetworkTimeTracker, and SafeBrowsing ClientSideDetectionService
   // (owned by the SafeBrowsingService) before the io_thread_ gets destroyed,
@@ -1468,6 +1466,11 @@ void BrowserProcessImpl::Unpin() {
 
   DCHECK(!shutting_down_);
   shutting_down_ = true;
+
+#if !defined(OS_ANDROID)
+  KeepAliveRegistry::GetInstance()->SetIsShuttingDown();
+#endif  // !defined(OS_ANDROID)
+
 #if BUILDFLAG(ENABLE_PRINTING)
   // Wait for the pending print jobs to finish. Don't do this later, since
   // this might cause a nested run loop to run, and we don't want pending

chrome/browser/ui/views/chrome_views_delegate.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_window_state.h"
+#include "components/keep_alive_registry/keep_alive_registry.h"
 #include "components/keep_alive_registry/keep_alive_types.h"
 #include "components/keep_alive_registry/scoped_keep_alive.h"
 #include "components/prefs/pref_service.h"
@@ -132,6 +133,10 @@ bool ChromeViewsDelegate::GetSavedWindowPlacement(
   return true;
 }
 
+bool ChromeViewsDelegate::IsShuttingDown() const {
+  return KeepAliveRegistry::GetInstance()->IsShuttingDown();
+}
+
 void ChromeViewsDelegate::AddRef() {
   if (ref_count_ == 0u) {
     keep_alive_.reset(

chrome/browser/ui/views/chrome_views_delegate.h

@@ -51,6 +51,7 @@ class ChromeViewsDelegate : public views::ViewsDelegate {
 
   void AddRef() override;
   void ReleaseRef() override;
+  bool IsShuttingDown() const override;
   void OnBeforeWidgetInit(
       views::Widget::InitParams* params,
       views::internal::NativeWidgetDelegate* delegate) override;

components/keep_alive_registry/keep_alive_registry.cc

@@ -80,6 +80,10 @@ bool KeepAliveRegistry::WouldRestartWithout(
   return registered_count == restart_allowed_count;
 }
 
+bool KeepAliveRegistry::IsShuttingDown() const {
+  return is_shutting_down_;
+}
+
 void KeepAliveRegistry::SetIsShuttingDown(bool value) {
   is_shutting_down_ = value;
 }

components/keep_alive_registry/keep_alive_registry.h

@@ -42,7 +42,10 @@ class KeepAliveRegistry {
   // provided |origins| were not registered.
   bool WouldRestartWithout(const std::vector<KeepAliveOrigin>& origins) const;
 
-  // Call when shutting down to ensure registering a new KeepAlive DCHECKs.
+  // True if shutdown is in progress. No new KeepAlive should be registered
+  // while shutting down.
+  bool IsShuttingDown() const;
+  // Call when shutting down to ensure registering a new KeepAlive CHECKs.
   void SetIsShuttingDown(bool value = true);
 
  private:

ui/views/controls/menu/menu_runner.cc

@@ -8,6 +8,7 @@
 
 #include "ui/views/controls/menu/menu_runner_handler.h"
 #include "ui/views/controls/menu/menu_runner_impl.h"
+#include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
 
 namespace views {
@@ -33,6 +34,13 @@ void MenuRunner::RunMenuAt(Widget* parent,
                            const gfx::Rect& bounds,
                            MenuAnchorPosition anchor,
                            ui::MenuSourceType source_type) {
+  // Do not attempt to show the menu if the application is currently shutting
+  // down. MenuDelegate::OnMenuClosed would not be called.
+  if (ViewsDelegate::GetInstance() &&
+      ViewsDelegate::GetInstance()->IsShuttingDown()) {
+    return;
+  }
+
   // If we are shown on mouse press, we will eat the subsequent mouse down and
   // the parent widget will not be able to reset its state (it might have mouse
   // capture from the mouse down). So we clear its state here.

ui/views/views_delegate.cc

@@ -96,6 +96,10 @@ NonClientFrameView* ViewsDelegate::CreateDefaultNonClientFrameView(
   return nullptr;
 }
 
+bool ViewsDelegate::IsShuttingDown() const {
+  return false;
+}
+
 void ViewsDelegate::AddRef() {}
 
 void ViewsDelegate::ReleaseRef() {}

ui/views/views_delegate.h

@@ -141,6 +141,9 @@ class VIEWS_EXPORT ViewsDelegate {
   // ensure we don't attempt to exit while a menu is showing.
   virtual void AddRef();
   virtual void ReleaseRef();
+  // Returns true if the application is shutting down. AddRef/Release should not
+  // be called in this situation.
+  virtual bool IsShuttingDown() const;
 
   // Gives the platform a chance to modify the properties of a Widget.
   virtual void OnBeforeWidgetInit(Widget::InitParams* params,