Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(894)

Unified Diff: chrome/browser/ui/views/sad_tab_view.cc

Issue 2261793002: Bring the feedback button to the Mac sad tab (Closed) Base URL: https://chromium.googlesource.com/chromium/src@master
Patch Set: Address second wave of comments, turn off feedback in non-Chrome builds Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/ui/views/sad_tab_view.h ('k') | chrome/test/BUILD.gn » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/views/sad_tab_view.cc
diff --git a/chrome/browser/ui/views/sad_tab_view.cc b/chrome/browser/ui/views/sad_tab_view.cc
index 5b27d3a9698fa1310afe54c2cdb5b2b6f18a3eb2..7da282d934478bc89a7a40ebea7d8eb273de7074 100644
--- a/chrome/browser/ui/views/sad_tab_view.cc
+++ b/chrome/browser/ui/views/sad_tab_view.cc
@@ -8,13 +8,6 @@
#include "base/metrics/histogram_macros.h"
#include "build/build_config.h"
-#include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/chrome_pages.h"
-#include "chrome/common/url_constants.h"
-#include "chrome/grit/generated_resources.h"
-#include "components/feedback/feedback_util.h"
-#include "components/strings/grit/components_strings.h"
-#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/web_contents.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
@@ -32,98 +25,16 @@
#include "ui/views/layout/layout_constants.h"
#include "ui/views/widget/widget.h"
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/memory/oom_memory_details.h"
-#endif
-
-using content::OpenURLParams;
-using content::WebContents;
-
namespace {
const int kMaxContentWidth = 600;
const int kMinColumnWidth = 120;
-const char kCategoryTagCrash[] = "Crash";
-const int kCrashesBeforeFeedbackIsDisplayed = 1;
-
-void RecordKillCreated() {
- static int killed = 0;
- killed++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.KillCreated", killed);
-}
-
-void RecordKillDisplayed() {
- static int killed = 0;
- killed++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.KillDisplayed", killed);
-}
-
-#if defined(OS_CHROMEOS)
-void RecordKillCreatedOOM() {
- static int oom_killed = 0;
- oom_killed++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.KillCreated.OOM", oom_killed);
-}
-
-void RecordKillDisplayedOOM() {
- static int oom_killed = 0;
- oom_killed++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.KillDisplayed.OOM", oom_killed);
-}
-#endif
} // namespace
-int SadTabView::total_crashes_ = 0;
-
-SadTabView::SadTabView(WebContents* web_contents, chrome::SadTabKind kind)
- : web_contents_(web_contents),
- kind_(kind),
- painted_(false),
- message_(nullptr),
- help_link_(nullptr),
- action_button_(nullptr),
- title_(nullptr),
- help_message_(nullptr) {
- DCHECK(web_contents);
-
- // These stats should use the same counting approach and bucket size used for
- // tab discard events in memory::OomPriorityManager so they can be directly
- // compared.
- // TODO(jamescook): Maybe track time between sad tabs?
- total_crashes_++;
-
- switch (kind_) {
- case chrome::SAD_TAB_KIND_CRASHED: {
- static int crashed = 0;
- crashed++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.CrashCreated", crashed);
- break;
- }
- case chrome::SAD_TAB_KIND_KILLED: {
- RecordKillCreated();
- LOG(WARNING) << "Tab Killed: "
- << web_contents->GetURL().GetOrigin().spec();
- break;
- }
- case chrome::SAD_TAB_KIND_OOM: {
- static int crashed_due_to_oom = 0;
- crashed_due_to_oom++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.OomCreated", crashed_due_to_oom);
- break;
- }
-#if defined(OS_CHROMEOS)
- case chrome::SAD_TAB_KIND_KILLED_BY_OOM: {
- RecordKillCreated();
- RecordKillCreatedOOM();
- const std::string spec = web_contents->GetURL().GetOrigin().spec();
- memory::OomMemoryDetails::Log(
- "Tab OOM-Killed Memory details: " + spec + ", ", base::Closure());
- break;
- }
-#endif
- }
-
+SadTabView::SadTabView(content::WebContents* web_contents,
+ chrome::SadTabKind kind)
+ : SadTab(web_contents, kind) {
// Set the background color.
set_background(
views::Background::CreateSolidBackground(GetNativeTheme()->GetSystemColor(
@@ -149,7 +60,7 @@ SadTabView::SadTabView(WebContents* web_contents, chrome::SadTabKind kind)
layout->StartRow(0, column_set_id);
layout->AddView(image, 2, 1);
- title_ = CreateLabel(l10n_util::GetStringUTF16(IDS_SAD_TAB_TITLE));
+ title_ = CreateLabel(l10n_util::GetStringUTF16(GetTitle()));
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
title_->SetFontList(rb.GetFontList(ui::ResourceBundle::LargeFont));
title_->SetMultiLine(true);
@@ -161,15 +72,7 @@ SadTabView::SadTabView(WebContents* web_contents, chrome::SadTabKind kind)
const SkColor text_color = GetNativeTheme()->GetSystemColor(
ui::NativeTheme::kColorId_LabelDisabledColor);
- int message_id = IDS_SAD_TAB_MESSAGE;
-#if defined(OS_CHROMEOS)
- if (kind_ == chrome::SAD_TAB_KIND_KILLED_BY_OOM)
- message_id = IDS_KILLED_TAB_BY_OOM_MESSAGE;
-#endif
- if (kind_ == chrome::SAD_TAB_KIND_OOM)
- message_id = IDS_SAD_TAB_OOM_MESSAGE;
-
- message_ = CreateLabel(l10n_util::GetStringUTF16(message_id));
+ message_ = CreateLabel(l10n_util::GetStringUTF16(GetMessage()));
message_->SetMultiLine(true);
message_->SetEnabledColor(text_color);
@@ -180,54 +83,52 @@ SadTabView::SadTabView(WebContents* web_contents, chrome::SadTabKind kind)
layout->AddView(message_, 2, 1, views::GridLayout::LEADING,
views::GridLayout::LEADING);
- if (web_contents_) {
- // In the cases of multiple crashes in a session the 'Feedback' button
- // replaces the 'Reload' button as primary action.
- int button_type = total_crashes_ > kCrashesBeforeFeedbackIsDisplayed ?
- SAD_TAB_BUTTON_FEEDBACK : SAD_TAB_BUTTON_RELOAD;
- action_button_ = views::MdTextButton::CreateSecondaryUiBlueButton(this,
- l10n_util::GetStringUTF16(button_type == SAD_TAB_BUTTON_FEEDBACK
- ? IDS_CRASHED_TAB_FEEDBACK_LINK
- : IDS_SAD_TAB_RELOAD_LABEL));
- action_button_->set_tag(button_type);
- help_link_ =
- CreateLink(l10n_util::GetStringUTF16(IDS_LEARN_MORE), text_color);
- layout->StartRowWithPadding(0, column_set_id, 0,
- views::kPanelVerticalSpacing);
- layout->AddView(help_link_, 1, 1, views::GridLayout::LEADING,
- views::GridLayout::CENTER);
- layout->AddView(action_button_, 1, 1, views::GridLayout::TRAILING,
- views::GridLayout::LEADING);
- }
+ action_button_ = views::MdTextButton::CreateSecondaryUiBlueButton(
+ this, l10n_util::GetStringUTF16(GetButtonTitle()));
+ help_link_ =
+ CreateLink(l10n_util::GetStringUTF16(GetHelpLinkTitle()), text_color);
+ layout->StartRowWithPadding(0, column_set_id, 0,
+ views::kPanelVerticalSpacing);
+ layout->AddView(help_link_, 1, 1, views::GridLayout::LEADING,
+ views::GridLayout::CENTER);
+ layout->AddView(action_button_, 1, 1, views::GridLayout::TRAILING,
+ views::GridLayout::LEADING);
+
layout->AddPaddingRow(2, views::kPanelSubVerticalSpacing);
+
+ views::Widget::InitParams sad_tab_params(
+ views::Widget::InitParams::TYPE_CONTROL);
+
+ // It is not possible to create a native_widget_win that has no parent in
+ // and later re-parent it.
+ // TODO(avi): This is a cheat. Can this be made cleaner?
+ sad_tab_params.parent = web_contents->GetNativeView();
+
+ set_owned_by_client();
+
+ views::Widget* sad_tab = new views::Widget;
+ sad_tab->Init(sad_tab_params);
+ sad_tab->SetContentsView(this);
+
+ views::Widget::ReparentNativeView(sad_tab->GetNativeView(),
+ web_contents->GetNativeView());
+ gfx::Rect bounds = web_contents->GetContainerBounds();
+ sad_tab->SetBounds(gfx::Rect(bounds.size()));
}
-SadTabView::~SadTabView() {}
+SadTabView::~SadTabView() {
+ if (GetWidget())
+ GetWidget()->Close();
+}
void SadTabView::LinkClicked(views::Link* source, int event_flags) {
- DCHECK(web_contents_);
- OpenURLParams params(GURL(total_crashes_ > kCrashesBeforeFeedbackIsDisplayed
- ? chrome::kCrashReasonFeedbackDisplayedURL
- : chrome::kCrashReasonURL),
- content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
- ui::PAGE_TRANSITION_LINK, false);
- web_contents_->OpenURL(params);
+ PerformAction(Action::HELP_LINK);
}
void SadTabView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
- DCHECK(web_contents_);
DCHECK_EQ(action_button_, sender);
-
- if (action_button_->tag() == SAD_TAB_BUTTON_FEEDBACK) {
- chrome::ShowFeedbackPage(
- chrome::FindBrowserWithWebContents(web_contents_),
- l10n_util::GetStringUTF8(kind_ == chrome::SAD_TAB_KIND_CRASHED ?
- IDS_CRASHED_TAB_FEEDBACK_MESSAGE : IDS_KILLED_TAB_FEEDBACK_MESSAGE),
- std::string(kCategoryTagCrash));
- } else {
- web_contents_->GetController().Reload(true);
- }
+ PerformAction(Action::BUTTON);
}
void SadTabView::Layout() {
@@ -237,72 +138,17 @@ void SadTabView::Layout() {
message_->SizeToFit(max_width);
title_->SizeToFit(max_width);
- if (help_message_ != nullptr)
- help_message_->SizeToFit(max_width);
-
View::Layout();
}
void SadTabView::OnPaint(gfx::Canvas* canvas) {
if (!painted_) {
- // These stats should use the same counting approach and bucket size used
- // for tab discard events in memory::OomPriorityManager so they can be
- // directly compared.
- switch (kind_) {
- case chrome::SAD_TAB_KIND_CRASHED: {
- static int crashed = 0;
- crashed++;
- UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab.CrashDisplayed", crashed);
- break;
- }
- case chrome::SAD_TAB_KIND_OOM: {
- static int crashed_due_to_oom = 0;
- crashed_due_to_oom++;
- UMA_HISTOGRAM_COUNTS_1000(
- "Tabs.SadTab.OomDisplayed", crashed_due_to_oom);
- break;
- }
- case chrome::SAD_TAB_KIND_KILLED:
- RecordKillDisplayed();
- break;
-#if defined(OS_CHROMEOS)
- case chrome::SAD_TAB_KIND_KILLED_BY_OOM:
- RecordKillDisplayed();
- RecordKillDisplayedOOM();
- break;
-#endif
- }
+ RecordFirstPaint();
painted_ = true;
}
View::OnPaint(canvas);
}
-void SadTabView::Show() {
- views::Widget::InitParams sad_tab_params(
- views::Widget::InitParams::TYPE_CONTROL);
-
- // It is not possible to create a native_widget_win that has no parent in
- // and later re-parent it.
- // TODO(avi): This is a cheat. Can this be made cleaner?
- sad_tab_params.parent = web_contents_->GetNativeView();
-
- set_owned_by_client();
-
- views::Widget* sad_tab = new views::Widget;
- sad_tab->Init(sad_tab_params);
- sad_tab->SetContentsView(this);
-
- views::Widget::ReparentNativeView(sad_tab->GetNativeView(),
- web_contents_->GetNativeView());
- gfx::Rect bounds = web_contents_->GetContainerBounds();
- sad_tab->SetBounds(gfx::Rect(bounds.size()));
-}
-
-void SadTabView::Close() {
- if (GetWidget())
- GetWidget()->Close();
-}
-
views::Label* SadTabView::CreateLabel(const base::string16& text) {
views::Label* label = new views::Label(text);
label->SetBackgroundColor(background()->get_color());
« no previous file with comments | « chrome/browser/ui/views/sad_tab_view.h ('k') | chrome/test/BUILD.gn » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698