Move non-browser specific ScreenshotTaker code to ui/snapshot.

ui/snapshot/screenshot_taker.cc

+// 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.
+
+#include "ui/snapshot/screenshot_taker.h"
+
+#include <climits>
+#include <string>
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "base/task_runner.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/time/time.h"
+#include "ui/gfx/image/image.h"
+#include "ui/snapshot/snapshot.h"
+
+#if defined(USE_AURA)
+#include "ui/aura/window.h"
+#endif
+
+namespace ui {
+
+namespace {
+// The minimum interval between two screenshot commands.  It has to be
+// more than 1000 to prevent the conflict of filenames.
+const int kScreenshotMinimumIntervalInMS = 1000;
+
+typedef base::Callback<void(ScreenshotTakerObserver::Result screenshot_result,

[sky, 2014/11/20 15:03:59]

using.

[flackr, 2014/11/26 18:05:08]

Done.

+                            const base::FilePath& screenshot_path)>
+    ShowNotificationCallback;
+
+void SaveScreenshot(scoped_refptr<base::TaskRunner> ui_task_runner,
+                    const ShowNotificationCallback& callback,
+                    const base::FilePath& screenshot_path,
+                    scoped_refptr<base::RefCountedBytes> png_data,
+                    ScreenshotTakerClient::WritableFileResult result,
+                    const base::FilePath& local_path) {
+  DCHECK(!screenshot_path.empty());
+
+  // Convert WritableFileResult into ScreenshotTakerObserver::Result.
+  ScreenshotTakerObserver::Result screenshot_result =
+      ScreenshotTakerObserver::SCREENSHOT_SUCCESS;
+  switch (result) {
+    case ScreenshotTakerClient::WRITABLE_FILE_SUCCESS:
+      // Successful so far, continue to attempt to take screenshot.
+      break;
+    case ScreenshotTakerClient::WRITABLE_FILE_CHECK_DIR_FAILED:
+      screenshot_result = ScreenshotTakerObserver::SCREENSHOT_CHECK_DIR_FAILED;
+      break;
+    case ScreenshotTakerClient::WRITABLE_FILE_CREATE_DIR_FAILED:
+      screenshot_result = ScreenshotTakerObserver::SCREENSHOT_CREATE_DIR_FAILED;
+      break;
+    case ScreenshotTakerClient::WRITABLE_FILE_CREATE_FAILED:
+      screenshot_result =
+          ScreenshotTakerObserver::SCREENSHOT_CREATE_FILE_FAILED;
+      break;
+  }
+
+  if (screenshot_result == ScreenshotTakerObserver::SCREENSHOT_SUCCESS &&
+      static_cast<size_t>(base::WriteFile(
+          local_path, reinterpret_cast<char*>(&(png_data->data()[0])),
+          static_cast<int>(png_data->size()))) != png_data->size()) {

[sky, 2014/11/20 15:03:59]

DCHECK size isn't too big (because of cast)?

[flackr, 2014/11/26 18:05:08]

Done.

+    LOG(ERROR) << "Failed to save to " << local_path.value();
+    screenshot_result = ScreenshotTakerObserver::SCREENSHOT_WRITE_FILE_FAILED;
+  }
+
+  // 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,
+    scoped_refptr<base::TaskRunner> ui_task_runner,
+    ScreenshotTakerClient::WritableFileCallback callback) {
+  DCHECK(!path.empty());
+
+  if (!base::CreateDirectory(path.DirName())) {
+    LOG(ERROR) << "Failed to ensure the existence of "
+               << path.DirName().value();
+    ui_task_runner->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   ScreenshotTakerClient::WRITABLE_FILE_CREATE_DIR_FAILED,
+                   path));
+    return;
+  }
+
+  callback.Run(ScreenshotTakerClient::WRITABLE_FILE_SUCCESS, path);

[sky, 2014/11/20 15:03:59]

Why is the callback run directly here but posted to the UI thread above?

[flackr, 2014/11/26 18:05:08]

Oops, we actually should always run the result on the blocking pool (which will
post to ui thread on error on line 71). Removed post to ui thread above and
added DCHECK that we're not running callback (SaveScreenshot) on ui thread.

+}
+
+}  // namespace
+
+void ScreenshotTakerClient::PrepareWritableFileAndRunOnBlockingPool(
+    const base::FilePath& path,
+    scoped_refptr<base::TaskRunner> blocking_task_runner,
+    WritableFileCallback callback_on_blocking_pool) {
+  blocking_task_runner->PostTask(
+      FROM_HERE, base::Bind(EnsureLocalDirectoryExists, path,
+                            blocking_task_runner, callback_on_blocking_pool));
+}
+
+ScreenshotTaker::ScreenshotTaker(
+    ScreenshotTakerClient* client,
+    scoped_refptr<base::TaskRunner> blocking_task_runner)
+    : client_(client),
+      blocking_task_runner_(blocking_task_runner),
+      factory_(this) {
+}
+
+ScreenshotTaker::~ScreenshotTaker() {
+}
+
+void ScreenshotTaker::TakeScreenshot(gfx::NativeWindow window,
+                                     const gfx::Rect& rect,
+                                     const base::FilePath& screenshot_path) {
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+  last_screenshot_timestamp_ = base::Time::Now();
+
+  bool is_partial = true;
+  // Window identifier is used to log a message on failure to capture a full
+  // screen (i.e. non partial) screenshot. The only time is_partial can be
+  // false, we will also have an identification string for the window.
+  std::string window_identifier;
+#if defined(USE_AURA)
+  aura::Window* aura_window = static_cast<aura::Window*>(window);
+  is_partial = rect == aura_window->bounds();

[sky, 2014/11/20 15:03:59]

I would have thought you compare sizes here? But this gets back to my question
in the .h as to what coordinates rect is in.

[flackr, 2014/11/26 18:05:08]

Oops, the inequality was wrong, but yes this can just be a size check. Done.

+  window_identifier = aura_window->GetBoundsInScreen().ToString();

[sky, 2014/11/20 15:03:59]

What is the window identifier used for? Seems bizarre to use the bounds in
screen as the identifier.

[flackr, 2014/11/26 18:05:08]

It's only used for the logged error message (line 180) to indicate which screen
the screenshot failed on.

+#endif
+  ui::GrabWindowSnapshotAsync(
+      window, rect, blocking_task_runner_,
+      base::Bind(&ScreenshotTaker::GrabWindowSnapshotAsyncCallback,
+                 factory_.GetWeakPtr(), window_identifier, screenshot_path,
+                 is_partial));
+}
+
+bool ScreenshotTaker::CanTakeScreenshot() {
+  return last_screenshot_timestamp_.is_null() ||
+         base::Time::Now() - last_screenshot_timestamp_ >
+             base::TimeDelta::FromMilliseconds(kScreenshotMinimumIntervalInMS);
+}
+
+void ScreenshotTaker::NotifyScreenshotCompleted(
+    ScreenshotTakerObserver::Result screenshot_result,
+    const base::FilePath& screenshot_path) {
+  DCHECK(base::MessageLoopForUI::IsCurrent());
+  FOR_EACH_OBSERVER(ScreenshotTakerObserver, observers_,
+                    OnScreenshotCompleted(screenshot_result, screenshot_path));
+}
+
+void ScreenshotTaker::AddObserver(ScreenshotTakerObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void ScreenshotTaker::RemoveObserver(ScreenshotTakerObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+bool ScreenshotTaker::HasObserver(
+    const ScreenshotTakerObserver* observer) const {
+  return observers_.HasObserver(observer);
+}
+
+void ScreenshotTaker::GrabWindowSnapshotAsyncCallback(
+    const std::string& window_identifier,
+    base::FilePath screenshot_path,
+    bool is_partial,
+    scoped_refptr<base::RefCountedBytes> png_data) {
+  if (!png_data.get()) {

[sky, 2014/11/20 15:03:59]

DCHECK for thread?

[flackr, 2014/11/26 18:05:08]

Done.

+    if (is_partial) {
+      LOG(ERROR) << "Failed to grab the window screenshot";
+      NotifyScreenshotCompleted(
+          ScreenshotTakerObserver::SCREENSHOT_GRABWINDOW_PARTIAL_FAILED,
+          screenshot_path);
+    } else {
+      LOG(ERROR) << "Failed to grab the window screenshot for "
+                 << window_identifier;
+      NotifyScreenshotCompleted(
+          ScreenshotTakerObserver::SCREENSHOT_GRABWINDOW_FULL_FAILED,
+          screenshot_path);
+    }
+    return;
+  }
+
+  client_->PrepareWritableFileAndRunOnBlockingPool(
+      screenshot_path, blocking_task_runner_,
+      base::Bind(&SaveScreenshot, base::MessageLoop::current()->task_runner(),

[sky, 2014/11/20 15:03:59]

Nested bindings... Holy unreadable code batman!

[flackr, 2014/11/26 18:05:08]

Does it help to locally declare the inner callback like so?

+                 base::Bind(&ScreenshotTaker::NotifyScreenshotCompleted,
+                            factory_.GetWeakPtr()),
+                 screenshot_path, png_data));
+}
+
+}  // namespace ui