Fix saved screenshots for feedback.

chrome/browser/ui/webui/screenshot_source.cc

Index: chrome/browser/ui/webui/screenshot_source.cc
diff --git a/chrome/browser/ui/webui/screenshot_source.cc b/chrome/browser/ui/webui/screenshot_source.cc
index b8a21bbf1883ab4565422184e770fb78a7adc75a..f7641a99a75575caeb295819ae12cb942ac5a3bc 100644
--- a/chrome/browser/ui/webui/screenshot_source.cc
+++ b/chrome/browser/ui/webui/screenshot_source.cc
@@ -4,10 +4,11 @@
 
 #include "chrome/browser/ui/webui/screenshot_source.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/file_util.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/path_service.h"
-#include "base/synchronization/waitable_event.h"
 #include "base/task.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/url_constants.h"
@@ -18,78 +19,71 @@ static const char kCurrentScreenshot[] = "current";
 static const char kSavedScreenshots[] = "saved/";
 #endif
 
-static const char kScreenshotsRelativePath[] = "/Screenshots/";
+void ScreenshotSource::CacheAndSendScreenshot(const std::string& path,
+    int request_id, const std::vector<unsigned char> bytes) {

[zel, 2011/08/12 13:01:37]

const std::vector<unsigned char>& bytes ?

[rkc, 2011/08/22 13:23:52]

Done.

+  cached_screenshots_[path] = bytes;
+  SendResponse(request_id, new RefCountedBytes(bytes));
+}
+
+std::vector<unsigned char> ScreenshotSource::GetCachedScreenshot(

[zel, 2011/08/12 13:01:37]

can you avoid data copy here? can't you just return
unsigned char* 

[rkc, 2011/08/22 13:23:52]

Done.

+    const std::string& full_path) {
+  std::string path = full_path.substr(0, full_path.find_first_of("?"));
+  if (cached_screenshots_.find(path) != cached_screenshots_.end()) {
+    return cached_screenshots_[path];
+  } else {
+    // TODO(rkc): Weird vc bug, return std::vector<unsigned char>() causes
+    // the object assigned to the return value of this function magically
+    // change it's address 0x0; look into this eventually.

[Daniel Erat, 2011/08/12 14:43:40]

nit: "its", not "it's"

[rkc, 2011/08/22 13:23:52]

Done.

+    std::vector<unsigned char> ret;
+    return ret;
+  }
+}
 
 #if defined(OS_CHROMEOS)
-// Read the file from the screenshots directory into the read_bytes vector.
-void ReadScreenshot(const std::string& filename,
-                   std::vector<unsigned char>* read_bytes,
-                   base::WaitableEvent* read_complete) {
-  read_bytes->clear();
+void ScreenshotSource::SendSavedScreenshot(const std::string& path,
+                                           int request_id) {
+  std::vector<unsigned char> read_bytes;
+  std::string filename = path.substr(strlen(kSavedScreenshots));
 
   FilePath fileshelf_path;
   if (!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &fileshelf_path)) {
-    read_complete->Signal();
+    CacheAndSendScreenshot(path, request_id, read_bytes);
     return;
   }
 
-  FilePath file(fileshelf_path.value() + std::string(kScreenshotsRelativePath) +
-                filename);
-
   int64 file_size = 0;
+  FilePath file = fileshelf_path.Append(filename);
   if (!file_util::GetFileSize(file, &file_size)) {
-    read_complete->Signal();
+    CacheAndSendScreenshot(path, request_id, read_bytes);
     return;
   }
 
-  // expand vector to file size
-  read_bytes->resize(file_size);
-  // read file into the vector
-  int bytes_read = 0;
-  if (!(bytes_read = file_util::ReadFile(file,
-                                         reinterpret_cast<char*>(
-                                             &read_bytes->front()),
-                                             static_cast<int>(file_size))))
-    read_bytes->clear();
-
-  // We're done, if successful, read_bytes will have the data
-  // otherwise, it'll be empty.
-  read_complete->Signal();
-}
+  read_bytes.resize(file_size);
+  if (!file_util::ReadFile(file, reinterpret_cast<char*>(&read_bytes.front()),
+                           static_cast<int>(file_size)))
+    read_bytes.clear();
 
-// Get a saved screenshot - read on the FILE thread.
-std::vector<unsigned char> GetSavedScreenshot(std::string filename) {
-  base::WaitableEvent read_complete(true, false);
-  std::vector<unsigned char> bytes;
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                          NewRunnableFunction(&ReadScreenshot, filename,
-                                              &bytes, &read_complete));
-  read_complete.Wait();
-  return bytes;
+  CacheAndSendScreenshot(path, request_id, read_bytes);
 }
 #endif
 
-std::vector<unsigned char> ScreenshotSource::GetScreenshot(
-    const std::string& full_path) {
+void ScreenshotSource::SendScreenshot(const std::string& full_path,
+                                      int request_id) {
   // Strip the query param value - we only use it as a hack to ensure our
   // image gets reloaded instead of being pulled from the browser cache
   std::string path = full_path.substr(0, full_path.find_first_of("?"));
   if (path == kCurrentScreenshot) {
-    return current_screenshot_;
+    CacheAndSendScreenshot(path, request_id, current_screenshot_);
 #if defined(OS_CHROMEOS)
   } else if (path.compare(0, strlen(kSavedScreenshots),
                           kSavedScreenshots) == 0) {
-    // Split the saved screenshot filename from the path
-    std::string filename = path.substr(strlen(kSavedScreenshots));
-
-    return GetSavedScreenshot(filename);
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                            base::Bind(&ScreenshotSource::SendSavedScreenshot,
+                                       base::Unretained(this), path,
+                                       request_id));
 #endif
   } else {
-    std::vector<unsigned char> ret;
-    // TODO(rkc): Weird vc bug, return std::vector<unsigned char>() causes
-    // the object assigned to the return value of this function magically
-    // change it's address 0x0; look into this eventually.
-    return ret;
+    CacheAndSendScreenshot(path, request_id, std::vector<unsigned char>());
   }
 }
 
@@ -105,10 +99,9 @@ ScreenshotSource::ScreenshotSource(
 
 ScreenshotSource::~ScreenshotSource() {}
 
-void ScreenshotSource::StartDataRequest(const std::string& path,
-                                            bool is_incognito,
-                                            int request_id) {
-  SendResponse(request_id, new RefCountedBytes(GetScreenshot(path)));
+void ScreenshotSource::StartDataRequest(const std::string& path, bool,
+                                        int request_id) {
+  SendScreenshot(path, request_id);
 }
 
 std::string ScreenshotSource::GetMimeType(const std::string&) const {