Crash fix + OS specific ss's enabled.

Fixes a crash in which if a feedback tab is open and a user opens it from a different tab again, feedback will crash on sending a report. This is due to browser_navigator; this change decouples us from the Singleton tab logic and adds our own.

Additionally, code to take screenshots on other platforms is also added.

BUG=64971,61847
TEST=Tested by opening feedback from multiple tabs; sent reports successfully. Additionally tested with sending screenshots from Linux.

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

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

7 files changed, 104 insertions, 26 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 31bb302..5c09d03 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -919,7 +919,7 @@ each locale. -->
           View s&ource
         </message>
         <message name="IDS_FEEDBACK" desc="The text label of the Report Bug menu item">
-          &amp;Report bug or broken website...
+          &amp;Report an Issue...
         </message>
         <message name="IDS_DEV_TOOLS" desc="The text label of the Developer Tools menu item">
           &amp;Developer tools
@@ -945,7 +945,7 @@ each locale. -->
           View S&amp;ource
         </message>
         <message name="IDS_FEEDBACK" desc="In Title Case: The text label of the Report Bug menu item">
-          &amp;Report Bug or Broken Website...
+          &amp;Report an Issue...
         </message>
         <message name="IDS_DEV_TOOLS" desc="In Title Case: The text label of the Developer Tools menu item">
           &amp;Developer Tools
@@ -4509,7 +4509,7 @@ Keep your key file in a safe place. You will need it to create new versions of y
         Reload this page
       </message>
       <message name="IDS_TOOLTIP_FEEDBACK" desc="Tooltip for the feedback button.">
-        Report a bug or an issue
+        Report an Issue
       </message>
       <message name="IDS_TOOLTIP_STOP" desc="The tooltip for the stop button">
         Stop loading this page
@@ -4918,7 +4918,7 @@ Keep your key file in a safe place. You will need it to create new versions of y
         Please tell us what is happening before sending the feedback.
       </message>
       <message name="IDS_BUGREPORT_NO_SAVED_SCREENSHOTS_HELP" desc="Message shown when no screenshots are available for selection">
-        You can select saved screenshots from here. Currently there are no screenshots available. You can press Ctrl + the "Overview Mode" key together to take a screenshot. The last three screenshots that you have taken will appear here. 
+        You can select saved screenshots from here. Currently there are no screenshots available. You can press Ctrl + the "Overview Mode" key together to take a screenshot. The last three screenshots that you have taken will appear here.
       </message>
       <message name="IDS_BUGREPORT_ISSUE_WITH" desc="Label for issue with">
         Where are you having problems? (required)
@@ -8861,7 +8861,7 @@ Keep your key file in a safe place. You will need it to create new versions of y
         </message>
         <!-- Help menu -->
         <message name="IDS_FEEDBACK_MAC" desc="The Mac menu item to report a bug in the Help menu.">
-          Report Bug or Broken Website...
+          Report an Issue...
         </message>
         <message name="IDS_HELP_MAC" desc="The Mac menu item to show help in the Help menu.">
           <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> Help
diff --git a/chrome/browser/dom_ui/bug_report_ui.cc b/chrome/browser/dom_ui/bug_report_ui.cc
index d3e2797..74ac629 100644
--- a/chrome/browser/dom_ui/bug_report_ui.cc
+++ b/chrome/browser/dom_ui/bug_report_ui.cc
@@ -116,8 +116,20 @@ std::string GetUserEmail() {
   else
     return manager->logged_in_user().email();
 }
-
 #endif
+
+// Returns the index of the feedback tab if already open, -1 otherwise
+int GetIndexOfFeedbackTab(Browser* browser) {
+  GURL bug_report_url(chrome::kChromeUIBugReportURL);
+  for (int i = 0; i < browser->tab_count(); ++i) {
+    TabContents* tab = browser->GetTabContentsAt(i);
+    if (tab && tab->GetURL().GetWithEmptyPath() == bug_report_url)
+      return i;
+  }
+
+  return -1;
+}
+
 }  // namespace
 
 
