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..dd082dd9b1f9df0a335c8308740a45f1c8c38040 100644
--- a/chrome/browser/ui/ash/screenshot_taker.cc
+++ b/chrome/browser/ui/ash/screenshot_taker.cc
@@ -18,14 +18,23 @@
 #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"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
@@ -44,20 +53,42 @@ const int64 kVisualFeedbackLayerDisplayTimeMs = 100;
 // more than 1000 to prevent the conflict of filenames.
 const int kScreenshotMinimumIntervalInMS = 1000;
 
+// Origin URL for notifications.
+const GURL kNotificationOriginUrl("chrome://screenshot/");
 
-void SaveScreenshotInternal(const base::FilePath& screenshot_path,
+void PostNotification(base::WeakPtr<ScreenshotTaker> screenshot_taker,
+                      bool success,
+                      const base::FilePath& screenshot_path) {
+#if defined(OS_CHROMEOS)
+  // Notifications only on Chrome OS.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&ScreenshotTaker::ShowNotification,
+                 screenshot_taker,
+                 success,
+                 screenshot_path));
+#endif
+}
+
+void SaveScreenshotInternal(base::WeakPtr<ScreenshotTaker> screenshot_taker,
+                            const base::FilePath& screenshot_path,
                             scoped_refptr<base::RefCountedBytes> png_data) {
   DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
   DCHECK(!screenshot_path.empty());
+  bool success = true;
   if (static_cast<size_t>(file_util::WriteFile(
           screenshot_path,
           reinterpret_cast<char*>(&(png_data->data()[0])),
           png_data->size())) != png_data->size()) {
     LOG(ERROR) << "Failed to save to " << screenshot_path.value();
+    success = false;
   }
+  PostNotification(screenshot_taker, success, screenshot_path);
 }
 
-void SaveScreenshot(const base::FilePath& screenshot_path,
+void SaveScreenshot(base::WeakPtr<ScreenshotTaker> screenshot_taker,
+                    const base::FilePath& screenshot_path,
                     scoped_refptr<base::RefCountedBytes> png_data) {
   DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
   DCHECK(!screenshot_path.empty());
@@ -65,24 +96,28 @@ 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();
+    PostNotification(screenshot_taker, false, screenshot_path);
     return;
   }
-  SaveScreenshotInternal(screenshot_path, png_data);
+  SaveScreenshotInternal(screenshot_taker, screenshot_path, png_data);
 }
 
 // TODO(kinaba): crbug.com/140425, remove this ungly #ifdef dispatch.
 #if defined(OS_CHROMEOS)
-void SaveScreenshotToDrive(scoped_refptr<base::RefCountedBytes> png_data,
+void SaveScreenshotToDrive(base::WeakPtr<ScreenshotTaker> screenshot_taker,
+                           scoped_refptr<base::RefCountedBytes> png_data,
                            drive::DriveFileError error,
                            const base::FilePath& local_path) {
   if (error != drive::DRIVE_FILE_OK) {
     LOG(ERROR) << "Failed to write screenshot image to Google Drive: " << error;
+    PostNotification(screenshot_taker, false, local_path);
     return;
   }
-  SaveScreenshotInternal(local_path, png_data);
+  SaveScreenshotInternal(screenshot_taker, local_path, png_data);
 }
 
 void EnsureDirectoryExistsCallback(
+    base::WeakPtr<ScreenshotTaker> screenshot_taker,
     Profile* profile,
     const base::FilePath& screenshot_path,
     scoped_refptr<base::RefCountedBytes> png_data,
@@ -91,39 +126,54 @@ 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,
-        base::Bind(&SaveScreenshotToDrive, png_data));
+        base::Bind(&SaveScreenshotToDrive, screenshot_taker, png_data));
   } else {
     LOG(ERROR) << "Failed to ensure the existence of the specified directory "
                << "in Google Drive: " << error;
+    PostNotification(screenshot_taker, false, screenshot_path);
   }
 }
 
-void PostSaveScreenshotTask(const base::FilePath& screenshot_path,
+void PostSaveScreenshotTask(base::WeakPtr<ScreenshotTaker> screenshot_taker,
+                            const base::FilePath& screenshot_path,
                             scoped_refptr<base::RefCountedBytes> png_data) {
   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(),
           base::Bind(&EnsureDirectoryExistsCallback,
+                     screenshot_taker,
                      profile,
                      screenshot_path,
                      png_data));
     }
   } else {
     content::BrowserThread::GetBlockingPool()->PostTask(
-        FROM_HERE, base::Bind(&SaveScreenshot, screenshot_path, png_data));
+        FROM_HERE, base::Bind(&SaveScreenshot,
+                              screenshot_taker,
+                              screenshot_path,
+                              png_data));
   }
 }
 #else
