Crash when taking screenshot onto Chrome OS Google Drive

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 e53f54349e43804bbe1b8d9a1ee32626e579a25c..5c6b60bb58c5b53f92347edb19d856a38e9ba23b 100644
--- a/chrome/browser/ui/ash/screenshot_taker.cc
+++ b/chrome/browser/ui/ash/screenshot_taker.cc
@@ -21,6 +21,7 @@
 #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"
@@ -50,10 +51,10 @@ const char kNotificationOriginUrl[] = "chrome://screenshot";
 // notification.
 class ScreenshotTakerNotificationDelegate : public NotificationDelegate {
  public:
-  ScreenshotTakerNotificationDelegate(const std::string& id_text,
+  ScreenshotTakerNotificationDelegate(const std::string& notification_id,
                                       bool success,
                                       const base::FilePath& screenshot_path)
-      : id_text_(id_text),
+      : notification_id_(notification_id),
         success_(success),
         screenshot_path_(screenshot_path) {
   }
@@ -71,7 +72,7 @@ class ScreenshotTakerNotificationDelegate : public NotificationDelegate {
     // TODO(sschmitz): perhaps add similar action for Windows.
 #endif
   }
-  virtual std::string id() const OVERRIDE { return id_text_; }
+  virtual std::string id() const OVERRIDE { return notification_id_; }
   virtual content::RenderViewHost* GetRenderViewHost() const OVERRIDE {
     return NULL;
   }
@@ -79,7 +80,7 @@ class ScreenshotTakerNotificationDelegate : public NotificationDelegate {
  private:
   virtual ~ScreenshotTakerNotificationDelegate() {}
 
-  const std::string id_text_;
+  const std::string notification_id_;
   const bool success_;
   const base::FilePath screenshot_path_;
 
@@ -223,10 +224,9 @@ bool GrabWindowSnapshot(aura::Window* window,
 
 }  // namespace
 
-ScreenshotTaker::ScreenshotTaker(Profile* profile)
-    : profile_(profile),
-      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
-  DCHECK(profile_);
+ScreenshotTaker::ScreenshotTaker()
+    : ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)),
+      profile_for_test_(NULL) {
 }
 
 ScreenshotTaker::~ScreenshotTaker() {
@@ -265,7 +265,7 @@ void ScreenshotTaker::HandleTakeScreenshotForAllRootWindows() {
     if (GrabWindowSnapshot(root_window, rect, &png_data->data())) {
       PostSaveScreenshotTask(
           base::Bind(&ScreenshotTaker::ShowNotification, factory_.GetWeakPtr()),
-          profile_,
+          GetProfile(),
           screenshot_path,
           png_data);
     } else {
@@ -302,7 +302,7 @@ void ScreenshotTaker::HandleTakePartialScreenshot(
     last_screenshot_timestamp_ = base::Time::Now();
     PostSaveScreenshotTask(
         base::Bind(&ScreenshotTaker::ShowNotification, factory_.GetWeakPtr()),
-        profile_,
+        GetProfile(),
         screenshot_path,
         png_data);
   } else {
@@ -326,9 +326,11 @@ void ScreenshotTaker::ShowNotification(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 #if defined(OS_CHROMEOS)
   // TODO(sschmitz): make this work for Windows.
+  if (!notification_id_.empty())
+    g_browser_process->notification_ui_manager()->CancelById(notification_id_);

[James Cook, 2013/04/18 23:46:09]

Why not just always give screenshots the same ID, like "screenshot"?  Would that
mess up some kind of notification history?

[sschmitz, 2013/04/19 00:51:28]

I talked to mukai@. And, yes, its ok and better to use the same notification id.
Done.

   static int id = 0;
-  std::string id_text = base::StringPrintf("screenshot_%3.3d", ++id);
-  string16 replace_id = UTF8ToUTF16(id_text);
+  notification_id_ = base::StringPrintf("screenshot_%3.3d", ++id);
+  string16 replace_id = UTF8ToUTF16("screenshot");
   bool success =
       (screenshot_result == ScreenshotTakerObserver::SCREENSHOT_SUCCESS);
   Notification notification(
@@ -346,10 +348,11 @@ void ScreenshotTaker::ShowNotification(
       WebKit::WebTextDirectionDefault,
       string16(),
       replace_id,
-      new ScreenshotTakerNotificationDelegate(id_text,
+      new ScreenshotTakerNotificationDelegate(notification_id_,
                                               success,
                                               screenshot_path));
-  g_browser_process->notification_ui_manager()->Add(notification, profile_);
+  g_browser_process->notification_ui_manager()->Add(notification,
+                                                    GetProfile());
 #endif
 
   FOR_EACH_OBSERVER(ScreenshotTakerObserver, observers_,
@@ -368,12 +371,22 @@ bool ScreenshotTaker::HasObserver(ScreenshotTakerObserver* observer) const {
   return observers_.HasObserver(observer);
 }
 
+Profile* ScreenshotTaker::GetProfile() {
+  if (profile_for_test_)
+    return profile_for_test_;
+  return ProfileManager::GetDefaultProfileOrOffTheRecord();
+}
+
 void ScreenshotTaker::SetScreenshotDirectoryForTest(
     const base::FilePath& directory) {
   screenshot_directory_for_test_ = directory;
 }
 
 void ScreenshotTaker::SetScreenshotBasenameForTest(
-    const std::string& basename){
+    const std::string& basename) {
   screenshot_basename_for_test_ = basename;
 }
+
+void ScreenshotTaker::SetScreenshotProfileForTest(Profile* profile) {
+  profile_for_test_ = profile;
+}