@@ -127,7 +139,27 @@ namespace browser {
 std::vector<unsigned char>* last_screenshot_png = 0;
 gfx::Rect screen_size;
 
+// Get bounds in different ways for different OS's;
+#if defined(TOOLKIT_VIEWS)
+// Windows/ChromeOS support Views - so we get dimensions from the
+// views::Window object
 void RefreshLastScreenshot(views::Window* parent) {
+  gfx::NativeWindow window = parent->GetNativeWindow();
+  int width = parent->GetBounds().width();
+  int height = parent->GetBounds().height();
+#elif defined(OS_LINUX)
+// Linux provides its bounds and a native window handle to the screen
+void RefreshLastScreenshot(gfx::NativeWindow window,
+                           const gfx::Rect& bounds) {
+  int width = bounds.width();
+  int height = bounds.height();
+#elif defined(OS_MACOSX)
+// Mac gets its bounds from the GrabWindowSnapshot function
+void RefreshLastScreenshot(NSWindow* window) {
+  int width = 0;
+  int height = 0;
+#endif
+
   // Grab an exact snapshot of the window that the user is seeing (i.e. as
   // rendered--do not re-render, and include windowed plugins).
   if (last_screenshot_png)
@@ -136,24 +168,47 @@ void RefreshLastScreenshot(views::Window* parent) {
     last_screenshot_png = new std::vector<unsigned char>;
 
 #if defined(USE_X11)
-  screen_size = parent->GetBounds();
-  x11_util::GrabWindowSnapshot(parent->GetNativeWindow(), last_screenshot_png);
+  x11_util::GrabWindowSnapshot(window, last_screenshot_png);
 #elif defined(OS_MACOSX)
-  int width = 0, height = 0;
-  mac_util::GrabWindowSnapshot(parent->GetNativeWindow(), last_screenshot_png,
-                               &width, &height);
+  mac_util::GrabWindowSnapshot(window, last_screenshot_png, &width, &height);
 #elif defined(OS_WIN)
-  screen_size = parent->GetBounds();
-  win_util::GrabWindowSnapshot(parent->GetNativeWindow(), last_screenshot_png);
+  win_util::GrabWindowSnapshot(window, last_screenshot_png);
 #endif
+
+  screen_size.set_width(width);
+  screen_size.set_height(height);
 }
 
-// Global "display this dialog" function declared in browser_dialogs.h.
+#if defined(TOOLKIT_VIEWS)
 void ShowHtmlBugReportView(views::Window* parent, Browser* browser) {
-  std::string bug_report_url = std::string(chrome::kChromeUIBugReportURL) +
-      "#" + base::IntToString(browser->selected_index());
+#elif defined(OS_LINUX)
+void ShowHtmlBugReportView(gfx::NativeWindow window, const gfx::Rect& bounds,
+                           Browser* browser) {
+#elif defined(OS_MACOSX)
+void ShowHtmlBugReportView(NSWindow* window, Browser* browser) {
+#endif
+
+  // First check if we're already open (we cannot depend on ShowSingletonTab
+  // for this functionality since we need to make *sure* we never get
+  // instantiated again while we are open - with singleton tabs, that can
+  // happen)
+  int feedback_tab_index = GetIndexOfFeedbackTab(browser);
+  if (feedback_tab_index >=0) {
+    // Do not refresh screenshot, do not create a new tab
+    browser->SelectTabContentsAt(feedback_tab_index, true);
+  }
 
+  // now for refreshing the last screenshot
+#if defined(TOOLKIT_VIEWS)
   RefreshLastScreenshot(parent);
+#elif defined(OS_LINUX)
+  RefreshLastScreenshot(window, bounds);
+#elif defined(OS_MACOSX)
+  RefreshLastScreenshot(window);
+#endif
+
+  std::string bug_report_url = std::string(chrome::kChromeUIBugReportURL) +
+      "#" + base::IntToString(browser->selected_index());
   browser->ShowSingletonTab(GURL(bug_report_url), false);
 }
 
@@ -662,6 +717,7 @@ void BugReportHandler::HandleSendReport(const ListValue* list_value) {
   // If we aren't sending the sys_info, cancel the gathering of the syslogs.
   if (!send_sys_info)
     CancelFeedbackCollection();
+#endif
 
   // Update the data in bug_report_ so it can be sent
   bug_report_->UpdateData(dom_ui_->GetProfile()
@@ -678,6 +734,7 @@ void BugReportHandler::HandleSendReport(const ListValue* list_value) {
 #endif
                           );
 
+#if defined(OS_CHROMEOS)
   // If we don't require sys_info, or we have it, or we never requested it
   // (because libcros failed to load), then send the report now.
   // Otherwise, the report will get sent when we receive sys_info.
diff --git a/chrome/browser/dom_ui/bug_report_ui.h b/chrome/browser/dom_ui/bug_report_ui.h
index 907dbb2..31d9eae 100644
--- a/chrome/browser/dom_ui/bug_report_ui.h
+++ b/chrome/browser/dom_ui/bug_report_ui.h
@@ -6,8 +6,29 @@
 #define CHROME_BROWSER_DOM_UI_BUG_REPORT_UI_H_
 
 #include "chrome/browser/dom_ui/html_dialog_ui.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/views/window.h"
+
+namespace gfx {
+class Rect;
+}  // namespace gfx
 
 class TabContents;
+class NSWindow;
+
+
+// TODO(rkc): The following code is very ugly and needs to be refactored.
+// http://code.google.com/p/chromium/issues/detail?id=65119
+namespace browser {
+#if defined(TOOLKIT_VIEWS)
+void ShowHtmlBugReportView(views::Window* parent, Browser* browser);
+#elif defined(OS_LINUX)
+void ShowHtmlBugReportView(gfx::NativeWindow window, const gfx::Rect& bounds,
+                           Browser* browser);
+#elif defined(OS_MACOSX)
+void ShowHtmlBugReportView(NSWindow* window, Browser* browser);
+#endif
+}  // namespace browser
 
 class BugReportUI : public HtmlDialogUI {
  public:
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index ca6237b..d847308 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -26,6 +26,7 @@
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/debugger/devtools_window.h"
+#include "chrome/browser/dom_ui/bug_report_ui.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/gtk/about_chrome_dialog.h"
@@ -916,7 +917,7 @@ DownloadShelf* BrowserWindowGtk::GetDownloadShelf() {
 }
 
 void BrowserWindowGtk::ShowReportBugDialog() {
-  NOTIMPLEMENTED();
+  browser::ShowHtmlBugReportView(window_, bounds_, browser_.get());
 }
 
 void BrowserWindowGtk::ShowClearBrowsingDataDialog() {
diff --git a/chrome/browser/resources/bug_report.html b/chrome/browser/resources/bug_report.html
index 3d3d058..4b45cee 100644
--- a/chrome/browser/resources/bug_report.html
+++ b/chrome/browser/resources/bug_report.html
@@ -19,9 +19,11 @@
  * Window onload handler, sets up the page.

  */

 function load() {

-  $('sysinfo-url').onclick = function(event) {

-    chrome.send('openSystemTab');

-  };

+  if ($('sysinfo-url')) {

+    $('sysinfo-url').onclick = function(event) {

+      chrome.send('openSystemTab');

+    };

+  }
   

   var menuOffPattern = /(^\?|&)menu=off($|&)/;
   var menuDisabled = menuOffPattern.test(window.location.search);
@@ -181,10 +183,10 @@ window.addEventListener('DOMContentLoaded', load);
                       </table>

                     </td>

                   </tr>

+<if expr="pp_ifdef('chromeos')">

                   <!--  User e-mail -->

                   <tr>

                     <td>

-<if expr="pp_ifdef('chromeos')">

                       <table id="user-email-table" class="bug-report-table">

                         <tr>

                           <td class="bug-report-fieldlabel">

@@ -199,7 +201,6 @@ window.addEventListener('DOMContentLoaded', load);
                           </td>

                         </tr>

                       </table>

-</if>                        

                     </td>

                   </tr>

                   <!--  System Information -->

@@ -219,6 +220,7 @@ window.addEventListener('DOMContentLoaded', load);
                       </table>

                     </td>

                   </tr>

+</if>

                   <!--  Screenshot -->

                   <tr>

                     <td>

diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.cc b/chrome/browser/ui/toolbar/wrench_menu_model.cc
index 6c83a00..c963685 100644
--- a/chrome/browser/ui/toolbar/wrench_menu_model.cc
+++ b/chrome/browser/ui/toolbar/wrench_menu_model.cc
@@ -188,7 +188,7 @@ void ToolsMenuModel::Build(Browser* browser) {
   AddItemWithStringId(IDC_CLEAR_BROWSING_DATA, IDS_CLEAR_BROWSING_DATA);
 
   AddSeparator();
-#if defined(OS_CHROMEOS)
+#if defined(OS_CHROMEOS) || defined(OS_WIN) || defined(OS_LINUX)
   AddItemWithStringId(IDC_FEEDBACK, IDS_FEEDBACK);
   AddSeparator();
 #endif
diff --git a/chrome/browser/ui/views/browser_dialogs.h b/chrome/browser/ui/views/browser_dialogs.h
index f49cc2a..50c0bbc 100644
--- a/chrome/browser/ui/views/browser_dialogs.h
+++ b/chrome/browser/ui/views/browser_dialogs.h
@@ -46,9 +46,6 @@ void ShowBugReportView(views::Window* parent,
                        Profile* profile,
                        TabContents* tab);
 
-// Shows the "Report a problem with this page" page in a new tab
-void ShowHtmlBugReportView(views::Window* parent, Browser* browser);
-
 // Shows the "Clear browsing data" dialog box. See ClearBrowsingDataView.
 void ShowClearBrowsingDataView(gfx::NativeWindow parent,
                                Profile* profile);