Remove file handling from ScreenshotGrabber.

This is the first patch in a series to make screenshots work in
mash. This first step is simplifying ui::ScreenshotGrabber into a
component that can be used without the filesystem. This moves file
writing to the chrome layer, switches to callbacks, and starts
separating ChromeScreenshotGrabber into the part that initiates a
screenshot (will be replaced with an ash side delegate that communicates
over mojo) and the part that accepts the data and writes it to the
filesystem (which will continue to exist and stay in chrome).

Change-Id: Ic2370e0d4426f51ffd09af8920304b98b8ebde0e
Bug: 706246
Reviewed-on: https://chromium-review.googlesource.com/894882Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533848}

chrome/browser/policy/policy_browsertest.cc

@@ -217,6 +217,7 @@
 #include "chrome/browser/chromeos/system/timezone_resolver_manager.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
+#include "chrome/browser/ui/ash/chrome_screenshot_grabber_test_observer.h"
 #include "chromeos/audio/cras_audio_handler.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
@@ -699,10 +700,11 @@ class PolicyTest : public InProcessBrowserTest {
   }
 
 #if defined(OS_CHROMEOS)
-  class QuitMessageLoopAfterScreenshot : public ui::ScreenshotGrabberObserver {
+  class QuitMessageLoopAfterScreenshot
+      : public ChromeScreenshotGrabberTestObserver {
    public:
     void OnScreenshotCompleted(
-        ScreenshotGrabberObserver::Result screenshot_result,
+        ui::ScreenshotResult screenshot_result,
         const base::FilePath& screenshot_path) override {
       BrowserThread::PostTaskAndReply(BrowserThread::IO, FROM_HERE,
                                       base::Bind(base::DoNothing),
@@ -716,11 +718,11 @@ class PolicyTest : public InProcessBrowserTest {
     // ScreenshotGrabber doesn't own this observer, so the observer's lifetime
     // is tied to the test instead.
     ChromeScreenshotGrabber* grabber = ChromeScreenshotGrabber::Get();
-    grabber->screenshot_grabber()->AddObserver(&observer_);
+    grabber->test_observer_ = &observer_;
     SetScreenshotPolicy(enabled);
     grabber->HandleTakeScreenshotForAllRootWindows();
     content::RunMessageLoop();
-    grabber->screenshot_grabber()->RemoveObserver(&observer_);
+    grabber->test_observer_ = nullptr;
   }
 #endif  // defined(OS_CHROMEOS)
 

chrome/browser/ui/BUILD.gn

@@ -1803,6 +1803,7 @@ split_static_library("ui") {
       "ash/chrome_new_window_client.h",
       "ash/chrome_screenshot_grabber.cc",
       "ash/chrome_screenshot_grabber.h",
+      "ash/chrome_screenshot_grabber_test_observer.h",
       "ash/chrome_shell_content_state.cc",
       "ash/chrome_shell_content_state.h",
       "ash/chrome_shell_content_state_chromeos.cc",

chrome/browser/ui/ash/chrome_screenshot_grabber.h

@@ -10,6 +10,7 @@
 #include "ash/screenshot_delegate.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "build/build_config.h"
 #include "components/drive/chromeos/file_system_interface.h"
 #include "components/drive/drive.pb.h"
@@ -23,10 +24,28 @@ namespace ash {
 class ChromeScreenshotGrabberTest;
 }  // namespace ash
 
-class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
-                                public ui::ScreenshotGrabberDelegate,
-                                public ui::ScreenshotGrabberObserver {
+namespace policy {
+class PolicyTest;
+}  // namespace policy
+
+class ChromeScreenshotGrabberBrowserTest;
+class ChromeScreenshotGrabberTestObserver;
+
+// Result of asynchronous file operations.
+enum class ScreenshotFileResult {
+  SUCCESS,
+  CHECK_DIR_FAILED,
+  CREATE_DIR_FAILED,
+  CREATE_FAILED
+};
+
+class ChromeScreenshotGrabber : public ash::ScreenshotDelegate {
  public:
+  // Callback called with the |result| of trying to create a local writable
+  // |path| for the possibly remote path.
+  using FileCallback = base::Callback<void(ScreenshotFileResult result,
+                                           const base::FilePath& path)>;
+
   ChromeScreenshotGrabber();
   ~ChromeScreenshotGrabber() override;
 
@@ -43,17 +62,28 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
   void HandleTakeWindowScreenshot(aura::Window* window) override;
   bool CanTakeScreenshot() override;
 
-  // ui::ScreenshotGrabberDelegate:
-  void PrepareFileAndRunOnBlockingPool(
-      const base::FilePath& path,
-      const FileCallback& callback_on_blocking_pool) override;
-
-  // ui::ScreenshotGrabberObserver:
-  void OnScreenshotCompleted(ui::ScreenshotGrabberObserver::Result result,
-                             const base::FilePath& screenshot_path) override;
+  // TODO(erg): This is the endpoint for a new interface which will be added in
+  // a future patch. It is intended to be both mojo proxyable, and usable as a
+  // callback from ScreenshotGrabber.
+  void OnTookScreenshot(const base::Time& screenshot_time,
+                        const base::Optional<int>& display_num,
+                        ui::ScreenshotResult result,
+                        scoped_refptr<base::RefCountedMemory> png_data);
 
  private:
   friend class ash::ChromeScreenshotGrabberTest;
+  friend class ChromeScreenshotGrabberBrowserTest;
+  friend class policy::PolicyTest;
+
+  // Prepares a writable file for |path|. If |path| is a non-local path (i.e.
+  // Google drive) and it is supported this will create a local cached copy of
+  // the remote file and call the callback with the local path.
+  void PrepareFileAndRunOnBlockingPool(const base::FilePath& path,
+                                       const FileCallback& callback);
+
+  // Called once all file writing is completed, or on error.
+  void OnScreenshotCompleted(ui::ScreenshotResult result,
+                             const base::FilePath& screenshot_path);
 
   // Callback method of FileSystemInterface::GetFile().
   // Runs ReadScreenshotFileForPreviewLocal if successful. Otherwise, runs
@@ -107,7 +137,7 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
   // notification is clicked.
   // |image| is a preview image attached to the notification. It can be empty.
   void OnReadScreenshotFileForPreviewCompleted(
-      ui::ScreenshotGrabberObserver::Result result,
+      ui::ScreenshotResult result,
       const base::FilePath& screenshot_path,
       gfx::Image image);
 
@@ -115,6 +145,9 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
 
   std::unique_ptr<ui::ScreenshotGrabber> screenshot_grabber_;
 
+  // Forwards OnScreenshotCompleted() events to a test.
+  ChromeScreenshotGrabberTestObserver* test_observer_ = nullptr;
+
   base::WeakPtrFactory<ChromeScreenshotGrabber> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeScreenshotGrabber);

chrome/browser/ui/ash/chrome_screenshot_grabber_browsertest.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
+#include "chrome/browser/ui/ash/chrome_screenshot_grabber_test_observer.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/session_manager/core/session_manager.h"
@@ -22,7 +23,7 @@
 
 class ChromeScreenshotGrabberBrowserTest
     : public InProcessBrowserTest,
-      public ui::ScreenshotGrabberObserver,
+      public ChromeScreenshotGrabberTestObserver,
       public ui::ClipboardObserver {
  public:
   ChromeScreenshotGrabberBrowserTest() = default;
@@ -36,9 +37,14 @@ class ChromeScreenshotGrabberBrowserTest
         base::Unretained(this)));
   }
 
+  void SetTestObserver(ChromeScreenshotGrabber* chrome_screenshot_grabber,
+                       ChromeScreenshotGrabberTestObserver* observer) {
+    chrome_screenshot_grabber->test_observer_ = observer;
+  }
+
   // Overridden from ui::ScreenshotGrabberObserver
   void OnScreenshotCompleted(
-      ScreenshotGrabberObserver::Result screenshot_result,
+      ui::ScreenshotResult screenshot_result,
       const base::FilePath& screenshot_path) override {
     screenshot_result_ = screenshot_result;
     screenshot_path_ = screenshot_path;
@@ -68,7 +74,7 @@ class ChromeScreenshotGrabberBrowserTest
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
 
   std::unique_ptr<NotificationDisplayServiceTester> display_service_;
-  ScreenshotGrabberObserver::Result screenshot_result_;
+  ui::ScreenshotResult screenshot_result_;
   base::FilePath screenshot_path_;
   bool notification_added_ = false;
   bool clipboard_changed_ = false;
@@ -80,25 +86,24 @@ class ChromeScreenshotGrabberBrowserTest
 IN_PROC_BROWSER_TEST_F(ChromeScreenshotGrabberBrowserTest, TakeScreenshot) {
   ChromeScreenshotGrabber* chrome_screenshot_grabber =
       ChromeScreenshotGrabber::Get();
-  chrome_screenshot_grabber->screenshot_grabber()->AddObserver(this);
+  SetTestObserver(chrome_screenshot_grabber, this);
   base::ScopedTempDir directory;
   ASSERT_TRUE(directory.CreateUniqueTempDir());
   EXPECT_TRUE(chrome_screenshot_grabber->CanTakeScreenshot());
 
-  chrome_screenshot_grabber->screenshot_grabber()->TakeScreenshot(
-      ash::Shell::GetPrimaryRootWindow(), gfx::Rect(0, 0, 100, 100),
-      directory.GetPath().AppendASCII("Screenshot.png"));
+  chrome_screenshot_grabber->HandleTakeWindowScreenshot(
+      ash::Shell::GetPrimaryRootWindow());
 
   EXPECT_FALSE(
       chrome_screenshot_grabber->screenshot_grabber()->CanTakeScreenshot());
 
   RunLoop();
-  chrome_screenshot_grabber->screenshot_grabber()->RemoveObserver(this);
+  SetTestObserver(chrome_screenshot_grabber, nullptr);
 
   EXPECT_TRUE(notification_added_);
   EXPECT_TRUE(display_service_->GetNotification(std::string("screenshot")));
 
-  EXPECT_EQ(ScreenshotGrabberObserver::SCREENSHOT_SUCCESS, screenshot_result_);
+  EXPECT_EQ(ui::ScreenshotResult::SUCCESS, screenshot_result_);
   EXPECT_TRUE(base::PathExists(screenshot_path_));
 
   EXPECT_FALSE(IsImageClipboardAvailable());

chrome/browser/ui/ash/chrome_screenshot_grabber_test_observer.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_UI_ASH_CHROME_SCREENSHOT_GRABBER_TEST_OBSERVER_H_
+#define CHROME_BROWSER_UI_ASH_CHROME_SCREENSHOT_GRABBER_TEST_OBSERVER_H_
+
+#include "ui/snapshot/screenshot_grabber.h"
+
+// Testing interface to ChromeScreenshotGrabber.
+class ChromeScreenshotGrabberTestObserver {
+ public:
+  // Dispatched after attempting to take a screenshot with the |result| and
+  // |screenshot_path| of the taken screenshot (if successful).
+  virtual void OnScreenshotCompleted(ui::ScreenshotResult screenshot_result,
+                                     const base::FilePath& screenshot_path) = 0;
+
+ protected:
+  virtual ~ChromeScreenshotGrabberTestObserver() {}
+};
+
+#endif  // CHROME_BROWSER_UI_ASH_CHROME_SCREENSHOT_GRABBER_TEST_OBSERVER_H_

ui/snapshot/BUILD.gn

@@ -10,7 +10,6 @@ jumbo_component("snapshot") {
   sources = [
     "screenshot_grabber.cc",
     "screenshot_grabber.h",
-    "screenshot_grabber_observer.h",
     "snapshot.cc",
     "snapshot.h",
     "snapshot_android.cc",

ui/snapshot/screenshot_grabber.cc

@@ -9,8 +9,8 @@
 #include <climits>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/callback.h"
-#include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ -20,7 +20,6 @@
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
-#include "ui/gfx/image/image.h"
 #include "ui/snapshot/snapshot.h"
 
 #if defined(USE_AURA)
@@ -35,80 +34,8 @@ namespace {
 // more than 1000 to prevent the conflict of filenames.
 const int kScreenshotMinimumIntervalInMS = 1000;
 
-using ShowNotificationCallback =
-    base::Callback<void(ScreenshotGrabberObserver::Result screenshot_result,
-                        const base::FilePath& screenshot_path)>;
-
-void SaveScreenshot(scoped_refptr<base::TaskRunner> ui_task_runner,
-                    const ShowNotificationCallback& callback,
-                    const base::FilePath& screenshot_path,
-                    scoped_refptr<base::RefCountedMemory> png_data,
-                    ScreenshotGrabberDelegate::FileResult result,
-                    const base::FilePath& local_path) {
-  DCHECK(!base::MessageLoopForUI::IsCurrent());
-  DCHECK(!screenshot_path.empty());
-
-  // Convert FileResult into ScreenshotGrabberObserver::Result.
-  ScreenshotGrabberObserver::Result screenshot_result =
-      ScreenshotGrabberObserver::SCREENSHOT_SUCCESS;
-  switch (result) {
-    case ScreenshotGrabberDelegate::FILE_SUCCESS:
-      // Successfully got a local file to write to, write png data.
-      DCHECK_GT(static_cast<int>(png_data->size()), 0);
-      if (static_cast<size_t>(base::WriteFile(
-              local_path, reinterpret_cast<const char*>(png_data->front()),
-              static_cast<int>(png_data->size()))) != png_data->size()) {
-        LOG(ERROR) << "Failed to save to " << local_path.value();
-        screenshot_result =
-            ScreenshotGrabberObserver::SCREENSHOT_WRITE_FILE_FAILED;
-      }
-      break;
-    case ScreenshotGrabberDelegate::FILE_CHECK_DIR_FAILED:
-      screenshot_result =
-          ScreenshotGrabberObserver::SCREENSHOT_CHECK_DIR_FAILED;
-      break;
-    case ScreenshotGrabberDelegate::FILE_CREATE_DIR_FAILED:
-      screenshot_result =
-          ScreenshotGrabberObserver::SCREENSHOT_CREATE_DIR_FAILED;
-      break;
-    case ScreenshotGrabberDelegate::FILE_CREATE_FAILED:
-      screenshot_result =
-          ScreenshotGrabberObserver::SCREENSHOT_CREATE_FILE_FAILED;
-      break;
-  }
-
-  // Report the result on the UI thread.
-  ui_task_runner->PostTask(
-      FROM_HERE, base::Bind(callback, screenshot_result, screenshot_path));
-}
-
-void EnsureLocalDirectoryExists(
-    const base::FilePath& path,
-    ScreenshotGrabberDelegate::FileCallback callback) {
-  DCHECK(!base::MessageLoopForUI::IsCurrent());
-  DCHECK(!path.empty());
-
-  if (!base::CreateDirectory(path.DirName())) {
-    LOG(ERROR) << "Failed to ensure the existence of "
-               << path.DirName().value();
-    callback.Run(ScreenshotGrabberDelegate::FILE_CREATE_DIR_FAILED, path);
-    return;
-  }
-
-  callback.Run(ScreenshotGrabberDelegate::FILE_SUCCESS, path);
-}
-
 }  // namespace
 
-void ScreenshotGrabberDelegate::PrepareFileAndRunOnBlockingPool(
-    const base::FilePath& path,
-    const FileCallback& callback_on_blocking_pool) {
-  base::PostTaskWithTraits(
-      FROM_HERE,
-      {base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
-      base::Bind(EnsureLocalDirectoryExists, path, callback_on_blocking_pool));
-}
-
 #if defined(USE_AURA)
 class ScreenshotGrabber::ScopedCursorHider {
  public:
@@ -138,15 +65,14 @@ class ScreenshotGrabber::ScopedCursorHider {
 };
 #endif
 
-ScreenshotGrabber::ScreenshotGrabber(ScreenshotGrabberDelegate* client)
-    : client_(client), factory_(this) {}
+ScreenshotGrabber::ScreenshotGrabber() : factory_(this) {}
 
 ScreenshotGrabber::~ScreenshotGrabber() {
 }
 
 void ScreenshotGrabber::TakeScreenshot(gfx::NativeWindow window,
                                        const gfx::Rect& rect,
-                                       const base::FilePath& screenshot_path) {
+                                       ScreenshotCallback callback) {
   DCHECK(base::MessageLoopForUI::IsCurrent());
   last_screenshot_timestamp_ = base::TimeTicks::Now();
 
@@ -165,8 +91,8 @@ void ScreenshotGrabber::TakeScreenshot(gfx::NativeWindow window,
   ui::GrabWindowSnapshotAsyncPNG(
       window, rect,
       base::Bind(&ScreenshotGrabber::GrabWindowSnapshotAsyncCallback,
-                 factory_.GetWeakPtr(), window_identifier, screenshot_path,
-                 is_partial));
+                 factory_.GetWeakPtr(), window_identifier, is_partial,
+                 base::Passed(&callback)));
 }
 
 bool ScreenshotGrabber::CanTakeScreenshot() {
@@ -175,58 +101,32 @@ bool ScreenshotGrabber::CanTakeScreenshot() {
              base::TimeDelta::FromMilliseconds(kScreenshotMinimumIntervalInMS);
 }
 
-void ScreenshotGrabber::NotifyScreenshotCompleted(
-    ScreenshotGrabberObserver::Result screenshot_result,
-    const base::FilePath& screenshot_path) {
-  DCHECK(base::MessageLoopForUI::IsCurrent());
-#if defined(USE_AURA)
-  cursor_hider_.reset();
-#endif
-  for (ScreenshotGrabberObserver& observer : observers_)
-    observer.OnScreenshotCompleted(screenshot_result, screenshot_path);
-}
-
-void ScreenshotGrabber::AddObserver(ScreenshotGrabberObserver* observer) {
-  observers_.AddObserver(observer);
-}
-
-void ScreenshotGrabber::RemoveObserver(ScreenshotGrabberObserver* observer) {
-  observers_.RemoveObserver(observer);
-}
-
-bool ScreenshotGrabber::HasObserver(
-    const ScreenshotGrabberObserver* observer) const {
-  return observers_.HasObserver(observer);
-}
-
 void ScreenshotGrabber::GrabWindowSnapshotAsyncCallback(
     const std::string& window_identifier,
-    base::FilePath screenshot_path,
     bool is_partial,
+    ScreenshotCallback callback,
     scoped_refptr<base::RefCountedMemory> png_data) {
   DCHECK(base::MessageLoopForUI::IsCurrent());
+
+#if defined(USE_AURA)
+  cursor_hider_.reset();
+#endif
+
   if (!png_data.get()) {
     if (is_partial) {
       LOG(ERROR) << "Failed to grab the window screenshot";
-      NotifyScreenshotCompleted(
-          ScreenshotGrabberObserver::SCREENSHOT_GRABWINDOW_PARTIAL_FAILED,
-          screenshot_path);
+      std::move(callback).Run(ScreenshotResult::GRABWINDOW_PARTIAL_FAILED,
+                              nullptr);
     } else {
       LOG(ERROR) << "Failed to grab the window screenshot for "
                  << window_identifier;
-      NotifyScreenshotCompleted(
-          ScreenshotGrabberObserver::SCREENSHOT_GRABWINDOW_FULL_FAILED,
-          screenshot_path);
+      std::move(callback).Run(ScreenshotResult::GRABWINDOW_FULL_FAILED,
+                              nullptr);
     }
     return;
   }
 
-  ShowNotificationCallback notification_callback(base::Bind(
-      &ScreenshotGrabber::NotifyScreenshotCompleted, factory_.GetWeakPtr()));
-  client_->PrepareFileAndRunOnBlockingPool(
-      screenshot_path,
-      base::Bind(&SaveScreenshot, base::ThreadTaskRunnerHandle::Get(),
-                 notification_callback, screenshot_path, png_data));
+  std::move(callback).Run(ScreenshotResult::SUCCESS, std::move(png_data));
 }
 
 }  // namespace ui

ui/snapshot/screenshot_grabber.h

@@ -14,61 +14,44 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/weak_ptr.h"
-#include "base/observer_list.h"
 #include "base/time/time.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/native_widget_types.h"
-#include "ui/snapshot/screenshot_grabber_observer.h"
 #include "ui/snapshot/snapshot_export.h"
 
 namespace ui {
 
-// TODO(flackr): Componentize google drive so that we don't need the
-// ScreenshotGrabberDelegate.
-class SNAPSHOT_EXPORT ScreenshotGrabberDelegate {
- public:
-  enum FileResult {
-    FILE_SUCCESS,
-    FILE_CHECK_DIR_FAILED,
-    FILE_CREATE_DIR_FAILED,
-    FILE_CREATE_FAILED
-  };
-
-  // Callback called with the |result| of trying to create a local writable
-  // |path| for the possibly remote path.
-  using FileCallback =
-      base::Callback<void(FileResult result, const base::FilePath& path)>;
-
-  ScreenshotGrabberDelegate() {}
-  virtual ~ScreenshotGrabberDelegate() {}
-
-  // Prepares a writable file for |path|. If |path| is a non-local path (i.e.
-  // Google drive) and it is supported this will create a local cached copy of
-  // the remote file and call the callback with the local path.
-  virtual void PrepareFileAndRunOnBlockingPool(
-      const base::FilePath& path,
-      const FileCallback& callback_on_blocking_pool);
+// Result of the entire screenshotting attempt. This enum is fat for various
+// file operations which could happen in the chrome layer.
+enum class ScreenshotResult {
+  SUCCESS,
+  GRABWINDOW_PARTIAL_FAILED,
+  GRABWINDOW_FULL_FAILED,
+  CREATE_DIR_FAILED,
+  GET_DIR_FAILED,
+  CHECK_DIR_FAILED,
+  CREATE_FILE_FAILED,
+  WRITE_FILE_FAILED,
+  DISABLED
 };
 
 class SNAPSHOT_EXPORT ScreenshotGrabber {
  public:
-  explicit ScreenshotGrabber(ScreenshotGrabberDelegate* client);
+  ScreenshotGrabber();
   ~ScreenshotGrabber();
 
+  // Callback for the new system, which ignores the observer crud.
+  using ScreenshotCallback =
+      base::OnceCallback<void(ui::ScreenshotResult screenshot_result,
+                              scoped_refptr<base::RefCountedMemory> png_data)>;
+
   // Takes a screenshot of |rect| in |window| in that window's coordinate space
-  // saving the result to |screenshot_path|.
+  // and return it to |callback|.
   void TakeScreenshot(gfx::NativeWindow window,
                       const gfx::Rect& rect,
-                      const base::FilePath& screenshot_path);
-  bool CanTakeScreenshot();
+                      ScreenshotCallback callback);
 
-  void NotifyScreenshotCompleted(
-      ScreenshotGrabberObserver::Result screenshot_result,
-      const base::FilePath& screenshot_path);
-
-  void AddObserver(ScreenshotGrabberObserver* observer);
-  void RemoveObserver(ScreenshotGrabberObserver* observer);
-  bool HasObserver(const ScreenshotGrabberObserver* observer) const;
+  bool CanTakeScreenshot();
 
  private:
 #if defined(USE_AURA)
@@ -77,13 +60,10 @@ class SNAPSHOT_EXPORT ScreenshotGrabber {
 
   void GrabWindowSnapshotAsyncCallback(
       const std::string& window_identifier,
-      base::FilePath screenshot_path,
       bool is_partial,
+      ScreenshotCallback callback,
       scoped_refptr<base::RefCountedMemory> png_data);
 
-  // A weak pointer to the screenshot taker client.
-  ScreenshotGrabberDelegate* client_;
-
   // The timestamp when the screenshot task was issued last time.
   base::TimeTicks last_screenshot_timestamp_;
 
@@ -92,7 +72,6 @@ class SNAPSHOT_EXPORT ScreenshotGrabber {
   std::unique_ptr<ScopedCursorHider> cursor_hider_;
 #endif
 
-  base::ObserverList<ScreenshotGrabberObserver> observers_;
   base::WeakPtrFactory<ScreenshotGrabber> factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenshotGrabber);

ui/snapshot/screenshot_grabber_observer.h

-// Copyright 2014 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.
-
-#ifndef UI_SNAPSHOT_SCREENSHOT_GRABBER_OBSERVER_H_
-#define UI_SNAPSHOT_SCREENSHOT_GRABBER_OBSERVER_H_
-
-#include "base/files/file_path.h"
-#include "ui/snapshot/snapshot_export.h"
-
-namespace ui {
-
-class SNAPSHOT_EXPORT ScreenshotGrabberObserver {
- public:
-  enum Result {
-    SCREENSHOT_SUCCESS = 0,
-    SCREENSHOT_GRABWINDOW_PARTIAL_FAILED,
-    SCREENSHOT_GRABWINDOW_FULL_FAILED,
-    SCREENSHOT_CREATE_DIR_FAILED,
-    SCREENSHOT_GET_DIR_FAILED,
-    SCREENSHOT_CHECK_DIR_FAILED,
-    SCREENSHOT_CREATE_FILE_FAILED,
-    SCREENSHOT_WRITE_FILE_FAILED,
-    SCREENSHOTS_DISABLED,
-    SCREENSHOT_RESULT_COUNT
-  };
-
-  // Dispatched after attempting to take a screenshot with the |result| and
-  // |screenshot_path| of the taken screenshot (if successful).
-  virtual void OnScreenshotCompleted(Result screenshot_result,
-                                     const base::FilePath& screenshot_path) = 0;
-
- protected:
-  virtual ~ScreenshotGrabberObserver() {}
-};
-
-}  // namespace ui
-
-#endif  // UI_SNAPSHOT_SCREENSHOT_GRABBER_OBSERVER_H_