capture_mode: Allow users to take snapshot if menu is left open.

Previously, the menu had capture so it would take some events from the
button. To use the bar, users would have to first click the bar to take
capture, and then click again to use a bar button. Doing so would also
close the menu.

This CL changes that by taking away capture from the menu on entering
capture mode. Since doing so will normally close the menu, this also
introduces a way for ash menus to ignore the mouse capture losses.

Bug: 1155223
Test: manual
Change-Id: I19d4ed01d874b4f21975f194ae96cd15af830f78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581027
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842187}

ash/capture_mode/capture_mode_controller.cc

@@ -345,12 +345,17 @@ void CaptureModeController::Start(CaptureModeEntryType entry_type) {
           kResetCaptureRegionDuration) {
     SetUserCaptureRegion(gfx::Rect(), /*by_user=*/false);
   }
+
+  delegate_->OnSessionStateChanged(/*started=*/true);
+
   capture_mode_session_ = std::make_unique<CaptureModeSession>(this);
 }
 
 void CaptureModeController::Stop() {
   DCHECK(IsActive());
   capture_mode_session_.reset();
+
+  delegate_->OnSessionStateChanged(/*started=*/false);
 }
 
 void CaptureModeController::SetUserCaptureRegion(const gfx::Rect& region,

ash/capture_mode/capture_mode_session.cc

@@ -23,6 +23,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "cc/paint/paint_flags.h"
+#include "ui/aura/client/capture_client.h"
 #include "ui/aura/window.h"
 #include "ui/base/cursor/cursor_factory.h"
 #include "ui/base/cursor/cursor_util.h"
@@ -445,12 +446,28 @@ CaptureModeSession::CaptureModeSession(CaptureModeController* controller)
 
   UpdateRootWindowDimmers();
 
+  // A context menu may have input capture when entering a session. Remove
+  // capture from it, otherwise subsequent mouse events will cause it to close,
+  // and then we won't be able to take a screenshot of the menu. Store it so we
+  // can return capture to it when exiting the session.
+  auto* capture_client = aura::client::GetCaptureClient(current_root_);
+  input_capture_window_ = capture_client->GetCaptureWindow();
+  if (input_capture_window_) {
+    capture_client->ReleaseCapture(input_capture_window_);
+    input_capture_window_->AddObserver(this);
+  }
+
   TabletModeController::Get()->AddObserver(this);
   current_root_->AddObserver(this);
   display::Screen::GetScreen()->AddObserver(this);
 }
 
 CaptureModeSession::~CaptureModeSession() {
+  if (input_capture_window_) {
+    input_capture_window_->RemoveObserver(this);
+    aura::client::GetCaptureClient(current_root_)
+        ->SetCapture(input_capture_window_);
+  }
   display::Screen::GetScreen()->RemoveObserver(this);
   current_root_->RemoveObserver(this);
   TabletModeController::Get()->RemoveObserver(this);
@@ -655,6 +672,11 @@ void CaptureModeSession::OnTabletModeEnded() {
 }
 
 void CaptureModeSession::OnWindowDestroying(aura::Window* window) {
+  if (window == input_capture_window_) {
+    input_capture_window_->RemoveObserver(this);
+    input_capture_window_ = nullptr;
+    return;
+  }
   DCHECK_EQ(current_root_, window);
   MaybeChangeRoot(Shell::GetPrimaryRootWindow());
 }

ash/capture_mode/capture_mode_session.h

@@ -305,6 +305,10 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
   // The current focused fine tune position. This changes as user tabs while a
   // in capture region mode.
   FineTunePosition focused_fine_tune_position_ = FineTunePosition::kNone;
+
+  // The window which had input capture prior to entering the session. It may be
+  // null if no such window existed.
+  aura::Window* input_capture_window_ = nullptr;
 };
 
 }  // namespace ash

ash/capture_mode/test_capture_mode_delegate.cc

@@ -121,4 +121,6 @@ TestCaptureModeDelegate::LaunchRecordingService() {
 void TestCaptureModeDelegate::BindAudioStreamFactory(
     mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) {}
 
+void TestCaptureModeDelegate::OnSessionStateChanged(bool started) {}
+
 }  // namespace ash

ash/capture_mode/test_capture_mode_delegate.h

@@ -41,6 +41,7 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
       override;
   void BindAudioStreamFactory(
       mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) override;
+  void OnSessionStateChanged(bool started) override;
 
  private:
   std::unique_ptr<FakeRecordingService> fake_service_;

ash/public/cpp/capture_mode_delegate.h

