Remove signout screenshot logic

This CL is a part of glanceables prototype cleanup.

Bug: b/270948434
Change-Id: I1f2879d00cd7c91b92c7c89f79d84b2a9025d463
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4329643
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Artsiom Mitrokhin <amitrokhin@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1117264}

ash/BUILD.gn

@@ -600,8 +600,6 @@ component("ash") {
     "glanceables/glanceables_weather_view.h",
     "glanceables/glanceables_welcome_label.cc",
     "glanceables/glanceables_welcome_label.h",
-    "glanceables/signout_screenshot_handler.cc",
-    "glanceables/signout_screenshot_handler.h",
     "glanceables/tasks/glanceables_tasks_client.h",
     "host/ash_window_tree_host.cc",
     "host/ash_window_tree_host.h",
@@ -2960,7 +2958,6 @@ test("ash_unittests") {
     "game_dashboard/game_dashboard_unittest.cc",
     "glanceables/glanceables_unittests.cc",
     "glanceables/glanceables_welcome_label_unittest.cc",
-    "glanceables/signout_screenshot_handler_unittest.cc",
     "host/ash_window_tree_host_platform_unittest.cc",
     "host/ash_window_tree_host_unified_unittest.cc",
     "ime/ime_controller_impl_unittest.cc",

ash/glanceables/glanceables_controller.cc

@@ -122,10 +122,6 @@ void GlanceablesController::RestoreSession() {
   DestroyUi();
 }
 
-bool GlanceablesController::ShouldTakeSignoutScreenshot() const {
-  return delegate_->ShouldTakeSignoutScreenshot();
-}
-
 void GlanceablesController::OnWindowActivated(
     wm::ActivationChangeObserver::ActivationReason reason,
     aura::Window* gained_focus,

ash/glanceables/glanceables_controller.h

@@ -48,9 +48,6 @@ class ASH_EXPORT GlanceablesController : public wm::ActivationChangeObserver,
   // Triggers a session restore.
   void RestoreSession();
 
-  // Returns true if a signout screenshot should be taken for this session.
-  bool ShouldTakeSignoutScreenshot() const;
-
   // wm::ActivationChangeObserver:
   void OnWindowActivated(wm::ActivationChangeObserver::ActivationReason reason,
                          aura::Window* gained_focus,

ash/glanceables/glanceables_delegate.h

@@ -21,12 +21,6 @@ class ASH_EXPORT GlanceablesDelegate {
 
   // Called after the glanceables UI is closed.
   virtual void OnGlanceablesClosed() = 0;
-
-  // Returns true if a signout screenshot should be taken for this session. This
-  // method exists to avoid taking screenshots for account types that don't
-  // support glanceables (e.g. forced app mode) and to avoid screenshots in
-  // tests.
-  virtual bool ShouldTakeSignoutScreenshot() = 0;
 };
 
 }  // namespace ash

ash/glanceables/glanceables_restore_view.cc

@@ -7,27 +7,17 @@
 #include <memory>
 
 #include "ash/glanceables/glanceables_controller.h"
-#include "ash/glanceables/glanceables_util.h"
-#include "ash/public/cpp/image_util.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/pill_button.h"
 #include "base/functional/bind.h"
-#include "services/data_decoder/public/mojom/image_decoder.mojom-shared.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_palette.h"
-#include "ui/gfx/geometry/size.h"
-#include "ui/gfx/image/image_skia.h"
-#include "ui/gfx/image/image_skia_operations.h"
-#include "ui/views/controls/button/button.h"
-#include "ui/views/controls/button/image_button.h"
 #include "ui/views/layout/box_layout.h"
 
 namespace ash {
 namespace {
 
-constexpr gfx::Size kScreenshotTargetSize(300, 200);
-
 void OnButtonPressed() {
   Shell::Get()->glanceables_controller()->RestoreSession();
 }
@@ -39,37 +29,11 @@ GlanceablesRestoreView::GlanceablesRestoreView() {
                        views::BoxLayout::Orientation::kVertical))
       ->set_cross_axis_alignment(views::BoxLayout::CrossAxisAlignment::kStart);
 
-  image_util::DecodeImageFile(
-      base::BindOnce(&GlanceablesRestoreView::OnSignoutScreenshotDecoded,
-                     weak_ptr_factory_.GetWeakPtr()),
-      glanceables_util::GetSignoutScreenshotPath(),
-      data_decoder::mojom::ImageCodec::kPng);
+  AddPillButton();
 }
 
 GlanceablesRestoreView::~GlanceablesRestoreView() = default;
 
-void GlanceablesRestoreView::OnSignoutScreenshotDecoded(
-    const gfx::ImageSkia& image) {
-  if (image.isNull()) {
-    // There is no image from previous shutdown or sign-out.
-    AddPillButton();
-    return;
-  }
-  AddImageButton(image);
-}
-
-void GlanceablesRestoreView::AddImageButton(const gfx::ImageSkia& image) {
-  image_button_ = AddChildView(std::make_unique<views::ImageButton>(
-      base::BindRepeating(&OnButtonPressed)));
-  image_button_->SetAccessibleName(
-      l10n_util::GetStringUTF16(IDS_GLANCEABLES_RESTORE_SESSION));
-  image_button_->SetImage(
-      views::Button::STATE_NORMAL,
-      gfx::ImageSkiaOperations::CreateResizedImage(
-          image, skia::ImageOperations::ResizeMethod::RESIZE_BETTER,
-          kScreenshotTargetSize));
-}
-
 void GlanceablesRestoreView::AddPillButton() {
   pill_button_ = AddChildView(std::make_unique<PillButton>(
       base::BindRepeating(&OnButtonPressed),

ash/glanceables/glanceables_restore_view.h

@@ -6,17 +6,8 @@
 #define ASH_GLANCEABLES_GLANCEABLES_RESTORE_VIEW_H_
 
 #include "ash/ash_export.h"
-#include "base/memory/weak_ptr.h"
 #include "ui/views/view.h"
 
-namespace gfx {
-class ImageSkia;
-}  // namespace gfx
-
-namespace views {
-class ImageButton;
-}  // namespace views
-
 namespace ash {
 
 class PillButton;
@@ -33,17 +24,10 @@ class ASH_EXPORT GlanceablesRestoreView : public views::View {
  private:
   friend class GlanceablesTest;
 
-  void OnSignoutScreenshotDecoded(const gfx::ImageSkia& image);
-
-  // Adds an image button with a screenshot image.
-  void AddImageButton(const gfx::ImageSkia& image);
-
   // Adds a "Restore" pill button.
   void AddPillButton();
 
-  views::ImageButton* image_button_ = nullptr;
   PillButton* pill_button_ = nullptr;
-  base::WeakPtrFactory<GlanceablesRestoreView> weak_ptr_factory_{this};
 };
 
 }  // namespace ash

ash/glanceables/glanceables_unittests.cc

@@ -17,10 +17,8 @@
 #include "ash/glanceables/glanceables_view.h"
 #include "ash/glanceables/glanceables_weather_view.h"
 #include "ash/glanceables/glanceables_welcome_label.h"
-#include "ash/glanceables/signout_screenshot_handler.h"
 #include "ash/glanceables/test_glanceables_delegate.h"
 #include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
-#include "ash/public/cpp/test/in_process_image_decoder.h"
 #include "ash/public/cpp/test/test_system_tray_client.h"
 #include "ash/shell.h"
 #include "ash/style/pill_button.h"
@@ -29,23 +27,17 @@
 #include "ash/system/time/calendar_utils.h"
 #include "ash/system/unified/unified_system_tray.h"
 #include "ash/test/ash_test_base.h"
-#include "base/base_paths.h"
 #include "base/check.h"
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
 #include "base/test/scoped_feature_list.h"
-#include "base/test/scoped_path_override.h"
 #include "base/time/time.h"
 #include "base/time/time_override.h"
 #include "chromeos/ash/components/settings/scoped_timezone_settings.h"
 #include "google_apis/calendar/calendar_api_response_types.h"
 #include "google_apis/common/api_error_codes.h"
-#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/compositor/layer.h"
 #include "ui/events/keycodes/keyboard_codes_posix.h"
 #include "ui/events/test/test_event.h"
-#include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_unittest_util.h"
 #include "ui/views/controls/image_view.h"
@@ -55,19 +47,6 @@
 namespace ash {
 namespace {
 
-// A SignoutScreenshotHandler that skips taking the screenshot and invokes its
-// done callback immediately.
-class TestSignoutScreenshotHandler : public SignoutScreenshotHandler {
- public:
-  // SignoutScreenshotHandler:
-  void TakeScreenshot(base::OnceClosure done_callback) override {
-    ++take_screenshot_count_;
-    std::move(done_callback).Run();
-  }
-
-  int take_screenshot_count_ = 0;
-};
-
 AmbientWeatherModel* GetWeatherModel() {
   return Shell::Get()->ambient_controller()->GetAmbientWeatherModel();
 }
@@ -181,10 +160,6 @@ class GlanceablesTest : public AshTestBase {
     return controller_->view_->restore_view_;
   }
 
-  views::ImageButton* GetRestoreViewImageButton() {
-    return GetRestoreView()->image_button_;
-  }
-
   PillButton* GetRestoreViewPillButton() {
     return GetRestoreView()->pill_button_;
   }
@@ -345,49 +320,11 @@ TEST_F(GlanceablesTest, UpNextEventItemViewRendersCorrectlyWithoutEventTitle) {
   EXPECT_EQ(view.event_title_label_for_test()->GetText(), u"(No title)");
 }
 
-TEST_F(GlanceablesTest, RestoreViewRendersScreenshot) {
-  InProcessImageDecoder data_decoder;
-  const SkColor expected_color = SK_ColorYELLOW;
-
-  // Override home directory.
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::ScopedPathOverride home_dir_override(base::DIR_HOME,
-                                             temp_dir.GetPath());
-
-  // Simulate that shutdown screenshot is there.
-  SkBitmap bitmap;
-  bitmap.allocN32Pixels(400, 300);
-  bitmap.eraseColor(expected_color);
-  std::vector<unsigned char> png_data;
-  gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &png_data);
-  ASSERT_TRUE(base::WriteFile(
-      temp_dir.GetPath().AppendASCII("signout_screenshot.png"), png_data));
-
-  controller_->CreateUi();
-  GlanceablesRestoreView* restore_view = GetRestoreView();
-  ASSERT_TRUE(restore_view);
-
-  // Wait for GlanceablesRestoreView `image_util::DecodeImageFile` callback.
-  base::RunLoop().RunUntilIdle();
-  views::ImageButton* image_button = GetRestoreViewImageButton();
-  ASSERT_TRUE(image_button);
-  ASSERT_FALSE(GetRestoreViewPillButton());
-  gfx::ImageSkia image = image_button->GetImage(views::Button::STATE_NORMAL);
-  EXPECT_FALSE(image.isNull());
-  EXPECT_GT(image.width(), 0);
-  EXPECT_GT(image.height(), 0);
-  EXPECT_EQ(image.bitmap()->getColor(150, 100), expected_color);
-}
-
 TEST_F(GlanceablesTest, ClickOnSessionRestore) {
   controller_->CreateUi();
   GlanceablesRestoreView* restore_view = GetRestoreView();
   ASSERT_TRUE(restore_view);
 
-  // Wait for GlanceablesRestoreView `image_util::DecodeImageFile` callback.
-  base::RunLoop().RunUntilIdle();
-
   PillButton* restore_button = GetRestoreViewPillButton();
   ASSERT_TRUE(restore_button);
   ASSERT_EQ(0, GetTestDelegate()->restore_session_count());
@@ -422,23 +359,4 @@ TEST_F(GlanceablesTest, DismissesOnlyOnAppWindowOpen) {
   EXPECT_FALSE(controller_->IsShowing());
 }
 
-TEST_F(GlanceablesTest, RequestRestartForUpdateTakesScreenshot) {
-  GetTestDelegate()->set_should_take_signout_screenshot(true);
-
-  auto* session_controller = Shell::Get()->session_controller();
-  auto screenshot_handler = std::make_unique<TestSignoutScreenshotHandler>();
-  auto* screenshot_handler_ptr = screenshot_handler.get();
-  session_controller->SetSignoutScreenshotHandlerForTest(
-      std::move(screenshot_handler));
-
-  session_controller->RequestRestartForUpdate();
-
-  // Screenshot was taken.
-  EXPECT_EQ(1, screenshot_handler_ptr->take_screenshot_count_);
-
-  // Restart was requested.
-  EXPECT_EQ(1,
-            GetSessionControllerClient()->request_restart_for_update_count());
-}
-
 }  // namespace ash

ash/glanceables/glanceables_util.cc

@@ -6,7 +6,12 @@
 
 #include "base/base_paths.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/functional/bind.h"
+#include "base/location.h"
 #include "base/path_service.h"
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool.h"
 
 namespace ash::glanceables_util {
 
@@ -16,4 +21,11 @@ base::FilePath GetSignoutScreenshotPath() {
   return home_dir.AppendASCII("signout_screenshot.png");
 }
 
+void DeleteScreenshot() {
+  base::ThreadPool::PostTask(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::LOWEST},
+      base::BindOnce(base::IgnoreResult(&base::DeleteFile),
+                     glanceables_util::GetSignoutScreenshotPath()));
+}
+
 }  // namespace ash::glanceables_util

ash/glanceables/glanceables_util.h

@@ -5,6 +5,8 @@
 #ifndef ASH_GLANCEABLES_GLANCEABLES_UTIL_H_
 #define ASH_GLANCEABLES_GLANCEABLES_UTIL_H_
 
+#include "ash/ash_export.h"
+
 namespace base {
 class FilePath;
 }  // namespace base
@@ -13,7 +15,10 @@ namespace ash::glanceables_util {
 
 // Returns the path to the signout screenshot, for example
 // /home/chronos/u-<hash>/signout_screenshot.png
-base::FilePath GetSignoutScreenshotPath();
+ASH_EXPORT base::FilePath GetSignoutScreenshotPath();
+
+// Removes signout screenshot located at `GetSignoutScreenshotPath()`.
+ASH_EXPORT void DeleteScreenshot();
 
 }  // namespace ash::glanceables_util
 

ash/glanceables/signout_screenshot_handler.cc

@@ -1,140 +0,0 @@
-// Copyright 2022 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ash/glanceables/signout_screenshot_handler.h"
-
-#include "ash/glanceables/glanceables_util.h"
-#include "ash/shell.h"
-#include "ash/wm/desks/desks_util.h"
-#include "base/check.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/functional/bind.h"
-#include "base/location.h"
-#include "base/logging.h"
-#include "base/memory/ref_counted_memory.h"
-#include "base/memory/scoped_refptr.h"
-#include "base/task/task_traits.h"
-#include "base/task/thread_pool.h"
-#include "ui/gfx/image/image.h"
-#include "ui/gfx/image/image_skia_operations.h"
-#include "ui/snapshot/snapshot.h"
-
-namespace ash {
-namespace {
-
-// Writes `png_data` to disk at `file_path`.
-void WriteScreenshotOnBlockingPool(
-    const base::FilePath& file_path,
-    scoped_refptr<base::RefCountedMemory> png_data) {
-  auto raw_data = base::make_span(png_data->data(), png_data->size());
-  if (!base::WriteFile(file_path, raw_data))
-    LOG(ERROR) << "Failed to write screenshot " << file_path.MaybeAsASCII();
-}
-
-// Deletes the file at `file_path`.
-void DeleteScreenshotOnBlockingPool(const base::FilePath& file_path) {
-  base::DeleteFile(file_path);
-}
-
-scoped_refptr<base::RefCountedMemory> EncodeImageAsPngOnThreadPool(
-    const gfx::Image& image) {
-  DCHECK(!image.IsEmpty());
-  return image.As1xPNGBytes();
-}
-
-}  // namespace
-
-SignoutScreenshotHandler::SignoutScreenshotHandler() = default;
-
-SignoutScreenshotHandler::~SignoutScreenshotHandler() = default;
-
-void SignoutScreenshotHandler::TakeScreenshot(base::OnceClosure done_callback) {
-  done_callback_ = std::move(done_callback);
-
-  // TODO(crbug.com/1353119): Support multiple displays. For now, use the most
-  // recently active display.
-  aura::Window* root = Shell::GetRootWindowForNewWindows();
-  DCHECK(root);
-  // The screenshot should only contain windows, not UI like the shelf. Take a
-  // screenshot of the active desk container.
-  aura::Window* active_desk = desks_util::GetActiveDeskContainerForRoot(root);
-  DCHECK(active_desk);
-  if (active_desk->children().empty()) {
-    // If there are no windows in the desk container, taking the screenshot will
-    // fail. Delete any existing screenshot so we know on startup that there are
-    // no windows to preview.
-    DeleteScreenshot();
-    return;
-  }
-  gfx::Size source_size = active_desk->bounds().size();
-  // Capture the screenshot at a smaller size than the desk. This speeds up PNG
-  // encoding and writing to disk.
-  screenshot_size_ =
-      gfx::Size(source_size.width() / 2, source_size.height() / 2);
-  // Snapshot scaling uses skia::ImageOperations::RESIZE_GOOD which should be
-  // fast. See SnapshotAsync::ScaleCopyOutputResult().
-  ui::GrabWindowSnapshotAndScaleAsync(
-      active_desk,
-      /*source_rect=*/gfx::Rect(gfx::Point(), source_size), screenshot_size_,
-      base::BindOnce(&SignoutScreenshotHandler::OnScreenshotTaken,
-                     weak_factory_.GetWeakPtr()));
-}
-
-void SignoutScreenshotHandler::OnScreenshotTaken(gfx::Image image) {
-  if (image.IsEmpty()) {
-    // If the screenshot failed, delete any existing screenshot so we don't show
-    // a stale image on startup.
-    DeleteScreenshot();
-    return;
-  }
-  base::ThreadPool::PostTaskAndReplyWithResult(
-      FROM_HERE, {base::TaskPriority::USER_BLOCKING},
-      base::BindOnce(&EncodeImageAsPngOnThreadPool, std::move(image)),
-      base::BindOnce(&SignoutScreenshotHandler::SaveScreenshot,
-                     weak_factory_.GetWeakPtr()));
-}
-
-void SignoutScreenshotHandler::SaveScreenshot(
-    scoped_refptr<base::RefCountedMemory> png_data) {
-  if (!png_data) {
-    // If PNG encoding failed failed, delete any existing screenshot so we don't
-    // show a stale image on startup.
-    DeleteScreenshot();
-    return;
-  }
-  base::FilePath file_path = GetScreenshotPath();
-  // Use priority USER_BLOCKING since the user is waiting for logout/shutdown.
-  base::ThreadPool::PostTaskAndReply(
-      FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
-      base::BindOnce(&WriteScreenshotOnBlockingPool, file_path, png_data),
-      base::BindOnce(&SignoutScreenshotHandler::OnScreenshotSaved,
-                     weak_factory_.GetWeakPtr()));
-}
-
-void SignoutScreenshotHandler::OnScreenshotSaved() {
-  std::move(done_callback_).Run();
-}
-
-void SignoutScreenshotHandler::DeleteScreenshot() {
-  base::FilePath file_path = GetScreenshotPath();
-  // Use priority USER_BLOCKING since the user is waiting for logout/shutdown.
-  base::ThreadPool::PostTaskAndReply(
-      FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
-      base::BindOnce(&DeleteScreenshotOnBlockingPool, file_path),
-      base::BindOnce(&SignoutScreenshotHandler::OnScreenshotDeleted,
-                     weak_factory_.GetWeakPtr()));
-}
-
-void SignoutScreenshotHandler::OnScreenshotDeleted() {
-  std::move(done_callback_).Run();
-}
-
-base::FilePath SignoutScreenshotHandler::GetScreenshotPath() const {
-  if (!screenshot_path_for_test_.empty())
-    return screenshot_path_for_test_;
-  return glanceables_util::GetSignoutScreenshotPath();
-}
-
-}  // namespace ash

ash/glanceables/signout_screenshot_handler.h

@@ -1,75 +0,0 @@
-// Copyright 2022 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef ASH_GLANCEABLES_SIGNOUT_SCREENSHOT_HANDLER_H_
-#define ASH_GLANCEABLES_SIGNOUT_SCREENSHOT_HANDLER_H_
-
-#include "ash/ash_export.h"
-#include "base/files/file_path.h"
-#include "base/functional/callback.h"
-#include "base/memory/ref_counted_memory.h"
-#include "base/memory/weak_ptr.h"
-#include "ui/gfx/geometry/size.h"
-
-namespace gfx {
-class Image;
-}  // namespace gfx
-
-namespace ash {
-
-// Handles taking a screenshot of the open windows on signout or shutdown.
-// Encodes the screenshot as PNG and writes it to the user data directory.
-// Invokes a callback when done so that signout can proceed. The screenshot is
-// displayed by the glanceables screen on the next login.
-class ASH_EXPORT SignoutScreenshotHandler {
- public:
-  SignoutScreenshotHandler();
-  SignoutScreenshotHandler(const SignoutScreenshotHandler&) = delete;
-  SignoutScreenshotHandler& operator=(const SignoutScreenshotHandler&) = delete;
-  virtual ~SignoutScreenshotHandler();
-
-  // Takes a screenshot of the windows on the active desk and writes it to disk.
-  // Invokes `done_callback` when done. Virtual for testing.
-  virtual void TakeScreenshot(base::OnceClosure done_callback);
-
-  void set_screenshot_path_for_test(const base::FilePath& path) {
-    screenshot_path_for_test_ = path;
-  }
-
-  gfx::Size screenshot_size_for_test() { return screenshot_size_; }
-
- private:
-  // Callback invoked when the screenshot is taken. gfx::Image is cheap to pass
-  // by value.
-  void OnScreenshotTaken(gfx::Image image);
-
-  // Saves the screenshot to disk.
-  void SaveScreenshot(scoped_refptr<base::RefCountedMemory> png_data);
-
-  // Callback invoked after the screenshot is saved.
-  void OnScreenshotSaved();
-
-  // Deletes an existing screenshot from disk.
-  void DeleteScreenshot();
-
-  // Callback invoked after the screenshot is deleted.
-  void OnScreenshotDeleted();
-
-  // Returns the path to the screenshot file.
-  base::FilePath GetScreenshotPath() const;
-
-  // Invoked when the screenshot is done.
-  base::OnceClosure done_callback_;
-
-  // Size of the output screenshot.
-  gfx::Size screenshot_size_;
-
-  base::FilePath screenshot_path_for_test_;
-
-  base::WeakPtrFactory<SignoutScreenshotHandler> weak_factory_{this};
-};
-
-}  // namespace ash
-
-#endif  // ASH_GLANCEABLES_SIGNOUT_SCREENSHOT_HANDLER_H_

ash/glanceables/signout_screenshot_handler_unittest.cc

@@ -1,80 +0,0 @@
-// Copyright 2022 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ash/glanceables/signout_screenshot_handler.h"
-
-#include <stdint.h>
-
-#include <memory>
-
-#include "ash/glanceables/glanceables_util.h"
-#include "ash/shell.h"
-#include "ash/test/ash_test_base.h"
-#include "ash/wm/desks/desks_util.h"
-#include "base/check.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
-#include "base/run_loop.h"
-#include "base/threading/thread_restrictions.h"
-#include "base/time/time.h"
-#include "ui/aura/window.h"
-
-namespace ash {
-namespace {
-
-class SignoutScreenshotHandlerTest : public AshTestBase {
- public:
-  SignoutScreenshotHandlerTest() {
-    CHECK(temp_dir_.CreateUniqueTempDir());
-    screenshot_path_ = temp_dir_.GetPath().AppendASCII("screenshot.png");
-  }
-
-  base::ScopedAllowBlockingForTesting allow_blocking_;
-  base::ScopedTempDir temp_dir_;
-  base::FilePath screenshot_path_;
-};
-
-// Tests that a screenshot is taken when there are windows open.
-TEST_F(SignoutScreenshotHandlerTest, TakeScreenshotWithWindowOpen) {
-  std::unique_ptr<aura::Window> window = CreateTestWindow();
-
-  SignoutScreenshotHandler handler;
-  handler.set_screenshot_path_for_test(screenshot_path_);
-  base::RunLoop run_loop;
-  handler.TakeScreenshot(run_loop.QuitClosure());
-  run_loop.Run();
-
-  // Screenshot is half the size of the desk container in each dimension.
-  gfx::Size screenshot_size = handler.screenshot_size_for_test();
-  aura::Window* active_desk =
-      desks_util::GetActiveDeskContainerForRoot(Shell::GetPrimaryRootWindow());
-  EXPECT_EQ(screenshot_size.width(), active_desk->bounds().width() / 2);
-  EXPECT_EQ(screenshot_size.height(), active_desk->bounds().height() / 2);
-
-  // Screenshot was taken and is not empty.
-  EXPECT_TRUE(base::PathExists(screenshot_path_));
-  int64_t file_size = 0;
-  ASSERT_TRUE(base::GetFileSize(screenshot_path_, &file_size));
-  EXPECT_GT(file_size, 0);
-}
-
-// Tests that no screenshot is taken when no windows are open and the existing
-// screenshot is deleted.
-TEST_F(SignoutScreenshotHandlerTest, TakeScreenshotWithNoWindows) {
-  // Create an empty file to simulate an old screenshot.
-  ASSERT_TRUE(base::WriteFile(screenshot_path_, ""));
-
-  SignoutScreenshotHandler handler;
-  handler.set_screenshot_path_for_test(screenshot_path_);
-  base::RunLoop run_loop;
-  handler.TakeScreenshot(run_loop.QuitClosure());
-  run_loop.Run();
-
-  // Existing screenshot was deleted.
-  EXPECT_FALSE(base::PathExists(screenshot_path_));
-}
-
-}  // namespace
-}  // namespace ash

ash/glanceables/test_glanceables_delegate.cc

@@ -18,8 +18,4 @@ void TestGlanceablesDelegate::OnGlanceablesClosed() {
   ++on_glanceables_closed_count_;
 }
 
-bool TestGlanceablesDelegate::ShouldTakeSignoutScreenshot() {
-  return should_take_signout_screenshot_;
-}
-
 }  // namespace ash

ash/glanceables/test_glanceables_delegate.h

@@ -20,7 +20,6 @@ class TestGlanceablesDelegate : public GlanceablesDelegate {
   // GlanceablesDelegate:
   void RestoreSession() override;
   void OnGlanceablesClosed() override;
-  bool ShouldTakeSignoutScreenshot() override;
 
   int restore_session_count() { return restore_session_count_; }
   int on_glanceables_closed_count() { return on_glanceables_closed_count_; }

ash/session/session_controller_impl.cc

@@ -8,10 +8,7 @@
 #include <string>
 #include <utility>
 
-#include "ash/constants/ash_features.h"
 #include "ash/constants/ash_pref_names.h"
-#include "ash/glanceables/glanceables_controller.h"
-#include "ash/glanceables/signout_screenshot_handler.h"
 #include "ash/metrics/user_metrics_recorder.h"
 #include "ash/public/cpp/session/scoped_screen_lock_blocker.h"
 #include "ash/public/cpp/session/session_activation_observer.h"
@@ -27,10 +24,7 @@
 #include "ash/system/privacy/screen_switch_check_controller.h"
 #include "ash/wm/lock_state_controller.h"
 #include "ash/wm/mru_window_tracker.h"
-#include "ash/wm/window_util.h"
-#include "base/command_line.h"
 #include "base/functional/bind.h"
-#include "base/functional/callback_helpers.h"
 #include "base/logging.h"
 #include "base/ranges/algorithm.h"
 #include "components/account_id/account_id.h"
@@ -64,10 +58,7 @@ class SessionControllerImpl::ScopedScreenLockBlockerImpl
 };
 
 SessionControllerImpl::SessionControllerImpl()
-    : fullscreen_controller_(std::make_unique<FullscreenController>(this)) {
-  if (features::AreGlanceablesEnabled())
-    signout_screenshot_handler_ = std::make_unique<SignoutScreenshotHandler>();
-}
+    : fullscreen_controller_(std::make_unique<FullscreenController>(this)) {}
 
 SessionControllerImpl::~SessionControllerImpl() {
   // Abort pending start lock request.
@@ -248,37 +239,15 @@ void SessionControllerImpl::HideLockScreen() {
 }
 
 void SessionControllerImpl::RequestSignOut() {
-  if (features::AreGlanceablesEnabled() &&
-      Shell::Get()->glanceables_controller()->ShouldTakeSignoutScreenshot()) {
-    DCHECK(IsActiveUserSessionStarted());
-    signout_screenshot_handler_->TakeScreenshot(
-        base::BindOnce(&SessionControllerImpl::ProceedWithSignOut,
-                       weak_ptr_factory_.GetWeakPtr()));
-    return;
-  }
-  ProceedWithSignOut();
-}
-
-void SessionControllerImpl::ProceedWithSignOut() {
-  if (client_)
+  if (client_) {
     client_->RequestSignOut();
+  }
 }
 
 void SessionControllerImpl::RequestRestartForUpdate() {
-  if (features::AreGlanceablesEnabled() &&
-      Shell::Get()->glanceables_controller()->ShouldTakeSignoutScreenshot()) {
-    DCHECK(IsActiveUserSessionStarted());
-    signout_screenshot_handler_->TakeScreenshot(
-        base::BindOnce(&SessionControllerImpl::ProceedWithRestartToUpdate,
-                       weak_ptr_factory_.GetWeakPtr()));
-    return;
-  }
-  ProceedWithRestartToUpdate();
-}
-
-void SessionControllerImpl::ProceedWithRestartToUpdate() {
-  if (client_)
+  if (client_) {
     client_->RequestRestartForUpdate();
+  }
 }
 
 void SessionControllerImpl::AttemptRestartChrome() {
@@ -531,11 +500,6 @@ void SessionControllerImpl::ClearUserSessionsForTest() {
   primary_session_id_ = 0u;
 }
 
-void SessionControllerImpl::SetSignoutScreenshotHandlerForTest(
-    std::unique_ptr<SignoutScreenshotHandler> handler) {
-  signout_screenshot_handler_ = std::move(handler);
-}
-
 void SessionControllerImpl::SetIsDemoSession() {
   if (is_demo_session_)
     return;

ash/session/session_controller_impl.h

@@ -31,7 +31,6 @@ class FullscreenController;
 class ScopedScreenLockBlocker;
 class SessionControllerClient;
 class SessionObserver;
-class SignoutScreenshotHandler;
 class TestSessionControllerClient;
 
 // Implements mojom::SessionController to cache session related info such as
@@ -228,8 +227,6 @@ class ASH_EXPORT SessionControllerImpl : public SessionController {
 
   // Test helpers.
   void ClearUserSessionsForTest();
-  void SetSignoutScreenshotHandlerForTest(
-      std::unique_ptr<SignoutScreenshotHandler> handler);
 
  private:
   friend class TestSessionControllerClient;
@@ -276,13 +273,6 @@ class ASH_EXPORT SessionControllerImpl : public SessionController {
   // window, tries to activate one.
   void EnsureActiveWindowAfterUnblockingUserSession();
 
-  // Proceeds with signout after the (optional) signout screenshot is taken.
-  void ProceedWithSignOut();
-
-  // Proceeds with restart to update after the (optional) signout screenshot is
-  // taken.
-  void ProceedWithRestartToUpdate();
-
   // Called when an object of `ScopedScreenLockBlockerImpl` is destroyed.
   void RemoveScopedScreenLockBlocker();
 
@@ -345,9 +335,6 @@ class ASH_EXPORT SessionControllerImpl : public SessionController {
 
   std::unique_ptr<FullscreenController> fullscreen_controller_;
 
-  // May be null if glanceables are not enabled.
-  std::unique_ptr<SignoutScreenshotHandler> signout_screenshot_handler_;
-
   int scoped_screen_lock_blocker_count_ = 0;
 
   SEQUENCE_CHECKER(sequence_checker_);

chrome/browser/ash/login/session/user_session_initializer.cc

@@ -6,6 +6,7 @@
 
 #include "ash/constants/ash_features.h"
 #include "ash/constants/ash_pref_names.h"
+#include "ash/glanceables/glanceables_util.h"
 #include "base/feature_list.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
@@ -268,6 +269,9 @@ void UserSessionInitializer::OnUserSessionStarted(bool is_primary_user) {
       ChromeGlanceablesDelegate::Get()->OnPrimaryUserSessionStarted(profile);
     }
 
+    // TODO(b/270948434): Temporary cleanup logic.
+    glanceables_util::DeleteScreenshot();
+
     // Ensure that the `GlanceablesKeyedService` for `primary_profile_` is
     // created.
     GlanceablesKeyedServiceFactory::GetInstance()->GetService(primary_profile_);

chrome/browser/ui/ash/glanceables/chrome_glanceables_delegate.cc

@@ -11,7 +11,6 @@
 #include "chrome/browser/ash/app_restore/full_restore_service.h"
 #include "chrome/browser/ash/app_restore/full_restore_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/app_restore/full_restore_save_handler.h"
@@ -82,23 +81,6 @@ void ChromeGlanceablesDelegate::OnGlanceablesClosed() {
   }
 }
 
-bool ChromeGlanceablesDelegate::ShouldTakeSignoutScreenshot() {
-  // Don't take a screenshot if the user hasn't signed in. This can happen
-  // in the emulator, where "Shut down" on the login screen runs the signout
-  // code path.
-  if (!primary_profile_)
-    return false;
-
-  // Only take a screenshot if the user is on the primary profile desktop.
-  if (ProfileManager::GetActiveUserProfile() != primary_profile_)
-    return false;
-
-  // Only take a screenshot if this session type would use it on the next login.
-  // This also avoids taking screenshots in browser tests (because they pass
-  // --no-first-run and skip glanceables).
-  return ShouldShowOnLogin();
-}
-
 void ChromeGlanceablesDelegate::OnRefreshTokenUpdatedForAccount(
     const CoreAccountInfo& account_info) {
   const CoreAccountInfo& primary_account_info =

chrome/browser/ui/ash/glanceables/chrome_glanceables_delegate.h

@@ -34,7 +34,6 @@ class ChromeGlanceablesDelegate : public ash::GlanceablesDelegate,
   // ash::GlanceablesDelegate:
   void RestoreSession() override;
   void OnGlanceablesClosed() override;
-  bool ShouldTakeSignoutScreenshot() override;
 
   // signin::IdentityManager::Observer:
   void OnRefreshTokenUpdatedForAccount(

chrome/browser/ui/ash/glanceables/glanceables_browsertest.cc

@@ -2,11 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+#include <vector>
+
 #include "ash/constants/ash_features.h"
 #include "ash/constants/ash_switches.h"
 #include "ash/glanceables/glanceables_controller.h"
+#include "ash/glanceables/glanceables_util.h"
 #include "ash/shell.h"
+#include "base/base_paths.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/task/thread_pool/thread_pool_instance.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_path_override.h"
+#include "base/threading/thread_restrictions.h"
 #include "chrome/browser/ash/profiles/profile_helper.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
@@ -25,6 +35,9 @@
 #include "components/user_manager/user_manager.h"
 #include "content/public/test/browser_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/codec/png_codec.h"
 
 namespace {
 
@@ -106,3 +119,51 @@ IN_PROC_BROWSER_TEST_F(GlanceablesBrowserTest, ShowsAndHide) {
   // Glanceables should close because a window opened.
   EXPECT_FALSE(glanceables_controller()->IsShowing());
 }
+
+class GlanceablesScreenshotDeletionBrowserTest
+    : public GlanceablesBrowserTest,
+      public testing::WithParamInterface<bool> {
+ public:
+  bool SetUpUserDataDirectory() override {
+    if (!temp_dir_.CreateUniqueTempDir()) {
+      return false;
+    }
+    home_dir_override_ = std::make_unique<base::ScopedPathOverride>(
+        base::DIR_HOME, temp_dir_.GetPath());
+
+    if (!GetParam()) {
+      return true;
+    }
+
+    SkBitmap bitmap;
+    bitmap.allocN32Pixels(400, 300);
+    bitmap.eraseColor(SK_ColorYELLOW);
+    std::vector<unsigned char> png_data;
+    gfx::PNGCodec::EncodeBGRASkBitmap(bitmap, true, &png_data);
+    return base::WriteFile(ash::glanceables_util::GetSignoutScreenshotPath(),
+                           png_data);
+  }
+
+ private:
+  base::ScopedTempDir temp_dir_;
+  std::unique_ptr<base::ScopedPathOverride> home_dir_override_;
+};
+
+IN_PROC_BROWSER_TEST_P(GlanceablesScreenshotDeletionBrowserTest,
+                       DeletesScreenshotAfterLogin) {
+  base::ScopedAllowBlockingForTesting allow_blocking;
+
+  CreateAndStartUserSession();
+  signin::MakePrimaryAccountAvailable(identity_manager(), kTestUserName,
+                                      signin::ConsentLevel::kSignin);
+
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(
+      base::PathExists(ash::glanceables_util::GetSignoutScreenshotPath()));
+}
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         GlanceablesScreenshotDeletionBrowserTest,
+                         testing::Bool());