Fix saved screenshots for feedback.

chrome/browser/ui/webui/screenshot_source.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_UI_WEBUI_SCREENSHOT_SOURCE_H_
 #define CHROME_BROWSER_UI_WEBUI_SCREENSHOT_SOURCE_H_
 
+#include <map>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/memory/linked_ptr.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 
+typedef std::vector<unsigned char> ScreenshotData;
+typedef linked_ptr<ScreenshotData> ScreenshotDataPtr;
+
 // ScreenshotSource is the data source that serves screenshots (saved
 // or current) to the bug report html ui
 class ScreenshotSource : public ChromeURLDataManager::DataSource {
  public:
   explicit ScreenshotSource(
       std::vector<unsigned char>* current_screenshot);
 
   // Called when the network layer has requested a resource underneath
   // the path we registered.
   virtual void StartDataRequest(const std::string& path,
                                 bool is_incognito,
                                 int request_id);
 
   virtual std::string GetMimeType(const std::string&) const;
 
-  std::vector<unsigned char> GetScreenshot(const std::string& path);
+  // Get the screenshot specified by the given relative path that we've cached

[Daniel Erat, 2011/08/23 16:58:37]

this is confusing to me.  the comment says "relative path" but the parameter is
named "full_path" (which i interpret as synonymous with "absolute path").

[rkc, 2011/08/25 13:09:33]

I've redone all the 'path' parameter names in both the files. The naming should
be accurate and make much more sense now.
Done.

+  // from a previous request to the screenshots source

[Daniel Erat, 2011/08/23 16:58:37]

nit: add period at end of sentence

[rkc, 2011/08/25 13:09:33]

Done.

+  // Note: Query string will be stripped from the full_path

[Daniel Erat, 2011/08/23 16:58:37]

nit: add period

[rkc, 2011/08/25 13:09:33]

Done.

+  ScreenshotDataPtr GetCachedScreenshot(const std::string& full_path);
 
  private:
   virtual ~ScreenshotSource();
 
-  std::vector<unsigned char> current_screenshot_;
+  // Sends the screenshot data to the requestor while caching it locally to the
+  // class instance, indexed by path.
+  void CacheAndSendScreenshot(const std::string& path,
+                              int request_id,
+                              ScreenshotDataPtr bytes);
+  // Send the screenshot specified by the given relative path (includes filename
+  // in case of a saved screenshot) to the requestor

[Daniel Erat, 2011/08/23 16:58:37]

nit: add period

[rkc, 2011/08/25 13:09:33]

Done.

+  void SendScreenshot(const std::string& path, int request_id);
+#if defined(OS_CHROMEOS)
+  // Send a saved screenshot specified by the given filename to the requestor
+  void SendSavedScreenshot(const std::string& filename, int request_id);

[Daniel Erat, 2011/08/23 16:58:37]

why do these methods still all have subtly different parameter names (full_path
vs. path vs. filename)?  also, the header still doesn't match the
implementation.  SendSavedScreenshot()'s parameter is "filename" here but "path"
in the .cc file.

[rkc, 2011/08/25 13:09:33]

Changed the names and comments to make sense all the functions.
Done.

+#endif
+
+  // Pointer to the screenshot data for the current screenshot
+  ScreenshotDataPtr current_screenshot_;

[Daniel Erat, 2011/08/23 16:58:37]

nit: add blank line after this one

[rkc, 2011/08/25 13:09:33]

Done.

+  // Key: Relative path to the screenshot (including filename)
+  // Value: Pointer to the screenshot data associated with the path
+  std::map<std::string, ScreenshotDataPtr> cached_screenshots_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenshotSource);
 };
 
 #endif  // CHROME_BROWSER_UI_WEBUI_SCREENSHOT_SOURCE_H_