cros: Fix ash ScreenshotDelegate ownership and creation

Many ash_unittests fail under --mus and --mash because the
TestScreenshotDelegate is not created. This happens because of some
legacy AcceleratorControllerDelegate code that dates back ~18 months
to when we couldn't use aura::Window* in mash code. That isn't a
problem anymore.

* Move ownership of ScreenshotDelegate to ScreenshotController
* Create the delegate in ShellDelegate, similar to how we do other
  ash delegates
* Move screenshot accelerator code back into AcceleratorController
* Re-enable tests under mus and mash via filter files

Screenshots don't work yet under --mus and --mash due to lack of
graphics readback support and lack of a mojo interface, but we now
exercise more of the UI and accelerator handling code.

Bug: 557397, 632111
Test: ash_unittests (--mus, --mash), manually take screenshots
Change-Id: I7dd1946ddc30bddb407c9dbf6095caf8efb4fd1f
Reviewed-on: https://chromium-review.googlesource.com/744295Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513018}

ash/accelerators/accelerator_controller.cc

@@ -41,6 +41,7 @@
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_notifier.h"
 #include "ash/system/web_notification/web_notification_tray.h"
+#include "ash/utility/screenshot_controller.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/overview/window_selector_controller.h"
 #include "ash/wm/screen_pinning_controller.h"
@@ -381,6 +382,22 @@ void HandleShowKeyboardOverlay() {
   Shell::Get()->new_window_controller()->ShowKeyboardOverlay();
 }
 
