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..6de05b5fd20ee8a623e5d5716943a15a14a7d44e 100644
--- a/chrome/browser/ui/ash/screenshot_taker.cc
+++ b/chrome/browser/ui/ash/screenshot_taker.cc
@@ -18,14 +18,24 @@
 #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 +43,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_;
+  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.

[Jun Mukai, 2013/03/29 23:01:37]

I am not so sure the point here.
If so, no feedback in other platforms? That doesn't sound quite well.
IIRC this file itself is only for ChromeOS right now. And if we want to allow
this to other platforms, probably we should consider the feedback on them.
Anyway the guard doesn't make much sense to me.

[sschmitz, 2013/03/29 23:40:44]

We had dicussions on this and decided non-ChromeOS should or do use other
methods of screenshot taking. We ended up putting the icon in "cros", i.e.
ChromeOS only, that necessitates the guard. We could reconsider in the future?
I propose to leave it for now.

[Jun Mukai, 2013/03/30 00:37:17]

Ah, got it. thanks!

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

On second thought... and more discussions. I have removed the Chrome OS ifdef.
It is now for Ash.

+  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();
+  DCHECK(profile);
+  g_browser_process->notification_ui_manager()->Add(notification, profile);
+
+  if (ScreenshotTaker::GetObserverForTest()) {
+    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 +142,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 +158,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 +172,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 +188,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 +197,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 +207,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 +251,8 @@ bool GrabWindowSnapshot(aura::Window* window,
 
 }  // namespace
 
+ScreenshotTakerObserver* ScreenshotTaker::observer_for_test_ = NULL;
+
 ScreenshotTaker::ScreenshotTaker() {
 }
 
@@ -156,8 +261,13 @@ ScreenshotTaker::~ScreenshotTaker() {
 
 void ScreenshotTaker::HandleTakeScreenshotForAllRootWindows() {
   base::FilePath screenshot_directory;
-  if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory))
+  if (screenshot_directory_for_test_.get()) {
+    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 +286,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 +305,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_.get()) {
+    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 +336,17 @@ bool ScreenshotTaker::CanTakeScreenshot() {
           kScreenshotMinimumIntervalInMS);
 }
 
-void ScreenshotTaker::CloseVisualFeedbackLayer() {
-  visual_feedback_layer_.reset();
+void ScreenshotTaker::SetScreenshotDirectoryForTest(
+    const base::FilePath& directory) {
+  screenshot_directory_for_test_.reset(new base::FilePath(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_;
 }