@@ -85,6 +85,9 @@ class ASH_PUBLIC_EXPORT CaptureModeDelegate {
   // Binds the given audio StreamFactory |receiver| to the audio service.
   virtual void BindAudioStreamFactory(
       mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) = 0;
+
+  // Called when a capture mode session starts or stops.
+  virtual void OnSessionStateChanged(bool started) = 0;
 };
 
 }  // namespace ash

ash/public/cpp/test/shell_test_api.h

@@ -116,6 +116,14 @@ class ASH_EXPORT ShellTestApi {
   // It returns nullptr when app-list is not shown.
   PaginationModel* GetAppListPaginationModel();
 
+  // Shows the context menu associated with the primary root window at point (1,
+  // 1).
+  void ShowContextMenu();
+
+  // Returns true if the context menu associated with the primary root window is
+  // shown.
+  bool IsContextMenuShown() const;
+
  private:
   Shell* shell_;  // not owned
 

ash/shell_test_api.cc

@@ -252,4 +252,13 @@ PaginationModel* ShellTestApi::GetAppListPaginationModel() {
   return view->GetAppsPaginationModel();
 }
 
+void ShellTestApi::ShowContextMenu() {
+  Shell::GetPrimaryRootWindowController()->ShowContextMenu(
+      gfx::Point(1, 1), ui::MENU_SOURCE_MOUSE);
+}
+
+bool ShellTestApi::IsContextMenuShown() const {
+  return Shell::GetPrimaryRootWindowController()->IsContextMenuShown();
+}
+
 }  // namespace ash

chrome/browser/ui/ash/capture_mode_browsertest.cc

