Index: chrome/browser/ui/sad_tab.cc |
diff --git a/chrome/browser/ui/sad_tab.cc b/chrome/browser/ui/sad_tab.cc |
index 03c5c4dc8d254bff248da5ab2a29104ca374703f..0a1c0ecc9f230e74ef0c296b0eaf38765af55a5f 100644 |
--- a/chrome/browser/ui/sad_tab.cc |
+++ b/chrome/browser/ui/sad_tab.cc |
@@ -2,20 +2,205 @@ |
// Use of this source code is governed by a BSD-style license that can be |
// found in the LICENSE file. |
-#include "build/build_config.h" |
#include "chrome/browser/ui/sad_tab.h" |
+#include "base/metrics/histogram_macros.h" |
+#include "chrome/browser/net/referrer.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" |
+ |
+#if defined(OS_CHROMEOS) |
+#include "chrome/browser/memory/oom_memory_details.h" |
+#endif |
+ |
+namespace { |
+ |
+// These stats should use the same counting approach and bucket size as tab |
+// discard events in memory::OomPriorityManager so they can be directly |
+// compared. |
+// TODO(jamescook): Maybe track time between sad tabs? |
James Cook
2016/09/08 20:15:50
Feel free to nuke this -- it's not going to happen
Sidney San Martín
2016/09/09 01:36:00
Done :).
|
+ |
+// This macro uses a static counter to track how many times it's hit in a |
+// session. See Tabs.SadTab.CrashCreated in histograms.xml for details. |
+#define UMA_SAD_TAB_COUNTER(NAME) \ |
James Cook
2016/09/08 20:15:50
I wouldn't do this with a macro and string concate
Sidney San Martín
2016/09/09 01:36:00
The macro deals with creating a block with a stati
James Cook
2016/09/09 04:37:08
This is fine. However, please change NAME to |hist
Sidney San Martín
2016/09/09 15:39:44
Done.
|
+ { \ |
+ static int count = 0; \ |
+ ++count; \ |
+ UMA_HISTOGRAM_COUNTS_1000("Tabs.SadTab." NAME, count); \ |
+ } |
+ |
+// This enum backs an UMA histogram, so it should be treated as append-only. |
+enum class SadTabEvent { |
+ DISPLAYED, |
+ BUTTON_CLICKED, |
+ HELP_LINK_CLICKED, |
+ MAX_SAD_TAB_EVENT |
+}; |
+ |
+void RecordEvent(bool feedback, SadTabEvent event) { |
+ if (feedback) { |
+ UMA_HISTOGRAM_ENUMERATION("Tabs.SadTab.StyleFeedback", event, |
James Cook
2016/09/08 20:15:50
I would call these either CreatedWithFeedback or D
Sidney San Martín
2016/09/09 01:36:00
These histograms are for other events (BUTTON_CLIC
James Cook
2016/09/09 04:37:08
Hrm, I missed that distinction when reading this c
Sidney San Martín
2016/09/09 15:39:44
Done. I like the idea of splitting it up.
|
+ SadTabEvent::MAX_SAD_TAB_EVENT); |
+ } else { |
+ UMA_HISTOGRAM_ENUMERATION("Tabs.SadTab.StyleReload", event, |
+ SadTabEvent::MAX_SAD_TAB_EVENT); |
+ } |
+} |
+ |
+static const int kCrashesBeforeFeedbackIsDisplayed = 1; |
James Cook
2016/09/08 20:15:50
nit: static not needed in anonymous namespace
Sidney San Martín
2016/09/09 01:36:00
Good catch, removed.
|
+static const char kCategoryTagCrash[] = "Crash"; |
+ |
+bool ShouldWantFeedback() { |
+ static int total_crashes = 0; |
+ return ++total_crashes > kCrashesBeforeFeedbackIsDisplayed; |
James Cook
2016/09/08 20:15:50
Aside: I had no idea we had this feature. It seems
Sidney San Martín
2016/09/09 01:36:00
It's what we do on other platforms, but I bet it w
|
+} |
+ |
+} // namespace |
+ |
namespace chrome { |
// static |
bool SadTab::ShouldShow(base::TerminationStatus status) { |
- return (status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION || |
- status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED || |
+ switch (status) { |
+ case base::TERMINATION_STATUS_ABNORMAL_TERMINATION: |
+ case base::TERMINATION_STATUS_PROCESS_WAS_KILLED: |
+#if defined(OS_CHROMEOS) |
+ case base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM: |
+#endif |
+ case base::TERMINATION_STATUS_PROCESS_CRASHED: |
+ case base::TERMINATION_STATUS_OOM: |
+ return true; |
+ case base::TERMINATION_STATUS_NORMAL_TERMINATION: |
+ case base::TERMINATION_STATUS_STILL_RUNNING: |
+#if defined(OS_ANDROID) |
+ case base::TERMINATION_STATUS_OOM_PROTECTED: |
+#endif |
+ case base::TERMINATION_STATUS_LAUNCH_FAILED: |
+ case base::TERMINATION_STATUS_MAX_ENUM: |
+ return false; |
Nico
2016/09/08 17:12:59
Nice change!
|
+ } |
+ NOTREACHED(); |
+ return false; |
+} |
+ |
+SadTab::SadTab(content::WebContents* web_contents, SadTabKind kind) |
James Cook
2016/09/08 20:15:50
nit: make function order in .cc file match declara
Sidney San Martín
2016/09/09 01:36:00
Done. I wonder how hard it would be to make `git c
James Cook
2016/09/09 04:37:08
Yeah, it's kind of a judgement call for me. If you
|
+ : web_contents_(web_contents), |
+ kind_(kind), |
+ want_feedback_(ShouldWantFeedback()) { |
+ switch (kind) { |
+ case chrome::SAD_TAB_KIND_CRASHED: |
+ UMA_SAD_TAB_COUNTER("CrashCreated"); |
+ break; |
+ case chrome::SAD_TAB_KIND_OOM: |
+ UMA_SAD_TAB_COUNTER("OomCreated"); |
+ break; |
+#if defined(OS_CHROMEOS) |
+ case chrome::SAD_TAB_KIND_KILLED_BY_OOM: |
+ UMA_SAD_TAB_COUNTER("KillCreated.OOM"); |
+ { |
+ const std::string spec = web_contents->GetURL().GetOrigin().spec(); |
+ memory::OomMemoryDetails::Log( |
+ "Tab OOM-Killed Memory details: " + spec + ", ", base::Closure()); |
+ } |
+// Fall through |
James Cook
2016/09/08 20:15:50
nit: Indent (unless this is the output of clang-fo
Sidney San Martín
2016/09/09 01:36:00
It is from clang-format, sadly.
Nico
2016/09/09 01:48:42
(I think if you switch the #endif and the comment
Sidney San Martín
2016/09/09 15:39:44
Done. I originally avoided doing this… but yeah, i
|
+#endif |
+ case chrome::SAD_TAB_KIND_KILLED: |
James Cook
2016/09/08 20:15:50
I would keep the LOG(WARNING) about kills. Kills a
Sidney San Martín
2016/09/09 01:36:00
I didn't know that! Thanks, done.
|
+ UMA_SAD_TAB_COUNTER("KillCreated"); |
+ break; |
+ } |
+} |
+ |
+int SadTab::GetTitle() { |
+ return IDS_SAD_TAB_TITLE; |
+} |
+ |
+int SadTab::GetMessage() { |
+ switch (kind_) { |
+#if defined(OS_CHROMEOS) |
+ case chrome::SAD_TAB_KIND_KILLED_BY_OOM: |
+ return IDS_KILLED_TAB_BY_OOM_MESSAGE; |
+#endif |
+ case chrome::SAD_TAB_KIND_OOM: |
+ return IDS_SAD_TAB_OOM_MESSAGE; |
+ case chrome::SAD_TAB_KIND_CRASHED: |
+ case chrome::SAD_TAB_KIND_KILLED: |
+ return IDS_SAD_TAB_MESSAGE; |
+ } |
+ NOTREACHED(); |
+ return 0; |
+} |
+ |
+int SadTab::GetButtonTitle() { |
+ return want_feedback_ ? IDS_CRASHED_TAB_FEEDBACK_LINK |
+ : IDS_SAD_TAB_RELOAD_LABEL; |
+} |
+ |
+int SadTab::GetHelpLinkTitle() { |
+ return IDS_SAD_TAB_LEARN_MORE_LINK; |
+} |
+ |
+const char* SadTab::GetHelpLinkURL() { |
+ return want_feedback_ ? chrome::kCrashReasonFeedbackDisplayedURL |
+ : chrome::kCrashReasonURL; |
+} |
+ |
+void SadTab::RecordFirstPaint() { |
+#if DCHECK_IS_ON() |
+ DCHECK(!recorded_paint_); |
+ recorded_paint_ = true; |
+#endif |
+ |
+ switch (kind_) { |
+ case chrome::SAD_TAB_KIND_CRASHED: |
+ UMA_SAD_TAB_COUNTER("CrashDisplayed"); |
+ break; |
+ case chrome::SAD_TAB_KIND_OOM: |
+ UMA_SAD_TAB_COUNTER("OomDisplayed"); |
+ break; |
#if defined(OS_CHROMEOS) |
- status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM || |
+ case chrome::SAD_TAB_KIND_KILLED_BY_OOM: |
+ UMA_SAD_TAB_COUNTER("KillDisplayed.OOM"); |
+// Fallthrough |
James Cook
2016/09/08 20:15:50
nit: ditto
|
#endif |
- status == base::TERMINATION_STATUS_PROCESS_CRASHED || |
- status == base::TERMINATION_STATUS_OOM); |
+ case chrome::SAD_TAB_KIND_KILLED: |
+ UMA_SAD_TAB_COUNTER("KillDisplayed"); |
+ break; |
+ } |
+ |
+ RecordEvent(want_feedback_, SadTabEvent::DISPLAYED); |
+} |
+ |
+void SadTab::PerformAction(SadTab::Action action) { |
+ DCHECK(recorded_paint_); |
+ switch (action) { |
+ case Action::BUTTON: |
+ RecordEvent(want_feedback_, SadTabEvent::BUTTON_CLICKED); |
+ if (want_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); |
+ } |
+ break; |
+ case Action::HELP_LINK: |
+ RecordEvent(want_feedback_, SadTabEvent::HELP_LINK_CLICKED); |
+ content::OpenURLParams params(GURL(GetHelpLinkURL()), content::Referrer(), |
+ CURRENT_TAB, ui::PAGE_TRANSITION_LINK, |
+ false); |
+ web_contents_->OpenURL(params); |
+ break; |
+ } |
} |
} // namespace chrome |