-void PostSaveScreenshotTask(const base::FilePath& screenshot_path,
+void PostSaveScreenshotTask(base::WeakPtr<ScreenshotTaker> screenshot_taker,
+                            const base::FilePath& screenshot_path,
                             scoped_refptr<base::RefCountedBytes> png_data) {
   content::BrowserThread::GetBlockingPool()->PostTask(
-      FROM_HERE, base::Bind(&SaveScreenshot, screenshot_path, png_data));
+      FROM_HERE, base::Bind(&SaveScreenshot,
+                            screenshot_taker,
+                            screenshot_path,
+                            png_data)):
+
 }
 #endif
 
@@ -148,7 +198,22 @@ bool GrabWindowSnapshot(aura::Window* window,
 
 }  // namespace
 
-ScreenshotTaker::ScreenshotTaker() {
+ScreenshotTaker::ScreenshotTaker()
+    :  ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
+       notification_id_(0) {
+#if defined(OS_CHROMEOS)
+  // Notifications only on Chrome OS.
+  notification_icon_ = ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+      IDR_SCREENSHOT_NOTIFICATION_ICON);
+  notification_title_success_ =
+      l10n_util::GetStringUTF16(IDS_ASH_SCREENSHOT_NOTIFICATION_TITLE_SUCCESS);
+  notification_title_fail_ =
+      l10n_util::GetStringUTF16(IDS_ASH_SCREENSHOT_NOTIFICATION_TITLE_FAIL);
+  notification_text_success_ =
+      l10n_util::GetStringUTF16(IDS_ASH_SCREENSHOT_NOTIFICATION_TEXT_SUCCESS);
+  notification_text_fail_ =
+      l10n_util::GetStringUTF16(IDS_ASH_SCREENSHOT_NOTIFICATION_TEXT_FAIL);
+#endif
 }
 
 ScreenshotTaker::~ScreenshotTaker() {
@@ -176,12 +241,13 @@ 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(factory_.GetWeakPtr(), screenshot_path, png_data);
     } else {
       LOG(ERROR) << "Failed to grab the window screenshot for " << i;
+      PostNotification(factory_.GetWeakPtr(), false, screenshot_path);
     }
   }
   last_screenshot_timestamp_ = base::Time::Now();
@@ -197,15 +263,15 @@ void ScreenshotTaker::HandleTakePartialScreenshot(
 
   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(factory_.GetWeakPtr(), screenshot_path, png_data);
   } else {
     LOG(ERROR) << "Failed to grab the window screenshot";
+    PostNotification(factory_.GetWeakPtr(), false, screenshot_path);
   }
 }
 
@@ -216,25 +282,62 @@ bool ScreenshotTaker::CanTakeScreenshot() {
           kScreenshotMinimumIntervalInMS);
 }
 
-void ScreenshotTaker::CloseVisualFeedbackLayer() {
-  visual_feedback_layer_.reset();
-}
+namespace {

[James Cook, 2013/03/26 23:54:25]

Anonymous namespace goes at top of file.

[sschmitz, 2013/03/27 14:58:49]

Done.

+class Delegate : public NotificationDelegate {

[James Cook, 2013/03/26 23:54:25]

Call this something other than "Delegate", maybe ScreenshotDelegate or
ScreenshotNotification or something?

[sschmitz, 2013/03/27 14:58:49]

Done.

+ public:
+  Delegate(const std::string& id_text,
+           bool success,
+           const base::FilePath& screenshot_path)
+      : id_text_(id_text),
+        success_(success),
+        screenshot_path_(screenshot_path) {
+  }
 
-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);
+  virtual void Display() OVERRIDE {}
 
-  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);
+  virtual void Error() OVERRIDE {}
 
-  MessageLoopForUI::current()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&ScreenshotTaker::CloseVisualFeedbackLayer,
-                 base::Unretained(this)),
-      base::TimeDelta::FromMilliseconds(kVisualFeedbackLayerDisplayTimeMs));
+  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 ~Delegate() {}
+
+  std::string id_text_;
+  bool success_;
+  base::FilePath screenshot_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(Delegate);
+};
+} // namespace
+
+void ScreenshotTaker::ShowNotification(

[James Cook, 2013/03/26 23:54:25]

Could you pass all the data you need as parameters to this function?  Then it
could be an anonymous function and you wouldn't need weak pointers to the class.

[sschmitz, 2013/03/27 14:58:49]

Good point. I found a similar solution. The more or less constant notification
data is split into a struct with static members. ShowNotification only needs a
bool and file-path. No more need to pass a weak ptr or data around.
Done

+    bool success, const base::FilePath& screenshot_path) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  std::string id_text =
+      base::StringPrintf("screenshot_%3.3d", ++notification_id_);
+  string16 notification_replace_id = UTF8ToUTF16(id_text);
+  Notification notification(
+      kNotificationOriginUrl,
+      notification_icon_,
+      success ? notification_title_success_ : notification_title_fail_,
+      success ? notification_text_success_ : notification_text_fail_,
+      WebKit::WebTextDirectionDefault,
+      string16(), // display_source
+      notification_replace_id,
+      new Delegate(id_text, success, screenshot_path));
+  Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
+  DCHECK(profile);
+  DCHECK(g_browser_process);
+  g_browser_process->notification_ui_manager()->Add(notification, profile);
 }