Fix saved screenshots for feedback.

Fixed the location where the saved screenshots are picked from. Also fixed the issue with saved screenshots blocking the UI.

R=zelidrag@chromium.org,derat@chromium.org
BUG=chromium-os:18227,chromium:73180
TEST=Tested with sending feedback using a saved screenshot.

Review URL: http://codereview.chromium.org/7635017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98934 0039d316-1c4b-4281-b951-d872f2087c98

14 files changed, 131 insertions, 548 deletions

diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 74d4752..9cf9fb7 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -40,7 +40,6 @@
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h"
 #import "chrome/browser/ui/cocoa/browser_window_cocoa.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
-#import "chrome/browser/ui/cocoa/bug_report_window_controller.h"
 #import "chrome/browser/ui/cocoa/confirm_quit_panel_controller.h"
 #import "chrome/browser/ui/cocoa/encoding_menu_controller_delegate_mac.h"
 #import "chrome/browser/ui/cocoa/history_menu_bridge.h"
@@ -743,6 +742,9 @@ const AEEventClass kAECloudPrintUninstallClass = 'GCPu';
           sync_ui_util::UpdateSyncItem(item, enable, lastProfile);
           break;
         }
+        case IDC_FEEDBACK:
+          enable = NO;
+          break;
         default:
           enable = menuState_->IsCommandEnabled(tag) ?
                    [self keyWindowIsNotModal] : NO;
@@ -861,17 +863,6 @@ const AEEventClass kAECloudPrintUninstallClass = 'GCPu';
       else
         Browser::OpenHelpWindow(lastProfile);
       break;
-    case IDC_FEEDBACK: {
-      Browser* browser = BrowserList::GetLastActive();
-      TabContents* currentTab =
-          browser ? browser->GetSelectedTabContents() : NULL;
-      BugReportWindowController* controller =
-          [[BugReportWindowController alloc]
-              initWithTabContents:currentTab
-                          profile:[self lastProfile]];
-      [controller runModalDialog];
-      break;
-    }
     case IDC_SYNC_BOOKMARKS:
       // The profile may be NULL during shutdown -- see
       // http://code.google.com/p/chromium/issues/detail?id=43048 .