+void HandleTakeWindowScreenshot() {
+  base::RecordAction(UserMetricsAction("Accel_Take_Window_Screenshot"));
+  Shell::Get()->screenshot_controller()->StartWindowScreenshotSession();
+}
+
+void HandleTakePartialScreenshot() {
+  base::RecordAction(UserMetricsAction("Accel_Take_Partial_Screenshot"));
+  Shell::Get()->screenshot_controller()->StartPartialScreenshotSession(
+      true /* draw_overlay_immediately */);
+}
+
+void HandleTakeScreenshot() {
+  base::RecordAction(UserMetricsAction("Accel_Take_Screenshot"));
+  Shell::Get()->screenshot_controller()->TakeScreenshotForAllRootWindows();
+}
+
 bool CanHandleToggleMessageCenterBubble() {
   aura::Window* target_root = Shell::GetRootWindowForNewWindows();
   StatusAreaWidget* status_area_widget =
@@ -1127,6 +1144,9 @@ bool AcceleratorController::CanPerformAction(
     case SHOW_KEYBOARD_OVERLAY:
     case SHOW_TASK_MANAGER:
     case SUSPEND:
+    case TAKE_PARTIAL_SCREENSHOT:
+    case TAKE_SCREENSHOT:
+    case TAKE_WINDOW_SCREENSHOT:
     case TOGGLE_FULLSCREEN:
     case TOGGLE_HIGH_CONTRAST:
     case TOGGLE_MAXIMIZED:
@@ -1345,6 +1365,15 @@ void AcceleratorController::PerformAction(AcceleratorAction action,
     case SWITCH_TO_PREVIOUS_USER:
       HandleCycleUser(CycleUserDirection::PREVIOUS);
       break;
+    case TAKE_PARTIAL_SCREENSHOT:
+      HandleTakePartialScreenshot();
+      break;
+    case TAKE_SCREENSHOT:
+      HandleTakeScreenshot();
+      break;
+    case TAKE_WINDOW_SCREENSHOT:
+      HandleTakeWindowScreenshot();
+      break;
     case TOGGLE_APP_LIST:
       HandleToggleAppList(accelerator);
       break;

ash/accelerators/accelerator_controller_delegate_classic.cc

@@ -18,17 +18,13 @@
 #include "ash/magnifier/magnification_controller.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/root_window_controller.h"
-#include "ash/screenshot_delegate.h"
 #include "ash/shell.h"
 #include "ash/system/power/power_button_controller.h"
 #include "ash/system/system_notifier.h"
 #include "ash/touch/touch_hud_debug.h"
-#include "ash/utility/screenshot_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_controller.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/wm_event.h"
-#include "base/metrics/histogram_macros.h"
-#include "base/metrics/user_metrics.h"
 #include "base/sys_info.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/accelerators/accelerator.h"
@@ -39,8 +35,6 @@
 namespace ash {
 namespace {
 
-using base::UserMetricsAction;
-
 bool CanHandleMagnifyScreen() {
   return Shell::Get()->magnification_controller()->IsEnabled();
 }
@@ -61,27 +55,6 @@ void HandleMagnifyScreen(int delta_index) {
   }
 }
 
-void HandleTakeWindowScreenshot(ScreenshotDelegate* screenshot_delegate) {
-  base::RecordAction(UserMetricsAction("Accel_Take_Window_Screenshot"));
-  DCHECK(screenshot_delegate);
-  Shell::Get()->screenshot_controller()->StartWindowScreenshotSession(
-      screenshot_delegate);
-}
-
-void HandleTakePartialScreenshot(ScreenshotDelegate* screenshot_delegate) {
-  base::RecordAction(UserMetricsAction("Accel_Take_Partial_Screenshot"));
-  DCHECK(screenshot_delegate);
-  Shell::Get()->screenshot_controller()->StartPartialScreenshotSession(
-      screenshot_delegate, true /* draw_overlay_immediately */);
-}
-
-void HandleTakeScreenshot(ScreenshotDelegate* screenshot_delegate) {
-  base::RecordAction(UserMetricsAction("Accel_Take_Screenshot"));
-  DCHECK(screenshot_delegate);
-  if (screenshot_delegate->CanTakeScreenshot())
-    screenshot_delegate->HandleTakeScreenshotForAllRootWindows();
-}
-
 bool CanHandleUnpin() {
   // Returns true only for WindowStateType::PINNED.
   // WindowStateType::TRUSTED_PINNED does not accept user's unpin operation.
@@ -110,11 +83,6 @@ AcceleratorControllerDelegateClassic::AcceleratorControllerDelegateClassic() {}
 
 AcceleratorControllerDelegateClassic::~AcceleratorControllerDelegateClassic() {}
 
-void AcceleratorControllerDelegateClassic::SetScreenshotDelegate(
-    std::unique_ptr<ScreenshotDelegate> screenshot_delegate) {
-  screenshot_delegate_ = std::move(screenshot_delegate);
-}
-
 bool AcceleratorControllerDelegateClassic::HandlesAction(
     AcceleratorAction action) {
   // NOTE: When adding a new accelerator that only depends on //ash/common code,
@@ -129,9 +97,6 @@ bool AcceleratorControllerDelegateClassic::HandlesAction(
     case MAGNIFY_SCREEN_ZOOM_OUT:
     case POWER_PRESSED:
     case POWER_RELEASED:
-    case TAKE_PARTIAL_SCREENSHOT:
-    case TAKE_SCREENSHOT:
-    case TAKE_WINDOW_SCREENSHOT:
     case TOGGLE_MESSAGE_CENTER_BUBBLE:
     case TOUCH_HUD_CLEAR:
     case TOUCH_HUD_MODE_CHANGE:
@@ -165,9 +130,6 @@ bool AcceleratorControllerDelegateClassic::CanPerformAction(
     case LOCK_RELEASED:
     case POWER_PRESSED:
     case POWER_RELEASED:
-    case TAKE_PARTIAL_SCREENSHOT:
-    case TAKE_SCREENSHOT:
-    case TAKE_WINDOW_SCREENSHOT:
     case TOUCH_HUD_PROJECTION_TOGGLE:
       return true;
 
@@ -219,15 +181,6 @@ void AcceleratorControllerDelegateClassic::PerformAction(
       // D-BUS), but we consume them to prevent them from getting
       // passed to apps -- see http://crbug.com/146609.
       break;
-    case TAKE_PARTIAL_SCREENSHOT:
-      HandleTakePartialScreenshot(screenshot_delegate_.get());
-      break;
-    case TAKE_SCREENSHOT:
-      HandleTakeScreenshot(screenshot_delegate_.get());
-      break;
-    case TAKE_WINDOW_SCREENSHOT:
-      HandleTakeWindowScreenshot(screenshot_delegate_.get());
-      break;
     case TOUCH_HUD_CLEAR:
       HandleTouchHudClear();
       break;

ash/accelerators/accelerator_controller_delegate_classic.h

@@ -12,8 +12,6 @@
 
 namespace ash {
 
-class ScreenshotDelegate;
-
 // Support for accelerators that only work in classic ash and not in mash,
 // for example accelerators related to display management. These sorts of
 // accelerators should be rare. Most new accelerators should be added to
@@ -24,12 +22,6 @@ class ASH_EXPORT AcceleratorControllerDelegateClassic
   AcceleratorControllerDelegateClassic();
   ~AcceleratorControllerDelegateClassic() override;
 
-  void SetScreenshotDelegate(
-      std::unique_ptr<ScreenshotDelegate> screenshot_delegate);
-  ScreenshotDelegate* screenshot_delegate() {
-    return screenshot_delegate_.get();
-  }
-
   // AcceleratorControllerDelegate:
   bool HandlesAction(AcceleratorAction action) override;
   bool CanPerformAction(AcceleratorAction action,
@@ -39,8 +31,6 @@ class ASH_EXPORT AcceleratorControllerDelegateClassic
                      const ui::Accelerator& accelerator) override;
 
  private:
-  std::unique_ptr<ScreenshotDelegate> screenshot_delegate_;
-
   DISALLOW_COPY_AND_ASSIGN(AcceleratorControllerDelegateClassic);
 };
 

ash/accelerators/accelerator_controller_unittest.cc

@@ -540,10 +540,6 @@ TEST_F(AcceleratorControllerTest, TestRepeatedSnap) {
 }
 
 TEST_F(AcceleratorControllerTest, RotateScreen) {
-  // TODO: needs GetDisplayInfo http://crbug.com/622480.
-  if (Shell::GetAshConfig() == Config::MASH)
-    return;
-
   display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
   display::Display::Rotation initial_rotation =
       GetActiveDisplayRotation(display.id());
@@ -675,10 +671,6 @@ TEST_F(AcceleratorControllerTest, ProcessOnce) {
 }
 
 TEST_F(AcceleratorControllerTest, GlobalAccelerators) {
-  // TODO: TestScreenshotDelegate is null in mash http://crbug.com/632111.
-  if (Shell::GetAshConfig() == Config::MASH)
-    return;
-
   // CycleBackward
   EXPECT_TRUE(ProcessInController(
       ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN)));
@@ -1030,10 +1022,6 @@ class PreferredReservedAcceleratorsTest : public AshTestBase {
 }  // namespace
 
 TEST_F(PreferredReservedAcceleratorsTest, AcceleratorsWithFullscreen) {
-  // TODO: needs LockStateController ported: http://crbug.com/632189.
-  if (Shell::GetAshConfig() == Config::MASH)
-    return;
-
   aura::Window* w1 = CreateTestWindowInShellWithId(0);
   aura::Window* w2 = CreateTestWindowInShellWithId(1);
   wm::ActivateWindow(w1);
@@ -1080,9 +1068,6 @@ TEST_F(PreferredReservedAcceleratorsTest, AcceleratorsWithFullscreen) {
 }
 
 TEST_F(PreferredReservedAcceleratorsTest, AcceleratorsWithPinned) {
-  // TODO: needs LockStateController ported: http://crbug.com/632189.
-  if (Shell::GetAshConfig() == Config::MASH)
-    return;
   aura::Window* w1 = CreateTestWindowInShellWithId(0);
   aura::Window* w2 = CreateTestWindowInShellWithId(1);
   wm::ActivateWindow(w1);
@@ -1111,10 +1096,6 @@ TEST_F(PreferredReservedAcceleratorsTest, AcceleratorsWithPinned) {
 }
 
 TEST_F(AcceleratorControllerTest, DisallowedAtModalWindow) {
-  // TODO: TestScreenshotDelegate is null in mash http://crbug.com/632111.
-  if (Shell::GetAshConfig() == Config::MASH)
-    return;
-
   std::set<AcceleratorAction> all_actions;
   for (size_t i = 0; i < kAcceleratorDataLength; ++i)
     all_actions.insert(kAcceleratorData[i].action);
@@ -1302,10 +1283,6 @@ class DeprecatedAcceleratorTester : public AcceleratorControllerTest {
 }  // namespace
 
 TEST_F(DeprecatedAcceleratorTester, TestDeprecatedAcceleratorsBehavior) {
-  // TODO: disabled because of UnblockUserSession() not working:
-  // http://crbug.com/632201.
-  if (Shell::GetAshConfig() == Config::MASH)
-    return;
   for (size_t i = 0; i < kDeprecatedAcceleratorsLength; ++i) {
     const AcceleratorData& entry = kDeprecatedAccelerators[i];
 

ash/mus/accelerators/accelerator_controller_delegate_mus.cc

@@ -39,9 +39,6 @@ bool AcceleratorControllerDelegateMus::HandlesAction(AcceleratorAction action) {
     case MAGNIFY_SCREEN_ZOOM_OUT:
     case POWER_PRESSED:
     case POWER_RELEASED:
-    case TAKE_PARTIAL_SCREENSHOT:
-    case TAKE_SCREENSHOT:
-    case TAKE_WINDOW_SCREENSHOT:
     case TOUCH_HUD_CLEAR:
     case TOUCH_HUD_MODE_CHANGE:
     case UNPIN:

ash/mus/bridge/shell_port_mash.cc

@@ -177,7 +177,7 @@ ShellPortMash::CreateAcceleratorController() {
   mash_state_->accelerator_controller_delegate =
       std::make_unique<AcceleratorControllerDelegateMus>(window_manager_);
   mash_state_->accelerator_controller_registrar =
-      base ::MakeUnique<AcceleratorControllerRegistrar>(
+      std::make_unique<AcceleratorControllerRegistrar>(
           window_manager_, accelerator_namespace_id);
   return std::make_unique<AcceleratorController>(
       mash_state_->accelerator_controller_delegate.get(),

ash/mus/shell_delegate_mus.cc

@@ -10,6 +10,7 @@
 #include "ash/accessibility/default_accessibility_delegate.h"
 #include "ash/gpu_support_stub.h"
 #include "ash/mus/wallpaper_delegate_mus.h"
+#include "ash/screenshot_delegate.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "components/user_manager/user_info_impl.h"
@@ -18,6 +19,30 @@
 #include "ui/keyboard/keyboard_ui.h"
 
 namespace ash {
+namespace {
+
+// TODO(jamescook): Replace with a mojo-compatible ScreenshotClient.
+class ScreenshotDelegateMash : public ScreenshotDelegate {
+ public:
+  ScreenshotDelegateMash() = default;
+  ~ScreenshotDelegateMash() override = default;
+
+  // ScreenshotDelegate:
+  void HandleTakeScreenshotForAllRootWindows() override { NOTIMPLEMENTED(); }
+  void HandleTakePartialScreenshot(aura::Window* window,
+                                   const gfx::Rect& rect) override {
+    NOTIMPLEMENTED();
+  }
+  void HandleTakeWindowScreenshot(aura::Window* window) override {
+    NOTIMPLEMENTED();
+  }
+  bool CanTakeScreenshot() override { return true; }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScreenshotDelegateMash);
+};
+
+}  // namespace
 
 ShellDelegateMus::ShellDelegateMus(service_manager::Connector* connector)
     : connector_(connector) {}
@@ -67,6 +92,11 @@ NetworkingConfigDelegate* ShellDelegateMus::GetNetworkingConfigDelegate() {
   return nullptr;
 }
 
+std::unique_ptr<ScreenshotDelegate>
+ShellDelegateMus::CreateScreenshotDelegate() {
+  return std::make_unique<ScreenshotDelegateMash>();
+}
+
 std::unique_ptr<WallpaperDelegate> ShellDelegateMus::CreateWallpaperDelegate() {
   return std::make_unique<WallpaperDelegateMus>();
 }

ash/mus/shell_delegate_mus.h

@@ -31,6 +31,7 @@ class ShellDelegateMus : public ShellDelegate {
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;
+  std::unique_ptr<ScreenshotDelegate> CreateScreenshotDelegate() override;
   std::unique_ptr<WallpaperDelegate> CreateWallpaperDelegate() override;
   AccessibilityDelegate* CreateAccessibilityDelegate() override;
   GPUSupport* CreateGPUSupport() override;

ash/shell.cc

@@ -58,6 +58,7 @@
 #include "ash/public/cpp/shelf_model.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/root_window_controller.h"
+#include "ash/screenshot_delegate.h"
 #include "ash/session/session_controller.h"
 #include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_controller.h"
@@ -1068,7 +1069,8 @@ void Shell::Init(const ShellInitParams& init_params) {
   // pre-target handler) at this point, because |mouse_cursor_filter_| needs to
   // process mouse events prior to screenshot session.
   // See http://crbug.com/459214
-  screenshot_controller_.reset(new ScreenshotController());
+  screenshot_controller_ = std::make_unique<ScreenshotController>(
+      shell_delegate_->CreateScreenshotDelegate());
   mouse_cursor_filter_ = std::make_unique<MouseCursorEventFilter>();
   PrependPreTargetHandler(mouse_cursor_filter_.get());
 

ash/shell/shell_delegate_impl.cc

@@ -16,6 +16,7 @@
 #include "ash/shell.h"
 #include "ash/shell/example_factory.h"
 #include "ash/shell/toplevel_window.h"
+#include "ash/test_screenshot_delegate.h"
 #include "ash/wm/window_state.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/user_manager/user_info_impl.h"
@@ -60,6 +61,11 @@ NetworkingConfigDelegate* ShellDelegateImpl::GetNetworkingConfigDelegate() {
   return nullptr;
 }
 
+std::unique_ptr<ash::ScreenshotDelegate>
+ShellDelegateImpl::CreateScreenshotDelegate() {
+  return std::make_unique<TestScreenshotDelegate>();
+}
+
 std::unique_ptr<WallpaperDelegate>
 ShellDelegateImpl::CreateWallpaperDelegate() {
   return std::make_unique<DefaultWallpaperDelegate>();

ash/shell/shell_delegate_impl.h

@@ -33,6 +33,7 @@ class ShellDelegateImpl : public ShellDelegate {
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;
+  std::unique_ptr<ScreenshotDelegate> CreateScreenshotDelegate() override;
   std::unique_ptr<WallpaperDelegate> CreateWallpaperDelegate() override;
   AccessibilityDelegate* CreateAccessibilityDelegate() override;
   GPUSupport* CreateGPUSupport() override;

ash/shell_delegate.h

@@ -39,6 +39,7 @@ namespace ash {
 class AccessibilityDelegate;
 class GPUSupport;
 class NetworkingConfigDelegate;
+class ScreenshotDelegate;
 class WallpaperDelegate;
 
 // Delegate of the Shell.
@@ -79,6 +80,9 @@ class ASH_EXPORT ShellDelegate {
   // Returns the delegate. May be null in tests.
   virtual NetworkingConfigDelegate* GetNetworkingConfigDelegate() = 0;
 
+  // TODO(jamescook): Replace with a mojo-compatible interface.
+  virtual std::unique_ptr<ScreenshotDelegate> CreateScreenshotDelegate() = 0;
+
   // Creates a wallpaper delegate. Shell takes ownership of the delegate.
   virtual std::unique_ptr<WallpaperDelegate> CreateWallpaperDelegate() = 0;
 

ash/system/palette/tools/capture_region_mode.cc

@@ -4,12 +4,8 @@
 
 #include "ash/system/palette/tools/capture_region_mode.h"
 
-#include "ash/accelerators/accelerator_controller_delegate_classic.h"
-#include "ash/public/cpp/config.h"
 #include "ash/resources/vector_icons/vector_icons.h"
-#include "ash/screenshot_delegate.h"
 #include "ash/shell.h"
-#include "ash/shell_port_classic.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/palette/palette_ids.h"
 #include "ash/system/toast/toast_data.h"
@@ -51,18 +47,9 @@ void CaptureRegionMode::OnEnable() {
                   kToastDurationMs, base::Optional<base::string16>());
   Shell::Get()->toast_manager()->Show(toast);
 
-  if (Shell::GetAshConfig() != Config::CLASSIC) {
-    // TODO(kaznacheev): Support MASH once http://crbug.com/557397 is fixed.
-    NOTIMPLEMENTED();
-    return;
-  }
-
   auto* screenshot_controller = Shell::Get()->screenshot_controller();
   screenshot_controller->set_pen_events_only(true);
   screenshot_controller->StartPartialScreenshotSession(
-      ShellPortClassic::Get()
-          ->accelerator_controller_delegate()
-          ->screenshot_delegate(),
       false /* draw_overlay_immediately */);
   screenshot_controller->set_on_screenshot_session_done(base::BindOnce(
       &CaptureRegionMode::OnScreenshotDone, weak_factory_.GetWeakPtr()));

ash/system/palette/tools/capture_screen_action.cc

@@ -4,14 +4,11 @@
 
 #include "ash/system/palette/tools/capture_screen_action.h"
 
-#include "ash/accelerators/accelerator_controller_delegate_classic.h"
-#include "ash/public/cpp/config.h"
 #include "ash/resources/vector_icons/vector_icons.h"
-#include "ash/screenshot_delegate.h"
 #include "ash/shell.h"
-#include "ash/shell_port_classic.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/palette/palette_ids.h"
+#include "ash/utility/screenshot_controller.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace ash {
@@ -35,16 +32,7 @@ void CaptureScreenAction::OnEnable() {
   delegate()->DisableTool(GetToolId());
   delegate()->HidePaletteImmediately();
 
-  if (Shell::GetAshConfig() != Config::CLASSIC) {
-    // TODO(kaznacheev): Support MASH once http://crbug.com/557397 is fixed.
-    NOTIMPLEMENTED();
-    return;
-  }
-
-  ShellPortClassic::Get()
-      ->accelerator_controller_delegate()
-      ->screenshot_delegate()
-      ->HandleTakeScreenshotForAllRootWindows();
+  Shell::Get()->screenshot_controller()->TakeScreenshotForAllRootWindows();
 }
 
 views::View* CaptureScreenAction::CreateView() {

ash/system/palette/tools/screenshot_unittest.cc

@@ -13,7 +13,6 @@
 #include "ash/system/palette/tools/capture_region_mode.h"
 #include "ash/system/palette/tools/capture_screen_action.h"
 #include "ash/test/ash_test_base.h"
-#include "ash/test/ash_test_helper.h"
 #include "ash/test_screenshot_delegate.h"
 #include "ash/utility/screenshot_controller.h"
 #include "base/macros.h"
@@ -33,10 +32,6 @@ class ScreenshotToolTest : public AshTestBase {
     palette_tool_delegate_ = std::make_unique<MockPaletteToolDelegate>();
   }
 
-  TestScreenshotDelegate* test_screenshot_delegate() {
-    return ash_test_helper()->test_screenshot_delegate();
-  }
-
  protected:
   std::unique_ptr<MockPaletteToolDelegate> palette_tool_delegate_;
 
@@ -75,10 +70,9 @@ TEST_F(ScreenshotToolTest, EnablingCaptureRegionCallsDelegateAndDisablesTool) {
   GetEventGenerator().ReleaseTouch();
 
   EXPECT_FALSE(IsPartialScreenshotActive());
-  EXPECT_EQ(1,
-            test_screenshot_delegate()->handle_take_partial_screenshot_count());
+  EXPECT_EQ(1, GetScreenshotDelegate()->handle_take_partial_screenshot_count());
   EXPECT_EQ(selection.ToString(),
-            test_screenshot_delegate()->last_rect().ToString());
+            GetScreenshotDelegate()->last_rect().ToString());
   testing::Mock::VerifyAndClearExpectations(palette_tool_delegate_.get());
 
   // Enable the tool again
@@ -101,7 +95,7 @@ TEST_F(ScreenshotToolTest, EnablingCaptureScreenCallsDelegateAndDisablesTool) {
               DisableTool(PaletteToolId::CAPTURE_SCREEN));
   EXPECT_CALL(*palette_tool_delegate_.get(), HidePaletteImmediately());
   tool->OnEnable();
-  EXPECT_EQ(1, test_screenshot_delegate()->handle_take_screenshot_count());
+  EXPECT_EQ(1, GetScreenshotDelegate()->handle_take_screenshot_count());
 }
 
 }  // namespace ash

ash/test/ash_test_base.cc

@@ -24,7 +24,9 @@
 #include "ash/shell/toplevel_window.h"
 #include "ash/test/ash_test_environment.h"
 #include "ash/test/ash_test_helper.h"
+#include "ash/test_screenshot_delegate.h"
 #include "ash/test_shell_delegate.h"
+#include "ash/utility/screenshot_controller.h"
 #include "ash/wm/window_positioner.h"
 #include "base/memory/ptr_util.h"
 #include "components/signin/core/account_id/account_id.h"
@@ -374,7 +376,8 @@ void AshTestBase::RunAllPendingInMessageLoop() {
 }
 
 TestScreenshotDelegate* AshTestBase::GetScreenshotDelegate() {
-  return ash_test_helper_->test_screenshot_delegate();
+  return static_cast<TestScreenshotDelegate*>(
+      Shell::Get()->screenshot_controller()->screenshot_delegate_.get());
 }
 
 TestSessionControllerClient* AshTestBase::GetSessionControllerClient() {

ash/test/ash_test_helper.cc

@@ -8,7 +8,6 @@
 #include <memory>
 #include <set>
 
-#include "ash/accelerators/accelerator_controller_delegate_classic.h"
 #include "ash/display/display_configuration_controller_test_api.h"
 #include "ash/mus/bridge/shell_port_mash.h"
 #include "ash/mus/shell_port_mus.h"
@@ -23,7 +22,6 @@
 #include "ash/system/screen_layout_observer.h"
 #include "ash/test/ash_test_environment.h"
 #include "ash/test/ash_test_views_delegate.h"
-#include "ash/test_screenshot_delegate.h"
 #include "ash/test_shell_delegate.h"
 #include "base/run_loop.h"
 #include "base/strings/string_split.h"
@@ -66,7 +64,6 @@ Config AshTestHelper::config_ = Config::CLASSIC;
 AshTestHelper::AshTestHelper(AshTestEnvironment* ash_test_environment)
     : ash_test_environment_(ash_test_environment),
       test_shell_delegate_(nullptr),
-      test_screenshot_delegate_(nullptr),
       dbus_thread_manager_initialized_(false),
       bluez_dbus_manager_initialized_(false) {
   ui::test::EnableTestConfigForPlatformWindows();
@@ -187,22 +184,6 @@ void AshTestHelper::SetUp(bool start_session, bool provide_local_state) {
   DisplayConfigurationControllerTestApi(
       shell->display_configuration_controller())
       .DisableDisplayAnimator();
-
-  if (config_ == Config::CLASSIC) {
-    // TODO: disabled for mash as AcceleratorControllerDelegateClassic isn't
-    // created in mash http://crbug.com/632111.
-    test_screenshot_delegate_ = new TestScreenshotDelegate();
-    ShellPortClassic::Get()
-        ->accelerator_controller_delegate()
-        ->SetScreenshotDelegate(
-            std::unique_ptr<ScreenshotDelegate>(test_screenshot_delegate_));
-  } else if (config_ == Config::MUS) {
-    test_screenshot_delegate_ = new TestScreenshotDelegate();
-    mus::ShellPortMus::Get()
-        ->accelerator_controller_delegate()
-        ->SetScreenshotDelegate(
-            std::unique_ptr<ScreenshotDelegate>(test_screenshot_delegate_));
-  }
 }
 
 void AshTestHelper::TearDown() {
@@ -217,8 +198,6 @@ void AshTestHelper::TearDown() {
   RunAllPendingInMessageLoop();
   ash_test_environment_->TearDown();
 
-  test_screenshot_delegate_ = NULL;
-
   if (config_ == Config::CLASSIC) {
     // Remove global message center state.
     message_center::MessageCenter::Shutdown();

ash/test/ash_test_helper.h

@@ -47,7 +47,6 @@ namespace ash {
 class AshTestEnvironment;
 class AshTestViewsDelegate;
 class RootWindowController;
-class TestScreenshotDelegate;
 class TestShellDelegate;
 class TestSessionControllerClient;
 
@@ -88,9 +87,6 @@ class AshTestHelper {
   void set_test_shell_delegate(TestShellDelegate* test_shell_delegate) {
     test_shell_delegate_ = test_shell_delegate;
   }
-  TestScreenshotDelegate* test_screenshot_delegate() {
-    return test_screenshot_delegate_;
-  }
   AshTestViewsDelegate* test_views_delegate() {
     return test_views_delegate_.get();
   }
@@ -140,9 +136,6 @@ class AshTestHelper {
   TestShellDelegate* test_shell_delegate_;  // Owned by ash::Shell.
   std::unique_ptr<ui::ScopedAnimationDurationScaleMode> zero_duration_mode_;
 
-  // Owned by ash::AcceleratorController.
-  TestScreenshotDelegate* test_screenshot_delegate_;
-
   std::unique_ptr<::wm::WMState> wm_state_;
   std::unique_ptr<AshTestViewsDelegate> test_views_delegate_;
 

ash/test_shell_delegate.cc

@@ -8,6 +8,7 @@
 #include "ash/gpu_support_stub.h"
 #include "ash/keyboard/test_keyboard_ui.h"
 #include "ash/system/tray/system_tray_notifier.h"
+#include "ash/test_screenshot_delegate.h"
 #include "ash/wallpaper/test_wallpaper_delegate.h"
 #include "base/logging.h"
 #include "ui/gfx/image/image.h"
@@ -48,6 +49,11 @@ NetworkingConfigDelegate* TestShellDelegate::GetNetworkingConfigDelegate() {
   return nullptr;
 }
 
+std::unique_ptr<ScreenshotDelegate>
+TestShellDelegate::CreateScreenshotDelegate() {
+  return std::make_unique<TestScreenshotDelegate>();
+}
+
 std::unique_ptr<WallpaperDelegate>
 TestShellDelegate::CreateWallpaperDelegate() {
   return std::make_unique<TestWallpaperDelegate>();

ash/test_shell_delegate.h

@@ -27,6 +27,7 @@ class TestShellDelegate : public ShellDelegate {
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;
+  std::unique_ptr<ScreenshotDelegate> CreateScreenshotDelegate() override;
   std::unique_ptr<WallpaperDelegate> CreateWallpaperDelegate() override;
   AccessibilityDelegate* CreateAccessibilityDelegate() override;
   GPUSupport* CreateGPUSupport() override;

ash/utility/screenshot_controller.cc

@@ -258,29 +258,35 @@ class ScreenshotController::ScopedCursorSetter {
   DISALLOW_COPY_AND_ASSIGN(ScopedCursorSetter);
 };
 
-ScreenshotController::ScreenshotController()
+ScreenshotController::ScreenshotController(
+    std::unique_ptr<ScreenshotDelegate> delegate)
     : mode_(NONE),
       root_window_(nullptr),
       selected_(nullptr),
-      screenshot_delegate_(nullptr) {
+      screenshot_delegate_(std::move(delegate)) {
   // Keep this here and don't move it to StartPartialScreenshotSession(), as it
   // needs to be pre-pended by MouseCursorEventFilter in Shell::Init().
   Shell::Get()->PrependPreTargetHandler(this);
 }
 
 ScreenshotController::~ScreenshotController() {
-  if (screenshot_delegate_)
+  if (in_screenshot_session_)
     CancelScreenshotSession();
   Shell::Get()->RemovePreTargetHandler(this);
 }
 
-void ScreenshotController::StartWindowScreenshotSession(
-    ScreenshotDelegate* screenshot_delegate) {
-  if (screenshot_delegate_) {
-    DCHECK_EQ(screenshot_delegate_, screenshot_delegate);
+void ScreenshotController::TakeScreenshotForAllRootWindows() {
+  DCHECK(screenshot_delegate_);
+  if (screenshot_delegate_->CanTakeScreenshot())
+    screenshot_delegate_->HandleTakeScreenshotForAllRootWindows();
+}
+
+void ScreenshotController::StartWindowScreenshotSession() {
+  DCHECK(screenshot_delegate_);
+  // Already in a screenshot session.
+  if (in_screenshot_session_)
     return;
-  }
-  screenshot_delegate_ = screenshot_delegate;
+  in_screenshot_session_ = true;
   mode_ = WINDOW;
 
   display::Screen::GetScreen()->AddObserver(this);
@@ -298,15 +304,12 @@ void ScreenshotController::StartWindowScreenshotSession(
 }
 
 void ScreenshotController::StartPartialScreenshotSession(
-    ScreenshotDelegate* screenshot_delegate,
     bool draw_overlay_immediately) {
+  DCHECK(screenshot_delegate_);
   // Already in a screenshot session.
-  if (screenshot_delegate_) {
-    DCHECK_EQ(screenshot_delegate_, screenshot_delegate);
+  if (in_screenshot_session_)
     return;
-  }
-
-  screenshot_delegate_ = screenshot_delegate;
+  in_screenshot_session_ = true;
   mode_ = PARTIAL;
   display::Screen::GetScreen()->AddObserver(this);
   for (aura::Window* root : Shell::GetAllRootWindows()) {
@@ -336,7 +339,7 @@ void ScreenshotController::CancelScreenshotSession() {
   pen_events_only_ = false;
   root_window_ = nullptr;
   SetSelectedWindow(nullptr);
-  screenshot_delegate_ = nullptr;
+  in_screenshot_session_ = false;
   display::Screen::GetScreen()->RemoveObserver(this);
   layers_.clear();
   cursor_setter_.reset();
@@ -454,7 +457,7 @@ bool ScreenshotController::ShouldProcessEvent(
 }
 
 void ScreenshotController::OnKeyEvent(ui::KeyEvent* event) {
-  if (!screenshot_delegate_)
+  if (!in_screenshot_session_)
     return;
 
   if (event->type() == ui::ET_KEY_RELEASED) {
@@ -474,7 +477,7 @@ void ScreenshotController::OnKeyEvent(ui::KeyEvent* event) {
 }
 
 void ScreenshotController::OnMouseEvent(ui::MouseEvent* event) {
-  if (!screenshot_delegate_ || !ShouldProcessEvent(event->pointer_details()))
+  if (!in_screenshot_session_ || !ShouldProcessEvent(event->pointer_details()))
     return;
   switch (mode_) {
     case NONE:
@@ -515,7 +518,7 @@ void ScreenshotController::OnMouseEvent(ui::MouseEvent* event) {
 }
 
 void ScreenshotController::OnTouchEvent(ui::TouchEvent* event) {
-  if (!screenshot_delegate_ || !ShouldProcessEvent(event->pointer_details()))
+  if (!in_screenshot_session_ || !ShouldProcessEvent(event->pointer_details()))
     return;
   switch (mode_) {
     case NONE:
@@ -556,14 +559,14 @@ void ScreenshotController::OnTouchEvent(ui::TouchEvent* event) {
 }
 
 void ScreenshotController::OnDisplayAdded(const display::Display& new_display) {
-  if (!screenshot_delegate_)
+  if (!in_screenshot_session_)
     return;
   CancelScreenshotSession();
 }
 
 void ScreenshotController::OnDisplayRemoved(
     const display::Display& old_display) {
-  if (!screenshot_delegate_)
+  if (!in_screenshot_session_)
     return;
   CancelScreenshotSession();
 }

ash/utility/screenshot_controller.h

@@ -32,28 +32,28 @@ namespace ash {
 class ScreenshotDelegate;
 
 // This class controls a session of taking partial/window screenshot, i.e.:
-// drawing
-// region rectangles during selection, and changing the mouse cursor to indicate
-// the current mode.
-// This class does not use aura::Window / views::Widget intentionally to avoid
+// drawing region rectangles during selection, and changing the mouse cursor to
+// indicate the current mode.
 class ASH_EXPORT ScreenshotController : public ui::EventHandler,
                                         public display::DisplayObserver,
                                         public aura::WindowObserver {
  public:
-  ScreenshotController();
+  explicit ScreenshotController(std::unique_ptr<ScreenshotDelegate> delegate);
   ~ScreenshotController() override;
 
+  // Takes a default "whole screen" screenshot.
+  void TakeScreenshotForAllRootWindows();
+
   // Starts the UI for taking partial screenshot; dragging to select a region.
   // ScreenshotController manage their own lifetime so caller must not
   // delete the returned values. |draw_overlay_immediately| controls if the grey
   // overlay will be drawn immediately. If false, then the overlay will be drawn
   // only after the user has started creating the clipping rect for the
   // screenshot.
-  void StartPartialScreenshotSession(ScreenshotDelegate* screenshot_delegate,
-                                     bool draw_overlay_immediately);
+  void StartPartialScreenshotSession(bool draw_overlay_immediately);
 
   // Starts the UI for taking a window screenshot;
-  void StartWindowScreenshotSession(ScreenshotDelegate* screenshot_delegate);
+  void StartWindowScreenshotSession();
 
   // Cancels any active screenshot session.
   void CancelScreenshotSession();
@@ -80,6 +80,7 @@ class ASH_EXPORT ScreenshotController : public ui::EventHandler,
     WINDOW,
   };
 
+  friend class AshTestBase;
   friend class ScreenshotControllerTest;
   friend class ScreenshotToolTest;
 
@@ -132,8 +133,11 @@ class ASH_EXPORT ScreenshotController : public ui::EventHandler,
   // The object to specify the crosshair cursor.
   std::unique_ptr<ScopedCursorSetter> cursor_setter_;
 
-  // ScreenshotDelegate to take the actual screenshot. No ownership.
-  ScreenshotDelegate* screenshot_delegate_;
+  // True while taking a partial or window screen.
+  bool in_screenshot_session_ = false;
+
+  // TODO(jamescook): Replace with a mojo-compatible interface.
+  std::unique_ptr<ScreenshotDelegate> screenshot_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenshotController);
 };

ash/utility/screenshot_controller_unittest.cc

@@ -37,20 +37,16 @@ class ScreenshotControllerTest : public AshTestBase {
   }
 
   void StartPartialScreenshotSession() {
-    screenshot_controller()->StartPartialScreenshotSession(
-        GetScreenshotDelegate(), true);
+    screenshot_controller()->StartPartialScreenshotSession(true);
   }
 
   void StartWindowScreenshotSession() {
-    screenshot_controller()->StartWindowScreenshotSession(
-        GetScreenshotDelegate());
+    screenshot_controller()->StartWindowScreenshotSession();
   }
 
   void Cancel() { screenshot_controller()->CancelScreenshotSession(); }
 
-  bool IsActive() {
-    return screenshot_controller()->screenshot_delegate_ != nullptr;
-  }
+  bool IsActive() { return screenshot_controller()->in_screenshot_session_; }
 
   const gfx::Point& GetStartPosition() const {
     return Shell::Get()->screenshot_controller()->start_position_;
@@ -272,7 +268,7 @@ TEST_F(PartialScreenshotControllerTest, MouseWarpTest) {
             aura::Env::GetInstance()->last_mouse_location());
 }
 
-TEST_F(PartialScreenshotControllerTest, VisibilityTest) {
+TEST_F(PartialScreenshotControllerTest, CursorVisibilityTest) {
   aura::client::CursorClient* client = Shell::Get()->cursor_manager();
 
   GetEventGenerator().PressKey(ui::VKEY_A, 0);

chrome/browser/policy/policy_browsertest.cc

@@ -11,7 +11,6 @@
 #include <utility>
 #include <vector>
 
-#include "ash/accelerators/accelerator_controller_delegate_classic.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
@@ -204,12 +203,9 @@
 #include "url/origin.h"
 
 #if defined(OS_CHROMEOS)
-#include "ash/accelerators/accelerator_controller.h"
-#include "ash/accelerators/accelerator_table.h"
 #include "ash/accessibility_types.h"
 #include "ash/public/cpp/ash_switches.h"
 #include "ash/shell.h"
-#include "ash/shell_port_classic.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
@@ -680,30 +676,16 @@ class PolicyTest : public InProcessBrowserTest {
   };
 
   void TestScreenshotFile(bool enabled) {
-    // AddObserver is an ash-specific method, so just replace the screenshot
-    // grabber with one we've created here.
-    std::unique_ptr<ChromeScreenshotGrabber> chrome_screenshot_grabber(
-        new ChromeScreenshotGrabber);
     // ScreenshotGrabber doesn't own this observer, so the observer's lifetime
     // is tied to the test instead.
-    chrome_screenshot_grabber->screenshot_grabber()->AddObserver(&observer_);
-    ash::ShellPortClassic::Get()
-        ->accelerator_controller_delegate()
-        ->SetScreenshotDelegate(std::move(chrome_screenshot_grabber));
-
+    ChromeScreenshotGrabber* grabber = ChromeScreenshotGrabber::Get();
+    grabber->screenshot_grabber()->AddObserver(&observer_);
     SetScreenshotPolicy(enabled);
-    ash::Shell::Get()->accelerator_controller()->PerformActionIfEnabled(
-        ash::TAKE_SCREENSHOT);
-
+    grabber->HandleTakeScreenshotForAllRootWindows();
     content::RunMessageLoop();
-    static_cast<ChromeScreenshotGrabber*>(
-        ash::ShellPortClassic::Get()
-            ->accelerator_controller_delegate()
-            ->screenshot_delegate())
-        ->screenshot_grabber()
-        ->RemoveObserver(&observer_);
+    grabber->screenshot_grabber()->RemoveObserver(&observer_);
   }
-#endif
+#endif  // defined(OS_CHROMEOS)
 
   ExtensionService* extension_service() {
     extensions::ExtensionSystem* system =

chrome/browser/ui/DEPS

@@ -17,11 +17,4 @@ specific_include_rules = {
   "browser_command_controller\.cc": [
     "+ash/accelerators/accelerator_commands_classic.h",
   ],
-  "browser_commands_chromeos\.cc": [
-    "+ash/accelerators/accelerator_controller_delegate_classic.h",
-    "+ash/mus/shell_port_mus.h",
-    # TODO(mash): Screenshot support. http://crbug.com/557397
-    "+ash/screenshot_delegate.h",
-    "+ash/shell_port_classic.h",
-  ]
 }

chrome/browser/ui/ash/ash_init.cc

@@ -4,17 +4,13 @@
 
 #include "chrome/browser/ui/ash/ash_init.h"
 
-#include "ash/accelerators/accelerator_controller.h"
-#include "ash/accelerators/accelerator_controller_delegate_classic.h"
 #include "ash/accessibility_types.h"
 #include "ash/high_contrast/high_contrast_controller.h"
 #include "ash/magnifier/magnification_controller.h"
-#include "ash/mus/shell_port_mus.h"  // mash-ok
 #include "ash/mus/window_manager.h"
 #include "ash/public/cpp/config.h"
 #include "ash/shell.h"
 #include "ash/shell_init_params.h"
-#include "ash/shell_port_classic.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/task_scheduler/post_task.h"
@@ -27,7 +23,6 @@
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/ash_config.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
-#include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
 #include "chrome/browser/ui/ash/chrome_shell_content_state.h"
 #include "chrome/browser/ui/ash/chrome_shell_delegate.h"
 #include "chrome/common/chrome_switches.h"
@@ -97,24 +92,6 @@ AshInit::AshInit() {
   else
     CreateClassicShell();
 
-  ash::AcceleratorControllerDelegateClassic* accelerator_controller_delegate =
-      nullptr;
-  if (chromeos::GetAshConfig() == ash::Config::CLASSIC) {
-    accelerator_controller_delegate =
-        ash::ShellPortClassic::Get()->accelerator_controller_delegate();
-  } else if (chromeos::GetAshConfig() == ash::Config::MUS) {
-    accelerator_controller_delegate =
-        ash::mus::ShellPortMus::Get()->accelerator_controller_delegate();
-  } else {
-    // TODO(mash): Screenshot accelerator support. http://crbug.com/557397
-    NOTIMPLEMENTED();
-  }
-  if (accelerator_controller_delegate) {
-    std::unique_ptr<ChromeScreenshotGrabber> screenshot_delegate =
-        base::MakeUnique<ChromeScreenshotGrabber>();
-    accelerator_controller_delegate->SetScreenshotDelegate(
-        std::move(screenshot_delegate));
-  }
   // TODO(flackr): Investigate exposing a blocking pool task runner to chromeos.
   chromeos::AccelerometerReader::GetInstance()->Initialize(
       base::CreateSequencedTaskRunnerWithTraits(

chrome/browser/ui/ash/chrome_screenshot_grabber.cc

@@ -65,6 +65,8 @@ constexpr base::TaskTraits kBlockingTaskTraits = {
     base::MayBlock(), base::TaskPriority::USER_VISIBLE,
     base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN};
 
+ChromeScreenshotGrabber* g_instance = nullptr;
+
 void CopyScreenshotToClipboard(scoped_refptr<base::RefCountedString> png_data) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
@@ -302,12 +304,21 @@ ChromeScreenshotGrabber::ChromeScreenshotGrabber()
     : screenshot_grabber_(new ui::ScreenshotGrabber(this)),
       weak_factory_(this) {
   screenshot_grabber_->AddObserver(this);
+  DCHECK(!g_instance);
+  g_instance = this;
 }
 
 ChromeScreenshotGrabber::~ChromeScreenshotGrabber() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
   screenshot_grabber_->RemoveObserver(this);
 }
 
+// static
+ChromeScreenshotGrabber* ChromeScreenshotGrabber::Get() {
+  return g_instance;
+}
+
 void ChromeScreenshotGrabber::HandleTakeScreenshotForAllRootWindows() {
   if (ScreenshotsDisabled()) {
     screenshot_grabber_->NotifyScreenshotCompleted(

chrome/browser/ui/ash/chrome_screenshot_grabber.h

@@ -30,6 +30,8 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
   ChromeScreenshotGrabber();
   ~ChromeScreenshotGrabber() override;
 
+  static ChromeScreenshotGrabber* Get();
+
   ui::ScreenshotGrabber* screenshot_grabber() {
     return screenshot_grabber_.get();
   }

chrome/browser/ui/ash/chrome_screenshot_grabber_browsertest.cc

@@ -48,8 +48,8 @@ class ChromeScreenshotGrabberBrowserTest
 IN_PROC_BROWSER_TEST_F(ChromeScreenshotGrabberBrowserTest, TakeScreenshot) {
   message_center::MessageCenter::Get()->AddObserver(this);
 
-  std::unique_ptr<ChromeScreenshotGrabber> chrome_screenshot_grabber =
-      base::MakeUnique<ChromeScreenshotGrabber>();
+  ChromeScreenshotGrabber* chrome_screenshot_grabber =
+      ChromeScreenshotGrabber::Get();
   chrome_screenshot_grabber->screenshot_grabber()->AddObserver(this);
   base::ScopedTempDir directory;
   ASSERT_TRUE(directory.CreateUniqueTempDir());
@@ -64,6 +64,7 @@ IN_PROC_BROWSER_TEST_F(ChromeScreenshotGrabberBrowserTest, TakeScreenshot) {
 
   message_loop_runner_ = new content::MessageLoopRunner;
   message_loop_runner_->Run();
+  chrome_screenshot_grabber->screenshot_grabber()->RemoveObserver(this);
 
   EXPECT_TRUE(notification_added_);
   EXPECT_NE(nullptr,

chrome/browser/ui/ash/chrome_shell_delegate.cc

@@ -41,6 +41,7 @@
 #include "chrome/browser/speech/tts_controller.h"
 #include "chrome/browser/sync/sync_error_notifier_factory_ash.h"
 #include "chrome/browser/ui/ash/chrome_keyboard_ui.h"
+#include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/browser/ui/ash/networking_config_delegate_chromeos.h"
@@ -479,6 +480,11 @@ ChromeShellDelegate::GetNetworkingConfigDelegate() {
   return networking_config_delegate_.get();
 }
 
+std::unique_ptr<ash::ScreenshotDelegate>
+ChromeShellDelegate::CreateScreenshotDelegate() {
+  return std::make_unique<ChromeScreenshotGrabber>();
+}
+
 std::unique_ptr<ash::WallpaperDelegate>
 ChromeShellDelegate::CreateWallpaperDelegate() {
   return base::WrapUnique(chromeos::CreateWallpaperDelegate());

chrome/browser/ui/ash/chrome_shell_delegate.h

@@ -38,6 +38,7 @@ class ChromeShellDelegate : public ash::ShellDelegate,
   std::unique_ptr<keyboard::KeyboardUI> CreateKeyboardUI() override;
   void OpenUrlFromArc(const GURL& url) override;
   ash::NetworkingConfigDelegate* GetNetworkingConfigDelegate() override;
+  std::unique_ptr<ash::ScreenshotDelegate> CreateScreenshotDelegate() override;
   std::unique_ptr<ash::WallpaperDelegate> CreateWallpaperDelegate() override;
   ash::AccessibilityDelegate* CreateAccessibilityDelegate() override;
   ash::GPUSupport* CreateGPUSupport() override;

chrome/browser/ui/browser_commands_chromeos.cc

@@ -4,34 +4,15 @@
 
 #include "chrome/browser/ui/browser_commands_chromeos.h"
 
-#include "ash/accelerators/accelerator_controller_delegate_classic.h"  // mash-ok
-#include "ash/mus/shell_port_mus.h"  // mash-ok
-#include "ash/public/cpp/config.h"
-#include "ash/screenshot_delegate.h"
-#include "ash/shell_port_classic.h"  // mash-ok
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
-#include "chrome/browser/chromeos/ash_config.h"
+#include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
 
 using base::UserMetricsAction;
 
 void TakeScreenshot() {
   base::RecordAction(UserMetricsAction("Menu_Take_Screenshot"));
-  ash::AcceleratorControllerDelegateClassic* accelerator_controller_delegate =
-      nullptr;
-  if (chromeos::GetAshConfig() == ash::Config::CLASSIC) {
-    accelerator_controller_delegate =
-        ash::ShellPortClassic::Get()->accelerator_controller_delegate();
-  } else if (chromeos::GetAshConfig() == ash::Config::MUS) {
-    accelerator_controller_delegate =
-        ash::mus::ShellPortMus::Get()->accelerator_controller_delegate();
-  } else {
-    // TODO(mash): Screenshot support. http://crbug.com/557397
-    NOTIMPLEMENTED();
-    return;
-  }
-  ash::ScreenshotDelegate* screenshot_delegate =
-      accelerator_controller_delegate->screenshot_delegate();
-  if (screenshot_delegate && screenshot_delegate->CanTakeScreenshot())
-    screenshot_delegate->HandleTakeScreenshotForAllRootWindows();
+  ChromeScreenshotGrabber* grabber = ChromeScreenshotGrabber::Get();
+  if (grabber->CanTakeScreenshot())
+    grabber->HandleTakeScreenshotForAllRootWindows();
 }

testing/buildbot/filters/ash_unittests_mash.filter

@@ -2,8 +2,6 @@
 # include a link. If a test should never run on mash, add an early exit to the
 # code instead of listing here. See Shell::GetAshConfig().
 
-# TODO: fails because of no screenshot in mash. http://crbug.com/698033.
--ClamshellPowerButtonScreenshotControllerTest.PowerButtonPressedFirst_StopClamshellPowerButtonTimer
 -CursorWindowControllerTest.DSF
 -CursorWindowControllerTest.MoveToDifferentDisplay
 -CursorWindowControllerTest.ShouldEnableCursorCompositing
@@ -96,32 +94,17 @@
 -OverviewGestureHandlerTest.HorizontalScrollInOverview
 -OverviewGestureHandlerTest.ScrollUpDownWithoutReleasing
 -OverviewGestureHandlerTest.VerticalScrolls
-# TODO: Screenshots. http://crbug.com/698033
--PartialScreenshotControllerTest.BasicMouse
--PartialScreenshotControllerTest.BasicTouch
--PartialScreenshotControllerTest.JustClick
+# TODO: Needs cursor manager support. http://crbug.com/698033
+-PartialScreenshotControllerTest.CursorVisibilityTest
 -PartialScreenshotControllerTest.LargeCursor
--PartialScreenshotControllerTest.PointerEventsWorkWhenPointerOnlyActive
--PartialScreenshotControllerTest.StartSessionWhileMousePressed
--PartialScreenshotControllerTest.TouchMousePointerHoverIgnoredWithPointerEvents
--PartialScreenshotControllerTest.TwoFingerTouch
--PartialScreenshotControllerTest.VisibilityTest
-# TODO: Screenshots. http://crbug.com/698033
--PowerButtonScreenshotControllerTest.PowerButtonPressedFirst_NoScreenshotChord
--PowerButtonScreenshotControllerTest.PowerButtonPressedFirst_ScreenshotChord
--PowerButtonScreenshotControllerTest.ReleaseBeforeAnotherPressed
--PowerButtonScreenshotControllerTest.TabletMode
--PowerButtonScreenshotControllerTest.VolumeDownPressedFirst_NoScreenshotChord
--PowerButtonScreenshotControllerTest.VolumeDownPressedFirst_ScreenshotChord
+# TODO: Needs LockStateController. http://crbug.com/632189.
+-PreferredReservedAcceleratorsTest.AcceleratorsWithFullscreen
+-PreferredReservedAcceleratorsTest.AcceleratorsWithPinned
 -ResizeShadowAndCursorTest.MaximizeRestore
 -ResizeShadowAndCursorTest.Minimize
 -ResizeShadowAndCursorTest.MouseDrag
 -ResizeShadowAndCursorTest.MouseHover
 -ResizeShadowAndCursorTest.Touch
-# TODO: Screenshots. http://crbug.com/698033
--ScreenshotControllerTest.MultipleDisplays
--ScreenshotToolTest.EnablingCaptureRegionCallsDelegateAndDisablesTool
--ScreenshotToolTest.EnablingCaptureScreenCallsDelegateAndDisablesTool
 -ShelfLayoutManagerTest.SwipingUpOnShelfInLaptopModeForFullscreenAppList
 -ShelfLayoutManagerTest.SwipingUpOnShelfInTabletModeForFullscreenAppList
 -ShelfLayoutManagerTest.AutoHide
@@ -160,10 +143,6 @@
 -VirtualKeyboardControllerAutoTest.SuppressedIfExternalKeyboardPresent
 -VirtualKeyboardControllerAutoTest.SuppressedInMaximizedMode
 -VirtualKeyboardControllerTest.RestoreKeyboardDevices
-# TODO: Screenshots. http://crbug.com/698033
--WindowScreenshotControllerTest.KeyboardOperation
--WindowScreenshotControllerTest.MouseOperation
--WindowScreenshotControllerTest.MultiDisplays
 -WindowTreeHostManagerTest.UpdateMouseLocationAfterDisplayChange_PrimaryDisconnected
 -WindowTreeHostManagerTest.UpdateMouseLocationAfterDisplayChange_SwapPrimary
 # TODO: fix this. It might fail because GL_OES_texture_npot is not supported.

testing/buildbot/filters/ash_unittests_mus.filter

-# TODO: Needs partial screenshot support.
--ScreenshotToolTest.EnablingCaptureRegionCallsDelegateAndDisablesTool
--ScreenshotToolTest.EnablingCaptureScreenCallsDelegateAndDisablesTool
-
 # TODO: Fails because DeviceDataManager isn't created and TouchDeviceTransform
 # is not supported by InputDeviceManager. http://crbug.com/734812.
 -TouchCalibratorControllerTest.CustomCalibration