From 3973de072a4da1a688847e6d7a4b161be34b6633 Mon Sep 17 00:00:00 2001 From: "rkc@chromium.org" Date: Thu, 2 Dec 2010 20:08:57 +0000 Subject: 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 --- chrome/app/generated_resources.grd | 10 ++-- chrome/browser/dom_ui/bug_report_ui.cc | 79 ++++++++++++++++++++++---- chrome/browser/dom_ui/bug_report_ui.h | 21 +++++++ chrome/browser/gtk/browser_window_gtk.cc | 3 +- chrome/browser/resources/bug_report.html | 12 ++-- chrome/browser/ui/toolbar/wrench_menu_model.cc | 2 +- chrome/browser/ui/views/browser_dialogs.h | 3 - 7 files changed, 104 insertions(+), 26 deletions(-) (limited to 'chrome') 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 - &Report bug or broken website... + &Report an Issue... &Developer tools @@ -945,7 +945,7 @@ each locale. --> View S&ource - &Report Bug or Broken Website... + &Report an Issue... &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 - Report a bug or an issue + Report an Issue 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. - 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. 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 - Report Bug or Broken Website... + Report an Issue... $1Google Chrome 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* 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; #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); + -
@@ -199,7 +201,6 @@ window.addEventListener('DOMContentLoaded', load);
- @@ -219,6 +220,7 @@ window.addEventListener('DOMContentLoaded', load); + 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); -- cgit v1.1