Screenshot effect non-obvious

chrome/browser/ui/ash/screenshot_taker.cc

Index: chrome/browser/ui/ash/screenshot_taker.cc
diff --git a/chrome/browser/ui/ash/screenshot_taker.cc b/chrome/browser/ui/ash/screenshot_taker.cc
index e5742f9dd8a57d100c2a75de28dde8fcefa44d0c..b1a14d8fd8440b6f9fa6f7048cbbbe9244672ffb 100644
--- a/chrome/browser/ui/ash/screenshot_taker.cc
+++ b/chrome/browser/ui/ash/screenshot_taker.cc
@@ -12,20 +12,29 @@
 #include "ash/shell_window_ids.h"
 #include "base/bind.h"
 #include "base/file_util.h"
-#include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/stringprintf.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/time.h"
+#include "base/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/extensions/file_manager_util.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/notifications/notification.h"
+#include "chrome/browser/notifications/notification_ui_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/webui/screenshot_source.h"
 #include "chrome/browser/ui/window_snapshot/window_snapshot.h"
 #include "content/public/browser/browser_thread.h"
+#include "grit/ash_strings.h"
+#include "grit/theme_resources.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/image/image.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
@@ -33,17 +42,95 @@
 #endif
 
 namespace {
-// How opaque should the layer that we flash onscreen to provide visual
-// feedback after the screenshot is taken be?
-const float kVisualFeedbackLayerOpacity = 0.25f;
-
-// How long should the visual feedback layer be displayed?
-const int64 kVisualFeedbackLayerDisplayTimeMs = 100;
-
 // The minimum interval between two screenshot commands.  It has to be
 // more than 1000 to prevent the conflict of filenames.
 const int kScreenshotMinimumIntervalInMS = 1000;
 
+const char kNotificationOriginUrl[] = "chrome://screenshot";
+
+class ScreenshotTakerNotificationDelegate : public NotificationDelegate {
+ public:
+  ScreenshotTakerNotificationDelegate(const std::string& id_text,
+                                      bool success,
+                                      const base::FilePath& screenshot_path)
+      : id_text_(id_text),
+        success_(success),
+        screenshot_path_(screenshot_path) {
+  }
+
+  virtual void Display() OVERRIDE {}
+
+  virtual void Error() OVERRIDE {}
+
+  virtual void Close(bool by_user) OVERRIDE {}
+
+  virtual void Click() OVERRIDE {
+    if (success_)
+      file_manager_util::ShowFileInFolder(screenshot_path_);
+  }
+
+  virtual std::string id() const OVERRIDE { return id_text_; }
+
+  virtual content::RenderViewHost* GetRenderViewHost() const OVERRIDE {
+    return NULL;
+  }
+
+ private:
+  virtual ~ScreenshotTakerNotificationDelegate() {}
+
+  std::string id_text_;

[sky, 2013/03/31 01:16:03]

Make all of these const if they can be.

[sschmitz, 2013/04/02 21:23:25]

Done.

+  bool success_;
+  base::FilePath screenshot_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScreenshotTakerNotificationDelegate);
+};
+
+void ShowNotification(ScreenshotTakerObserver::Result screenshot_result,
+                      const base::FilePath& screenshot_path) {
+#if defined(OS_CHROMEOS)
+  // Notifications only on Chrome OS.

[sky, 2013/03/31 01:16:03]

Why is the notification only for chromeos?

[sschmitz, 2013/04/02 21:23:25]

Removed.

+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  static int id = 0;
+  std::string id_text = base::StringPrintf("screenshot_%3.3d", ++id);
+  string16 replace_id = UTF8ToUTF16(id_text);
+  bool success =
+      (screenshot_result == ScreenshotTakerObserver::SCREENSHOT_SUCCESS);
+  Notification notification(
+      GURL(kNotificationOriginUrl),
+      ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+          IDR_SCREENSHOT_NOTIFICATION_ICON),
+      l10n_util::GetStringUTF16(
+          success ?
+          IDS_ASH_SCREENSHOT_NOTIFICATION_TITLE_SUCCESS :
+          IDS_ASH_SCREENSHOT_NOTIFICATION_TITLE_FAIL),
+      l10n_util::GetStringUTF16(
+          success ?
+          IDS_ASH_SCREENSHOT_NOTIFICATION_TEXT_SUCCESS :
+          IDS_ASH_SCREENSHOT_NOTIFICATION_TEXT_FAIL),
+      WebKit::WebTextDirectionDefault,
+      string16(),
+      replace_id,
+      new ScreenshotTakerNotificationDelegate(id_text,
+                                              success,
+                                              screenshot_path));
+  Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();