diff --git a/chrome/browser/bug_report_data.cc b/chrome/browser/bug_report_data.cc
index 4fccd79..2311b04 100644
--- a/chrome/browser/bug_report_data.cc
+++ b/chrome/browser/bug_report_data.cc
@@ -30,7 +30,7 @@ void BugReportData::UpdateData(Profile* profile,
                                const int problem_type,
                                const std::string& page_url,
                                const std::string& description,
-                               const std::vector<unsigned char>& image
+                               ScreenshotDataPtr image
 #if defined(OS_CHROMEOS)
                                , const std::string& user_email
                                , const bool send_sys_info
@@ -61,16 +61,12 @@ void BugReportData::SendReport() {
   sent_report_ = true;
 #endif
 
-  int image_data_size = image_.size();
-  char* image_data = image_data_size ?
-      reinterpret_cast<char*>(&(image_.front())) : NULL;
   gfx::Rect& screen_size = BugReportUtil::GetScreenshotSize();
   BugReportUtil::SendReport(profile_
                             , problem_type_
                             , page_url_
                             , description_
-                            , image_data
-                            , image_data_size
+                            , image_
                             , screen_size.width()
                             , screen_size.height()
 #if defined(OS_CHROMEOS)
diff --git a/chrome/browser/bug_report_data.h b/chrome/browser/bug_report_data.h
index e329b81..2b404b2 100644
--- a/chrome/browser/bug_report_data.h
+++ b/chrome/browser/bug_report_data.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/ui/webui/screenshot_source.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/system/syslogs_provider.h"
@@ -28,7 +29,7 @@ class BugReportData {
                   const int problem_type,
                   const std::string& page_url,
                   const std::string& description,
-                  const std::vector<unsigned char>& image
+                  ScreenshotDataPtr image
 #if defined(OS_CHROMEOS)
                   , const std::string& user_email
                   , const bool send_sys_info
@@ -46,7 +47,7 @@ class BugReportData {
   int problem_type() const { return problem_type_; }
   const std::string& page_url() const { return page_url_; }
   const std::string& description() const { return description_; }
-  const std::vector<unsigned char>& image() const { return image_; }
+  ScreenshotDataPtr image() const { return image_; }
 #if defined(OS_CHROMEOS)
   const std::string& user_email() const { return user_email_; }
   chromeos::system::LogDictionaryType* sys_info() const { return sys_info_; }
@@ -65,7 +66,7 @@ class BugReportData {
   int problem_type_;
   std::string page_url_;
   std::string description_;
-  std::vector<unsigned char> image_;
+  ScreenshotDataPtr image_;
 
 #if defined(OS_CHROMEOS)
   // Chromeos specific values for SendReport.
diff --git a/chrome/browser/bug_report_util.cc b/chrome/browser/bug_report_util.cc
index f00f85e..8c4340a 100644
--- a/chrome/browser/bug_report_util.cc
+++ b/chrome/browser/bug_report_util.cc
@@ -253,8 +253,7 @@ void BugReportUtil::SendReport(
     , int problem_type
     , const std::string& page_url_text
     , const std::string& description
-    , const char* png_data
-    , int png_data_length
+    , ScreenshotDataPtr image_data_ptr
     , int png_width
     , int png_height
 #if defined(OS_CHROMEOS)
@@ -309,7 +308,7 @@ void BugReportUtil::SendReport(
   AddFeedbackData(&feedback_data, std::string(kOsVersionTag), os_version);
 
   // Include the page image if we have one.
-  if (png_data) {
+  if (image_data_ptr.get() && image_data_ptr->size()) {
     userfeedback::PostedScreenshot screenshot;
     screenshot.set_mime_type(kPngMimeType);
     // Set the dimensions of the screenshot
@@ -317,7 +316,10 @@ void BugReportUtil::SendReport(
     dimensions.set_width(static_cast<float>(png_width));
     dimensions.set_height(static_cast<float>(png_height));
     *(screenshot.mutable_dimensions()) = dimensions;
-    screenshot.set_binary_content(std::string(png_data, png_data_length));
+
+    int image_data_size = image_data_ptr->size();
+    char* image_data = reinterpret_cast<char*>(&(image_data_ptr->front()));
+    screenshot.set_binary_content(std::string(image_data, image_data_size));
 
     // Set the screenshot object in feedback
     *(feedback_data.mutable_screenshot()) = screenshot;
diff --git a/chrome/browser/bug_report_util.h b/chrome/browser/bug_report_util.h
index edc9739..a6fc808 100644
--- a/chrome/browser/bug_report_util.h
+++ b/chrome/browser/bug_report_util.h
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "chrome/browser/ui/webui/screenshot_source.h"
 #include "chrome/browser/userfeedback/proto/common.pb.h"
 #include "chrome/browser/userfeedback/proto/extension.pb.h"
 #include "chrome/browser/userfeedback/proto/math.pb.h"
@@ -68,8 +69,7 @@ class BugReportUtil {
       , int problem_type
       , const std::string& page_url_text
       , const std::string& description
-      , const char* png_data
-      , int png_data_length
+      , ScreenshotDataPtr png_data
       , int png_width
       , int png_height
 #if defined(OS_CHROMEOS)
diff --git a/chrome/browser/ui/cocoa/browser_window_cocoa.mm b/chrome/browser/ui/cocoa/browser_window_cocoa.mm
index 9ad67b9..cf51c3f 100644
--- a/chrome/browser/ui/cocoa/browser_window_cocoa.mm
+++ b/chrome/browser/ui/cocoa/browser_window_cocoa.mm
@@ -21,7 +21,6 @@
 #import "chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/browser_window_utils.h"
-#import "chrome/browser/ui/cocoa/bug_report_window_controller.h"
 #import "chrome/browser/ui/cocoa/chrome_event_processing_window.h"
 #import "chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.h"
 #import "chrome/browser/ui/cocoa/download/download_shelf_controller.h"
diff --git a/chrome/browser/ui/cocoa/bug_report_window_controller.h b/chrome/browser/ui/cocoa/bug_report_window_controller.h
deleted file mode 100644
index 272c5bf..0000000
--- a/chrome/browser/ui/cocoa/bug_report_window_controller.h
+++ /dev/null
@@ -1,112 +0,0 @@
-// 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_COCOA_BUG_REPORT_WINDOW_CONTROLLER_H_
-#define CHROME_BROWSER_UI_COCOA_BUG_REPORT_WINDOW_CONTROLLER_H_
-#pragma once
-
-#import <Cocoa/Cocoa.h>
-
-#include <vector>
-
-#include "base/memory/scoped_nsobject.h"
-
-class Profile;
-class TabContents;
-
-// A window controller for managing the "Report Bug" feature. Modally
-// presents a dialog that allows the user to either file a bug report on
-// a broken page, or go directly to Google's "Report Phishing" page and
-// file a report there.
-@interface BugReportWindowController : NSWindowController {
- @private
-  TabContents* currentTab_;  // Weak, owned by browser.
-  Profile* profile_;  // Weak, owned by browser.
-
-  // Holds screenshot of current tab.
-  std::vector<unsigned char> pngData_;
-  // Width and height of the current tab's screenshot.
-  int pngWidth_;
-  int pngHeight_;
-
-  // Values bound to data in the dialog box. These values cannot be boxed in
-  // scoped_nsobjects because we use them for bindings.
-  NSString* bugDescription_;  // Strong.
-  NSUInteger bugTypeIndex_;
-  NSString* pageTitle_;  // Strong.
-  NSString* pageURL_;  // Strong.
-
-  // We keep a pointer to this button so we can change its title.
-  IBOutlet NSButton* sendReportButton_;
-
-  // This button must be moved when the send report button changes title.
-  IBOutlet NSButton* cancelButton_;
-
-  // The popup button that allows choice of bug type.
-  IBOutlet NSPopUpButton* bugTypePopUpButton_;
-
-  // YES sends a screenshot along with the bug report.
-  BOOL sendScreenshot_;
-
-  // Disable screenshot if no browser window is open.
-  BOOL disableScreenshotCheckbox_;
-
-  // Menu for the bug type popup button.  We create it here instead of in
-  // IB so that we can nicely check whether the phishing page is selected,
-  // and so that we can create a menu without "page" options when no browser
-  // window is open.
-  NSMutableArray* bugTypeList_;  // Strong.
-
-  // When dialog switches from regular bug reports to phishing page, "save
-  // screenshot" and "description" are disabled. Save the state of this value
-  // to restore if the user switches back to a regular bug report before
-  // sending.
-  BOOL saveSendScreenshot_;
-  scoped_nsobject<NSString> saveBugDescription_;  // Strong
-
-  // Maps bug type menu item title strings to BugReportUtil::BugType ints.
-  NSDictionary* bugTypeDictionary_;  // Strong
-}
-
-// Properties for bindings.
-@property(nonatomic, copy) NSString* bugDescription;
-@property(nonatomic) NSUInteger bugTypeIndex;
-@property(nonatomic, copy) NSString* pageTitle;
-@property(nonatomic, copy) NSString* pageURL;
-@property(nonatomic) BOOL sendScreenshot;
-@property(nonatomic) BOOL disableScreenshotCheckbox;
-@property(nonatomic, readonly) NSArray* bugTypeList;
-
-// Initialize with the contents of the tab to be reported as buggy / wrong.
-// If dialog is called without an open window, currentTab may be null; in
-// that case, a dialog is opened with options for reporting a bugs not
-// related to a specific page.  Profile is passed to BugReportUtil, who
-// will not send a report if the value is null.
-- (id)initWithTabContents:(TabContents*)currentTab profile:(Profile*)profile;
-
-// Run the dialog with an application-modal event loop.  If the user accepts,
-// send the report of the bug or broken web site.
-- (void)runModalDialog;
-
-// IBActions for the dialog buttons.
-- (IBAction)sendReport:(id)sender;
-- (IBAction)cancel:(id)sender;
-
-// YES if the user has selected the phishing report option.
-- (BOOL)isPhishingReport;
-
-// Converts the bug type from the menu into the correct value for the bug type
-// from BugReportUtil::BugType.
-- (int)bugTypeFromIndex;
-
-// Force the description text field to allow "return" to go to the next line
-// within the description field. Without this delegate method, "return" falls
-// back to the "Send Report" action, because this button has been bound to
-// the return key in IB.
-- (BOOL)control:(NSControl*)control textView:(NSTextView*)textView
-    doCommandBySelector:(SEL)commandSelector;
-
-@end
-
-#endif  // CHROME_BROWSER_UI_COCOA_BUG_REPORT_WINDOW_CONTROLLER_H_
diff --git a/chrome/browser/ui/cocoa/bug_report_window_controller.mm b/chrome/browser/ui/cocoa/bug_report_window_controller.mm
deleted file mode 100644
index abe5a53..0000000
--- a/chrome/browser/ui/cocoa/bug_report_window_controller.mm
+++ /dev/null
@@ -1,232 +0,0 @@
-// 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.
-
-#import "chrome/browser/ui/cocoa/bug_report_window_controller.h"
-
-#include "base/mac/mac_util.h"
-#include "base/sys_string_conversions.h"
-#include "chrome/browser/bug_report_util.h"
-#include "chrome/browser/ui/window_snapshot/window_snapshot.h"
-#include "content/browser/tab_contents/tab_contents.h"
-#include "content/browser/tab_contents/tab_contents_view.h"
-#include "grit/chromium_strings.h"
-#include "grit/generated_resources.h"
-#include "third_party/GTM/AppKit/GTMUILocalizerAndLayoutTweaker.h"
-#include "ui/base/l10n/l10n_util_mac.h"
-
-@implementation BugReportWindowController
-
-@synthesize bugDescription = bugDescription_;
-@synthesize bugTypeIndex = bugTypeIndex_;
-@synthesize pageURL = pageURL_;
-@synthesize pageTitle = pageTitle_;
-@synthesize sendScreenshot = sendScreenshot_;
-@synthesize disableScreenshotCheckbox = disableScreenshotCheckbox_;
-@synthesize bugTypeList = bugTypeList_;
-
-- (id)initWithTabContents:(TabContents*)currentTab
-                  profile:(Profile*)profile {
-  NSString* nibpath = [base::mac::MainAppBundle() pathForResource:@"ReportBug"
-                                                          ofType:@"nib"];
-  if ((self = [super initWithWindowNibPath:nibpath owner:self])) {
-    currentTab_ = currentTab;
-    profile_ = profile;
-
-    // The order of strings in this array must match the order of the bug types
-    // declared below in the bugTypeFromIndex function.
-    bugTypeList_ = [[NSMutableArray alloc] initWithObjects:
-        l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_CHROME_MISBEHAVES),
-        l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_SOMETHING_MISSING),
-        l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_BROWSER_CRASH),
-        l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_OTHER_PROBLEM),
-        nil];
-
-    pngHeight_ = 0;
-    pngWidth_ = 0;
-
-    if (currentTab_ != NULL) {
-      // Get data from current tab, if one exists. This dialog could be called
-      // from the main menu with no tab contents, so currentTab_ is not
-      // guaranteed to be non-NULL.
-      // TODO(mirandac): This dialog should be a tab-modal sheet if a browser
-      // window exists.
-      [self setSendScreenshot:YES];
-      [self setDisableScreenshotCheckbox:NO];
-      // Insert menu items about bugs related to specific pages.
-      [bugTypeList_ insertObjects:
-          [NSArray arrayWithObjects:
-              l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_PAGE_WONT_LOAD),
-              l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_PAGE_LOOKS_ODD),
-              l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_PHISHING_PAGE),
-              l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_CANT_SIGN_IN),
-              nil]
-          atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, 4)]];
-
-      [self setPageURL:base::SysUTF8ToNSString(
-          currentTab_->controller().GetActiveEntry()->url().spec())];
-      [self setPageTitle:base::SysUTF16ToNSString(currentTab_->GetTitle())];
-      gfx::Rect pngRect = browser::GrabWindowSnapshot(
-          currentTab_->view()->GetTopLevelNativeWindow(), &pngData_);
-      pngWidth_ = pngRect.width();
-      pngHeight_ = pngRect.height();
-    } else {
-      // If no current tab exists, create a menu without the "broken page"
-      // options, with page URL and title empty, and screenshot disabled.
-      [self setSendScreenshot:NO];
-      [self setDisableScreenshotCheckbox:YES];
-    }
-  }
-  return self;
-}
-
-- (void)dealloc {
-  [pageURL_ release];
-  [pageTitle_ release];
-  [bugDescription_ release];
-  [bugTypeList_ release];
-  [bugTypeDictionary_ release];
-  [super dealloc];
-}
-
-// Delegate callback so that closing the window deletes the controller.
-- (void)windowWillClose:(NSNotification*)notification {
-  [self autorelease];
-}
-
-- (void)closeDialog {
-  [NSApp stopModal];
-  [[self window] close];
-}
-
-- (void)runModalDialog {
-  NSWindow* bugReportWindow = [self window];
-  [bugReportWindow center];
-  [NSApp runModalForWindow:bugReportWindow];
-}
-
-- (IBAction)sendReport:(id)sender {
-  if ([self isPhishingReport]) {
-    BugReportUtil::ReportPhishing(currentTab_,
-        pageURL_ ? base::SysNSStringToUTF8(pageURL_) : "");
-  } else {
-    BugReportUtil::SendReport(
-        profile_,
-        [self bugTypeFromIndex],
-        base::SysNSStringToUTF8(pageURL_),
-        base::SysNSStringToUTF8(bugDescription_),
-        sendScreenshot_ && !pngData_.empty() ?
-            reinterpret_cast<const char *>(&(pngData_[0])) : NULL,
-        pngData_.size(), pngWidth_, pngHeight_);
-  }
-  [self closeDialog];
-}
-
-- (IBAction)cancel:(id)sender {
-  [self closeDialog];
-}
-
-- (BOOL)isPhishingReport {
-  return [self bugTypeFromIndex] == BugReportUtil::PHISHING_PAGE;
-}
-
-- (int)bugTypeFromIndex {
-  // The order of these bugs must match the ordering in the bugTypeList_,
-  // and thereby the menu in the popup button in the dialog box.
-  const BugReportUtil::BugType typesForMenuIndices[] = {
-    BugReportUtil::PAGE_WONT_LOAD,
-    BugReportUtil::PAGE_LOOKS_ODD,
-    BugReportUtil::PHISHING_PAGE,
-    BugReportUtil::CANT_SIGN_IN,
-    BugReportUtil::CHROME_MISBEHAVES,
-    BugReportUtil::SOMETHING_MISSING,
-    BugReportUtil::BROWSER_CRASH,
-    BugReportUtil::OTHER_PROBLEM
-  };
-  // The bugs for the shorter menu start at index 4.
-  NSUInteger adjustedBugTypeIndex_ = [bugTypeList_ count] == 8 ? bugTypeIndex_ :
-      bugTypeIndex_ + 4;
-  DCHECK_LT(adjustedBugTypeIndex_, arraysize(typesForMenuIndices));
-  return typesForMenuIndices[adjustedBugTypeIndex_];
-}
-
-// Custom setter to update the UI for different bug types.
-- (void)setBugTypeIndex:(NSUInteger)bugTypeIndex {
-  bugTypeIndex_ = bugTypeIndex;
-
-  // The "send" button's title is based on the type of report.
-  NSString* buttonTitle = [self isPhishingReport] ?
-      l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_SEND_PHISHING_REPORT) :
-      l10n_util::GetNSStringWithFixup(IDS_BUGREPORT_SEND_REPORT);
-  if (![buttonTitle isEqualTo:[sendReportButton_ title]]) {
-    NSRect sendFrame1 = [sendReportButton_ frame];
-    NSRect cancelFrame1 = [cancelButton_ frame];
-
-    [sendReportButton_ setTitle:buttonTitle];
-    CGFloat deltaWidth =
-        [GTMUILocalizerAndLayoutTweaker sizeToFitView:sendReportButton_].width;
-
-    NSRect sendFrame2 = [sendReportButton_ frame];
-    sendFrame2.origin.x -= deltaWidth;
-    NSRect cancelFrame2 = cancelFrame1;
-    cancelFrame2.origin.x -= deltaWidth;
-
-    // Since the buttons get updated/resize, use a quick animation so it is
-    // a little less jarring in the UI.
-    NSDictionary* sendReportButtonResize =
-        [NSDictionary dictionaryWithObjectsAndKeys:
-         sendReportButton_, NSViewAnimationTargetKey,
-         [NSValue valueWithRect:sendFrame1], NSViewAnimationStartFrameKey,
-         [NSValue valueWithRect:sendFrame2], NSViewAnimationEndFrameKey,
-         nil];
-    NSDictionary* cancelButtonResize =
-        [NSDictionary dictionaryWithObjectsAndKeys:
-         cancelButton_, NSViewAnimationTargetKey,
-         [NSValue valueWithRect:cancelFrame1], NSViewAnimationStartFrameKey,
-         [NSValue valueWithRect:cancelFrame2], NSViewAnimationEndFrameKey,
-         nil];
-    NSAnimation* animation =
-        [[[NSViewAnimation alloc] initWithViewAnimations:
-           [NSArray arrayWithObjects:sendReportButtonResize, cancelButtonResize,
-             nil]] autorelease];
-    const NSTimeInterval kQuickTransitionInterval = 0.1;
-    [animation setDuration:kQuickTransitionInterval];
-    [animation startAnimation];
-
-    // Save or reload description when moving between phishing page and other
-    // bug report types.
-    if ([self isPhishingReport]) {
-        saveBugDescription_.reset([[self bugDescription] retain]);
-        [self setBugDescription:nil];
-        saveSendScreenshot_ = sendScreenshot_;
-        [self setSendScreenshot:NO];
-    } else {
-        [self setBugDescription:saveBugDescription_.get()];
-        saveBugDescription_.reset();
-        [self setSendScreenshot:saveSendScreenshot_];
-    }
-  }
-}
-
-- (BOOL)control:(NSControl*)control textView:(NSTextView*)textView
-    doCommandBySelector:(SEL)commandSelector {
-  if (commandSelector == @selector(insertNewline:)) {
-    [textView insertNewlineIgnoringFieldEditor:self];
-    return YES;
-  }
-  return NO;
-}
-
-// BugReportWindowController needs to change the title of the Send Report
-// button when the user chooses the phishing bug type, so we need to bind
-// the function that changes the button title to the bug type key.
-+ (NSSet*)keyPathsForValuesAffectingValueForKey:(NSString*)key {
-  NSSet* paths = [super keyPathsForValuesAffectingValueForKey:key];
-  if ([key isEqualToString:@"isPhishingReport"]) {
-    paths = [paths setByAddingObject:@"bugTypeIndex"];
-  }
-  return paths;
-}
-
-@end
-
diff --git a/chrome/browser/ui/cocoa/bug_report_window_controller_unittest.mm b/chrome/browser/ui/cocoa/bug_report_window_controller_unittest.mm
deleted file mode 100644
index 22632fd..0000000
--- a/chrome/browser/ui/cocoa/bug_report_window_controller_unittest.mm
+++ /dev/null
@@ -1,78 +0,0 @@
-// 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.
-
-#import <Cocoa/Cocoa.h>
-
-#include "base/memory/ref_counted.h"
-#include "chrome/browser/profiles/profile.h"
-#import "chrome/browser/ui/cocoa/bug_report_window_controller.h"
-#include "chrome/test/base/testing_profile.h"
-#include "content/browser/browser_thread.h"
-#include "content/browser/renderer_host/test_render_view_host.h"
-#include "content/browser/site_instance.h"
-#include "content/browser/tab_contents/test_tab_contents.h"
-#import "testing/gtest_mac.h"
-
-namespace {
-
-class BugReportWindowControllerUnittest : public RenderViewHostTestHarness {
-};
-
-// See http://crbug.com/29019 for why it's disabled.
-TEST_F(BugReportWindowControllerUnittest, DISABLED_ReportBugWithNewTabPageOpen) {
-  BrowserThread ui_thread(BrowserThread::UI, MessageLoop::current());
-  // Create a "chrome://newtab" test tab.  SiteInstance will be deleted when
-  // tabContents is deleted.
-  SiteInstance* instance =
-      SiteInstance::CreateSiteInstance(profile_.get());
-  TestTabContents* tabContents = new TestTabContents(profile_.get(),
-                                                      instance);
-  tabContents->controller().LoadURL(GURL("chrome://newtab"),
-      GURL(), PageTransition::START_PAGE);
-
-  BugReportWindowController* controller = [[BugReportWindowController alloc]
-      initWithTabContents:tabContents
-                  profile:profile_.get()];
-
-  // The phishing report bug is stored at index 2 in the Report Bug dialog.
-  [controller setBugTypeIndex:2];
-  EXPECT_TRUE([controller isPhishingReport]);
-  [controller setBugTypeIndex:1];
-  EXPECT_FALSE([controller isPhishingReport]);
-
-  // Make sure that the tab was correctly recorded.
-  EXPECT_NSEQ(@"chrome://newtab/", [controller pageURL]);
-  EXPECT_NSEQ(@"New Tab", [controller pageTitle]);
-
-  // When we call "report bug" with non-empty tab contents, all menu options
-  // should be available, and we should send screenshot by default.
-  EXPECT_EQ([[controller bugTypeList] count], 8U);
-  EXPECT_TRUE([controller sendScreenshot]);
-
-  delete tabContents;
-  [controller release];
-}
-
-// See http://crbug.com/29019 for why it's disabled.
-TEST_F(BugReportWindowControllerUnittest, DISABLED_ReportBugWithNoWindowOpen) {
-  BugReportWindowController* controller = [[BugReportWindowController alloc]
-      initWithTabContents:NULL
-                  profile:profile_.get()];
-
-  // Make sure that no page title or URL are recorded. Note that IB reports
-  // empty textfields as NULL values.
-  EXPECT_FALSE([controller pageURL]);
-  EXPECT_FALSE([controller pageTitle]);
-
-  // When we call "report bug" with empty tab contents, only menu options
-  // that don't refer to a specific page should be available, and the send
-  // screenshot option should be turned off.
-  EXPECT_EQ([[controller bugTypeList] count], 4U);
-  EXPECT_FALSE([controller sendScreenshot]);
-
-  [controller release];
-}
-
-}  // namespace
-
diff --git a/chrome/browser/ui/webui/bug_report_ui.cc b/chrome/browser/ui/webui/bug_report_ui.cc
index c5df51a..93c12bd 100644
--- a/chrome/browser/ui/webui/bug_report_ui.cc
+++ b/chrome/browser/ui/webui/bug_report_ui.cc
@@ -53,9 +53,7 @@ const char kScreenshotBaseUrl[] = "chrome://screenshots/";
 const char kCurrentScreenshotUrl[] = "chrome://screenshots/current";
 #if defined(OS_CHROMEOS)
 const char kSavedScreenshotsUrl[] = "chrome://screenshots/saved/";
-
-const char kScreenshotPattern[] = "*.png";
-const char kScreenshotsRelativePath[] = "/Screenshots";
+const char kScreenshotPattern[] = "screenshot-*.png";
 
 const size_t kMaxSavedScreenshots = 2;
 #endif
@@ -73,11 +71,7 @@ void GetSavedScreenshots(std::vector<std::string>* saved_screenshots,
     return;
   }
 
-  // TODO(rkc): Change this to use FilePath.Append() once the cros
-  // issue with it is fixed
-  FilePath screenshots_path(fileshelf_path.value() +
-                            std::string(kScreenshotsRelativePath));
-  file_util::FileEnumerator screenshots(screenshots_path, false,
+  file_util::FileEnumerator screenshots(fileshelf_path, false,
                                         file_util::FileEnumerator::FILES,
                                         std::string(kScreenshotPattern));
   FilePath screenshot = screenshots.Next();
@@ -513,9 +507,9 @@ void BugReportHandler::HandleSendReport(const ListValue* list_value) {
   screenshot_path.erase(0, strlen(kScreenshotBaseUrl));
 
   // Get the image to send in the report.
-  std::vector<unsigned char> image;
+  ScreenshotDataPtr image_ptr;
   if (!screenshot_path.empty())
-    image = screenshot_source_->GetScreenshot(screenshot_path);
+    image_ptr = screenshot_source_->GetCachedScreenshot(screenshot_path);
 
 #if defined(OS_CHROMEOS)
   if (++i == list_value->end()) {
@@ -547,7 +541,7 @@ void BugReportHandler::HandleSendReport(const ListValue* list_value) {
                                , problem_type
                                , page_url
                                , description
-                               , image
+                               , image_ptr
 #if defined(OS_CHROMEOS)
                                , user_email
                                , send_sys_info
diff --git a/chrome/browser/ui/webui/screenshot_source.cc b/chrome/browser/ui/webui/screenshot_source.cc
index b8a21bb..32059b8 100644
--- a/chrome/browser/ui/webui/screenshot_source.cc
+++ b/chrome/browser/ui/webui/screenshot_source.cc
@@ -4,115 +4,111 @@
 
 #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"
 #include "content/browser/browser_thread.h"
 
-static const char kCurrentScreenshot[] = "current";
+static const char kCurrentScreenshotFilename[] = "current";
 #if defined(OS_CHROMEOS)
-static const char kSavedScreenshots[] = "saved/";
+static const char kSavedScreenshotsBasePath[] = "saved/";
 #endif
 
-static const char kScreenshotsRelativePath[] = "/Screenshots/";
+ScreenshotSource::ScreenshotSource(
+    std::vector<unsigned char>* current_screenshot)
+    : DataSource(chrome::kChromeUIScreenshotPath, MessageLoop::current()) {
+  // Setup the last screenshot taken.
+  if (current_screenshot)
+    current_screenshot_.reset(new ScreenshotData(*current_screenshot));
+  else
+    current_screenshot_.reset(new ScreenshotData());
+}
 
-#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();
+ScreenshotSource::~ScreenshotSource() {}
 
-  FilePath fileshelf_path;
-  if (!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &fileshelf_path)) {
-    read_complete->Signal();
-    return;
-  }
+void ScreenshotSource::StartDataRequest(const std::string& path, bool,
+                                        int request_id) {
+  SendScreenshot(path, request_id);
+}
 
-  FilePath file(fileshelf_path.value() + std::string(kScreenshotsRelativePath) +
-                filename);
+std::string ScreenshotSource::GetMimeType(const std::string&) const {
+  // We need to explicitly return a mime type, otherwise if the user tries to
+  // drag the image they get no extension.
+  return "image/png";
+}
 
-  int64 file_size = 0;
-  if (!file_util::GetFileSize(file, &file_size)) {
-    read_complete->Signal();
-    return;
+ScreenshotDataPtr ScreenshotSource::GetCachedScreenshot(
+    const std::string& screenshot_path) {
+  std::map<std::string, ScreenshotDataPtr>::iterator pos;
+  std::string path = screenshot_path.substr(
+      0, screenshot_path.find_first_of("?"));
+  if ((pos = cached_screenshots_.find(path)) != cached_screenshots_.end()) {
+    return pos->second;
+  } else {
+    return ScreenshotDataPtr(new ScreenshotData);
   }
-
-  // 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();
 }
 
-// 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;
-}
-#endif
-
-std::vector<unsigned char> ScreenshotSource::GetScreenshot(
-    const std::string& full_path) {
+void ScreenshotSource::SendScreenshot(const std::string& screenshot_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_;
+  std::string path = screenshot_path.substr(
+      0, screenshot_path.find_first_of("?"));
+  if (path == kCurrentScreenshotFilename) {
+    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);
+  } else if (path.compare(0, strlen(kSavedScreenshotsBasePath),
+                          kSavedScreenshotsBasePath) == 0) {
+    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, ScreenshotDataPtr(new ScreenshotData()));
   }
 }
 
-ScreenshotSource::ScreenshotSource(
-    std::vector<unsigned char>* current_screenshot)
-    : DataSource(chrome::kChromeUIScreenshotPath, MessageLoop::current()) {
-  // Setup the last screenshot taken.
-  if (current_screenshot)
-    current_screenshot_ = *current_screenshot;
-  else
-    current_screenshot_.clear();
-}
+#if defined(OS_CHROMEOS)
+void ScreenshotSource::SendSavedScreenshot(const std::string& screenshot_path,
+                                           int request_id) {
+  ScreenshotDataPtr read_bytes(new ScreenshotData);
+  std::string filename = screenshot_path.substr(
+      strlen(kSavedScreenshotsBasePath));
 
-ScreenshotSource::~ScreenshotSource() {}
+  FilePath fileshelf_path;
+  if (!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &fileshelf_path)) {
+    CacheAndSendScreenshot(screenshot_path, request_id, read_bytes);
+    return;
+  }
+
+  int64 file_size = 0;
+  FilePath file = fileshelf_path.Append(filename);
+  if (!file_util::GetFileSize(file, &file_size)) {
+    CacheAndSendScreenshot(screenshot_path, request_id, read_bytes);
+    return;
+  }
+
+  read_bytes->resize(file_size);
+  if (!file_util::ReadFile(file, reinterpret_cast<char*>(&read_bytes->front()),
+                           static_cast<int>(file_size)))
+    read_bytes->clear();
 
-void ScreenshotSource::StartDataRequest(const std::string& path,
-                                            bool is_incognito,
-                                            int request_id) {
-  SendResponse(request_id, new RefCountedBytes(GetScreenshot(path)));
+  CacheAndSendScreenshot(screenshot_path, request_id, read_bytes);
 }
+#endif
 
-std::string ScreenshotSource::GetMimeType(const std::string&) const {
-  // We need to explicitly return a mime type, otherwise if the user tries to
-  // drag the image they get no extension.
-  return "image/png";
+void ScreenshotSource::CacheAndSendScreenshot(
+    const std::string& screenshot_path,
+    int request_id,
+    ScreenshotDataPtr bytes) {
+  cached_screenshots_[screenshot_path] = bytes;
+  SendResponse(request_id, new RefCountedBytes(*bytes));
 }
diff --git a/chrome/browser/ui/webui/screenshot_source.h b/chrome/browser/ui/webui/screenshot_source.h
index 4b071d3..3bfac8a 100644
--- a/chrome/browser/ui/webui/screenshot_source.h
+++ b/chrome/browser/ui/webui/screenshot_source.h
@@ -5,14 +5,19 @@
 #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
+// or current) to the bug report html ui.
 class ScreenshotSource : public ChromeURLDataManager::DataSource {
  public:
   explicit ScreenshotSource(
@@ -26,12 +31,36 @@ class ScreenshotSource : public ChromeURLDataManager::DataSource {
 
   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
+  // from a previous request to the screenshots source.
+  // Note: This method strips the query string from the given path.
+  ScreenshotDataPtr GetCachedScreenshot(const std::string& screenshot_path);
 
  private:
   virtual ~ScreenshotSource();
 
-  std::vector<unsigned char> current_screenshot_;
+  // Send the screenshot specified by the given relative path to the requestor.
+  // This is the ancestor for SendSavedScreenshot and CacheAndSendScreenshot.
+  // All calls to send a screenshot should only call this method.
+  // Note: This method strips the query string from the given path.
+  void SendScreenshot(const std::string& screenshot_path, int request_id);
+#if defined(OS_CHROMEOS)
+  // Send a saved screenshot image file specified by the given screenshot path
+  // to the requestor.
+  void SendSavedScreenshot(const std::string& screenshot_path, int request_id);
+#endif
+  // Sends the screenshot data to the requestor while caching it locally to the
+  // class instance, indexed by path.
+  void CacheAndSendScreenshot(const std::string& screenshot_path,
+                              int request_id,
+                              ScreenshotDataPtr bytes);
+
+  // Pointer to the screenshot data for the current screenshot.
+  ScreenshotDataPtr current_screenshot_;
+
+  // 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);
 };
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 2c39a0f..81cca7c 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2379,8 +2379,6 @@
 	'browser/ui/cocoa/browser_window_utils.mm',
         'browser/ui/cocoa/bubble_view.h',
         'browser/ui/cocoa/bubble_view.mm',
-        'browser/ui/cocoa/bug_report_window_controller.h',
-        'browser/ui/cocoa/bug_report_window_controller.mm',
         'browser/ui/cocoa/certificate_viewer.mm',
         'browser/ui/cocoa/chrome_browser_window.h',
         'browser/ui/cocoa/chrome_browser_window.mm',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index fe7f1ba..0bff73f 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1747,7 +1747,6 @@
         'browser/ui/cocoa/browser_window_cocoa_unittest.mm',
         'browser/ui/cocoa/browser_window_controller_unittest.mm',
         'browser/ui/cocoa/bubble_view_unittest.mm',
-        'browser/ui/cocoa/bug_report_window_controller_unittest.mm',
         'browser/ui/cocoa/chrome_browser_window_unittest.mm',
         'browser/ui/cocoa/chrome_event_processing_window_unittest.mm',
         'browser/ui/cocoa/clickhold_button_cell_unittest.mm',