+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/public/cpp/ash_features.h"
+#include "ash/public/cpp/capture_mode_test_api.h"
+#include "ash/public/cpp/test/shell_test_api.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/browser_test.h"
+
+class CaptureModeBrowserTest : public InProcessBrowserTest {
+ public:
+  CaptureModeBrowserTest() = default;
+  CaptureModeBrowserTest(const CaptureModeBrowserTest&) = delete;
+  CaptureModeBrowserTest& operator=(const CaptureModeBrowserTest&) = delete;
+  ~CaptureModeBrowserTest() override = default;
+
+  // InProcessBrowserTest:
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(ash::features::kCaptureMode);
+    InProcessBrowserTest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(CaptureModeBrowserTest, ContextMenuStaysOpen) {
+  ash::ShellTestApi shell_test_api;
+  shell_test_api.ShowContextMenu();
+  ASSERT_TRUE(shell_test_api.IsContextMenuShown());
+
+  ash::CaptureModeTestApi().StartForWindow(/*for_video=*/false);
+  EXPECT_TRUE(shell_test_api.IsContextMenuShown());
+}

chrome/browser/ui/ash/chrome_capture_mode_delegate.cc

@@ -173,3 +173,7 @@ void ChromeCaptureModeDelegate::BindAudioStreamFactory(
     mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) {
   content::GetAudioService().BindStreamFactory(std::move(receiver));
 }
+
+void ChromeCaptureModeDelegate::OnSessionStateChanged(bool started) {
+  is_session_active_ = started;
+}

chrome/browser/ui/ash/chrome_capture_mode_delegate.h

@@ -20,6 +20,8 @@ class ChromeCaptureModeDelegate : public ash::CaptureModeDelegate {
 
   static ChromeCaptureModeDelegate* Get();
 
+  bool is_session_active() const { return is_session_active_; }
+
   // Sets |is_screen_capture_locked_| to the given |locked|, and interrupts any
   // on going video capture.
   void SetIsScreenCaptureLocked(bool locked);
@@ -47,6 +49,7 @@ class ChromeCaptureModeDelegate : public ash::CaptureModeDelegate {
       override;
   void BindAudioStreamFactory(
       mojo::PendingReceiver<audio::mojom::StreamFactory> receiver) override;
+  void OnSessionStateChanged(bool started) override;
 
  private:
   // Used to temporarily disable capture mode in certain cases for which neither
@@ -60,6 +63,9 @@ class ChromeCaptureModeDelegate : public ash::CaptureModeDelegate {
   // locked.
   // This is only non-null during recording.
   base::OnceClosure interrupt_video_recording_callback_;
+
+  // True when a capture mode session is currently active.
+  bool is_session_active_ = false;
 };
 
 #endif  // CHROME_BROWSER_UI_ASH_CHROME_CAPTURE_MODE_DELEGATE_H_

chrome/browser/ui/views/DEPS

@@ -7,8 +7,8 @@ include_rules = [
 
 specific_include_rules = {
   "chrome_views_delegate_chromeos\.cc": [
+    "+ash/public/cpp/ash_features.h",
     "+ash/shell.h",
-    "+ash/wm/window_state.h",
   ],
   "qrcode_generator\.*": [
     "+chrome/services/qrcode_generator/public/cpp/qrcode_generator_service.h"

chrome/browser/ui/views/chrome_views_delegate.h

@@ -36,6 +36,7 @@ class ChromeViewsDelegate : public views::ViewsDelegate {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   ProcessMenuAcceleratorResult ProcessAcceleratorWhileMenuShowing(
       const ui::Accelerator& accelerator) override;
+  bool ShouldCloseMenuIfMouseCaptureLost() const override;
   std::unique_ptr<views::NonClientFrameView> CreateDefaultNonClientFrameView(
       views::Widget* widget) override;
 #endif

chrome/browser/ui/views/chrome_views_delegate_chromeos.cc

@@ -5,10 +5,12 @@
 #include "chrome/browser/ui/views/chrome_views_delegate.h"
 
 #include "ash/public/cpp/accelerators.h"
+#include "ash/public/cpp/ash_features.h"
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/task/current_thread.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/ui/ash/chrome_capture_mode_delegate.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 
@@ -37,6 +39,12 @@ ChromeViewsDelegate::ProcessAcceleratorWhileMenuShowing(
   return views::ViewsDelegate::ProcessMenuAcceleratorResult::LEAVE_MENU_OPEN;
 }
 
+bool ChromeViewsDelegate::ShouldCloseMenuIfMouseCaptureLost() const {
+  // Menu closes unless an ongoing screen capture session is underway.
+  return !(ash::features::IsCaptureModeEnabled() &&
+           ChromeCaptureModeDelegate::Get()->is_session_active());
+}
+
 std::unique_ptr<views::NonClientFrameView>
 ChromeViewsDelegate::CreateDefaultNonClientFrameView(views::Widget* widget) {
   return ash::Shell::Get()->CreateDefaultNonClientFrameView(widget);

chrome/test/BUILD.gn

@@ -2782,6 +2782,7 @@ if (!is_android) {
         "../browser/ui/ash/accelerator_commands_browsertest.cc",
         "../browser/ui/ash/assistant/assistant_context_browsertest.cc",
         "../browser/ui/ash/back_gesture_browsertest.cc",
+        "../browser/ui/ash/capture_mode_browsertest.cc",
         "../browser/ui/ash/chrome_new_window_client_browsertest.cc",
         "../browser/ui/ash/chrome_screenshot_grabber_browsertest.cc",
         "../browser/ui/ash/clipboard_history_browsertest.cc",

ui/views/controls/menu/menu_host.cc

@@ -20,6 +20,7 @@
 #include "ui/views/controls/menu/menu_scroll_view_container.h"
 #include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/round_rect_painter.h"
+#include "ui/views/views_delegate.h"
 #include "ui/views/widget/native_widget_private.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
@@ -235,6 +236,10 @@ internal::RootView* MenuHost::CreateRootView() {
 void MenuHost::OnMouseCaptureLost() {
   if (destroying_ || ignore_capture_lost_)
     return;
+
+  if (!ViewsDelegate::GetInstance()->ShouldCloseMenuIfMouseCaptureLost())
+    return;
+
   MenuController* menu_controller =
       submenu_->GetMenuItem()->GetMenuController();
   if (menu_controller && !menu_controller->drag_in_progress())

ui/views/views_delegate.cc

@@ -73,6 +73,10 @@ ViewsDelegate::ProcessAcceleratorWhileMenuShowing(
   return ProcessMenuAcceleratorResult::LEAVE_MENU_OPEN;
 }
 
+bool ViewsDelegate::ShouldCloseMenuIfMouseCaptureLost() const {
+  return true;
+}
+
 #if defined(OS_WIN)
 HICON ViewsDelegate::GetDefaultWindowIcon() const {
   return nullptr;

ui/views/views_delegate.h

@@ -125,6 +125,10 @@ class VIEWS_EXPORT ViewsDelegate {
   virtual ProcessMenuAcceleratorResult ProcessAcceleratorWhileMenuShowing(
       const ui::Accelerator& accelerator);
 
+  // If a menu is showing and its window loses mouse capture, it will close if
+  // this returns true.
+  virtual bool ShouldCloseMenuIfMouseCaptureLost() const;
+
 #if defined(OS_WIN)
   // Retrieves the default window icon to use for windows if none is specified.
   virtual HICON GetDefaultWindowIcon() const;