[sky, 2013/03/31 01:16:03]

Can ScreenshotTaker so that we minimize uses of GetDefaultPro...

[sschmitz, 2013/04/02 21:23:25]

Changed: ScreenshotTake takes profile in ctor.

+  DCHECK(profile);
+  g_browser_process->notification_ui_manager()->Add(notification, profile);
+
+  if (ScreenshotTaker::GetObserverForTest()) {

[sky, 2013/03/31 01:16:03]

Why is this in the ifdef?

[sschmitz, 2013/04/02 21:23:25]

Removed.

+    ScreenshotTaker::GetObserverForTest()->OnScreenshotCompleted(
+        screenshot_result, screenshot_path);
+  }
+#endif
+}
+
+void PostShowNotification(ScreenshotTakerObserver::Result screenshot_result,
+                          const base::FilePath& screenshot_path) {
+  DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&ShowNotification, screenshot_result, screenshot_path));
+}
 
 void SaveScreenshotInternal(const base::FilePath& screenshot_path,
                             scoped_refptr<base::RefCountedBytes> png_data) {
@@ -54,7 +141,12 @@ void SaveScreenshotInternal(const base::FilePath& screenshot_path,
           reinterpret_cast<char*>(&(png_data->data()[0])),
           png_data->size())) != png_data->size()) {
     LOG(ERROR) << "Failed to save to " << screenshot_path.value();
+    PostShowNotification(ScreenshotTakerObserver::SCREENSHOT_WRITE_FILE_FAILED,
+                         screenshot_path);
+    return;
   }
+  PostShowNotification(ScreenshotTakerObserver::SCREENSHOT_SUCCESS,
+                       screenshot_path);
 }
 
 void SaveScreenshot(const base::FilePath& screenshot_path,
@@ -65,6 +157,8 @@ void SaveScreenshot(const base::FilePath& screenshot_path,
   if (!file_util::CreateDirectory(screenshot_path.DirName())) {
     LOG(ERROR) << "Failed to ensure the existence of "
                << screenshot_path.DirName().value();
+    PostShowNotification(ScreenshotTakerObserver::SCREENSHOT_CREATE_DIR_FAILED,
+                         screenshot_path);
     return;
   }
   SaveScreenshotInternal(screenshot_path, png_data);
@@ -77,6 +171,8 @@ void SaveScreenshotToDrive(scoped_refptr<base::RefCountedBytes> png_data,
                            const base::FilePath& local_path) {
   if (error != drive::DRIVE_FILE_OK) {
     LOG(ERROR) << "Failed to write screenshot image to Google Drive: " << error;
+    PostShowNotification(
+        ScreenshotTakerObserver::SCREENSHOT_CREATE_FILE_FAILED, local_path);
     return;
   }
   SaveScreenshotInternal(local_path, png_data);
@@ -91,6 +187,8 @@ void EnsureDirectoryExistsCallback(
   // of the target file exists.
   if (error == drive::DRIVE_FILE_OK ||
       error == drive::DRIVE_FILE_ERROR_EXISTS) {
+    // Note: The last two arguments of SaveScreenshotToDrive are appended by
+    // PrepareWritableFileAndRun.
     drive::util::PrepareWritableFileAndRun(
         profile,
         screenshot_path,
@@ -98,6 +196,8 @@ void EnsureDirectoryExistsCallback(
   } else {
     LOG(ERROR) << "Failed to ensure the existence of the specified directory "
                << "in Google Drive: " << error;
+    ShowNotification(ScreenshotTakerObserver::SCREENSHOT_CHECK_DIR_FAILED,
+                     screenshot_path);
   }
 }
 
@@ -106,6 +206,8 @@ void PostSaveScreenshotTask(const base::FilePath& screenshot_path,
   if (drive::util::IsUnderDriveMountPoint(screenshot_path)) {
     Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
     if (profile) {
+      // Note: The last argument of EnsureDirectoryExistsCallback is
+      // appended by EnsureDirectoryExists.
       drive::util::EnsureDirectoryExists(
           profile,
           screenshot_path.DirName(),
@@ -148,6 +250,8 @@ bool GrabWindowSnapshot(aura::Window* window,
 
 }  // namespace
 
+ScreenshotTakerObserver* ScreenshotTaker::observer_for_test_ = NULL;
+
 ScreenshotTaker::ScreenshotTaker() {
 }
 
@@ -156,8 +260,13 @@ ScreenshotTaker::~ScreenshotTaker() {
 
 void ScreenshotTaker::HandleTakeScreenshotForAllRootWindows() {
   base::FilePath screenshot_directory;
-  if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory))
+  if (!screenshot_directory_for_test_.empty()) {
+    screenshot_directory = screenshot_directory_for_test_;
+  } else if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory)) {
+    ShowNotification(ScreenshotTakerObserver::SCREENSHOT_GET_DIR_FAILED,
+                     base::FilePath());
     return;
+  }
 
   std::string screenshot_basename =
       ScreenshotSource::GetScreenshotBaseFilename();
@@ -176,12 +285,15 @@ void ScreenshotTaker::HandleTakeScreenshotForAllRootWindows() {
     gfx::Rect rect = root_window->bounds();
     if (root_windows.size() > 1)
       basename += base::StringPrintf(" - Display %d", static_cast<int>(i + 1));
+    base::FilePath screenshot_path =
+        screenshot_directory.AppendASCII(basename + ".png");
     if (GrabWindowSnapshot(root_window, rect, &png_data->data())) {
-      DisplayVisualFeedback(rect);
-      PostSaveScreenshotTask(
-          screenshot_directory.AppendASCII(basename + ".png"), png_data);
+      PostSaveScreenshotTask(screenshot_path, png_data);
     } else {
       LOG(ERROR) << "Failed to grab the window screenshot for " << i;
+      ShowNotification(
+          ScreenshotTakerObserver::SCREENSHOT_GRABWINDOW_FULL_FAILED,
+          screenshot_path);
     }
   }
   last_screenshot_timestamp_ = base::Time::Now();
@@ -192,20 +304,27 @@ void ScreenshotTaker::HandleTakePartialScreenshot(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   base::FilePath screenshot_directory;
-  if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory))
+  if (!screenshot_directory_for_test_.empty()) {
+    screenshot_directory = screenshot_directory_for_test_;
+  } else if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory)) {
+    ShowNotification(ScreenshotTakerObserver::SCREENSHOT_GET_DIR_FAILED,
+                     base::FilePath());
     return;
+  }
 
   scoped_refptr<base::RefCountedBytes> png_data(new base::RefCountedBytes);
 
+  base::FilePath screenshot_path =
+      screenshot_directory.AppendASCII(
+          ScreenshotSource::GetScreenshotBaseFilename() + ".png");
   if (GrabWindowSnapshot(window, rect, &png_data->data())) {
     last_screenshot_timestamp_ = base::Time::Now();
-    DisplayVisualFeedback(rect);
-    PostSaveScreenshotTask(
-        screenshot_directory.AppendASCII(
-                   ScreenshotSource::GetScreenshotBaseFilename() + ".png"),
-                   png_data);
+    PostSaveScreenshotTask(screenshot_path, png_data);
   } else {
     LOG(ERROR) << "Failed to grab the window screenshot";
+    ShowNotification(
+        ScreenshotTakerObserver::SCREENSHOT_GRABWINDOW_PARTIAL_FAILED,
+        screenshot_path);
   }
 }
 
@@ -216,25 +335,17 @@ bool ScreenshotTaker::CanTakeScreenshot() {
           kScreenshotMinimumIntervalInMS);
 }
 
-void ScreenshotTaker::CloseVisualFeedbackLayer() {
-  visual_feedback_layer_.reset();
+void ScreenshotTaker::SetScreenshotDirectoryForTest(
+    const base::FilePath& directory) {
+  screenshot_directory_for_test_ = directory;
 }
 
-void ScreenshotTaker::DisplayVisualFeedback(const gfx::Rect& rect) {
-  visual_feedback_layer_.reset(new ui::Layer(ui::LAYER_SOLID_COLOR));
-  visual_feedback_layer_->SetColor(SK_ColorWHITE);
-  visual_feedback_layer_->SetOpacity(kVisualFeedbackLayerOpacity);
-  visual_feedback_layer_->SetBounds(rect);
-
-  ui::Layer* parent = ash::Shell::GetContainer(
-      ash::Shell::GetActiveRootWindow(),
-      ash::internal::kShellWindowId_OverlayContainer)->layer();
-  parent->Add(visual_feedback_layer_.get());
-  visual_feedback_layer_->SetVisible(true);
+// static
+void ScreenshotTaker::SetObserverForTest(ScreenshotTakerObserver* observer) {
+  observer_for_test_ = observer;
+}
 
-  MessageLoopForUI::current()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&ScreenshotTaker::CloseVisualFeedbackLayer,
-                 base::Unretained(this)),
-      base::TimeDelta::FromMilliseconds(kVisualFeedbackLayerDisplayTimeMs));
+// static
+ScreenshotTakerObserver* ScreenshotTaker::GetObserverForTest() {
+  return observer_for_test_;
 }