Manual crash uploads for mac and win

chrome/browser/ui/webui/crashes_ui.cc

Index: chrome/browser/ui/webui/crashes_ui.cc
diff --git a/chrome/browser/ui/webui/crashes_ui.cc b/chrome/browser/ui/webui/crashes_ui.cc
index af48299bb57453d4df0cb1a1eee9ddd41653a03a..a20706a1db3b8006db4eb2572d73e38b251c9097 100644
--- a/chrome/browser/ui/webui/crashes_ui.cc
+++ b/chrome/browser/ui/webui/crashes_ui.cc
@@ -16,6 +16,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/crash_upload_list/crash_upload_list.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/browser/metrics/metrics_reporting_state.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
@@ -92,6 +93,11 @@ class CrashesDOMHandler : public WebUIMessageHandler,
   // Sends the recent crashes list JS.
   void UpdateUI();
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // Asynchronously requests a user triggered upload. Called from JS.
+  void HandleRequestSingleCrashUpload(const base::ListValue* args);
+#endif
+
   scoped_refptr<CrashUploadList> upload_list_;
   bool list_available_;
   bool first_load_;
@@ -121,6 +127,13 @@ void CrashesDOMHandler::RegisterMessages() {
       base::Bind(&CrashesDOMHandler::HandleRequestUploads,
                  base::Unretained(this)));
 #endif
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  web_ui()->RegisterMessageCallback(
+      crash::kCrashesUIRequestSingleCrashUpload,
+      base::Bind(&CrashesDOMHandler::HandleRequestSingleCrashUpload,
+                 base::Unretained(this)));
+#endif
 }
 
 void CrashesDOMHandler::HandleRequestCrashes(const base::ListValue* args) {
@@ -160,12 +173,26 @@ void CrashesDOMHandler::UpdateUI() {
   system_crash_reporter = true;
 #endif
 
+  bool support_manual_uploads = false;

[Mark Mentovai, 2016/08/25 20:25:11]

Put “const bool support_manual_uploads = …” in both branches of the #if that
follows, so that it’s impossible to accidentally change the value.

[gayane -on leave until 09-2017, 2016/08/26 00:12:54]

Done.

   base::ListValue crash_list;
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // Maunal uploads currently are supported only for Windows and Mac platforms.
+  // Allow manual uploads only if crash uploads are not disabled by policy.

[Mark Mentovai, 2016/08/25 20:25:11]

s/Windows and Mac/Crashpad-using/

[gayane -on leave until 09-2017, 2016/08/26 00:12:55]

Done.

+  support_manual_uploads =
+      crash_reporting_enabled || IsMetricsReportingPolicyManaged();

[Mark Mentovai, 2016/08/25 20:25:11]

We want to support manual uploads if the user has disabled crash reporting too.
That’s a major reason to do this, in fact. But we don’t want to support it if
policy says “no”.

[gayane -on leave until 09-2017, 2016/08/26 00:12:55]

Thanks. There was a logic problem here. Now manual uploads would be off only if
crash reporting is disabled and policy is on.

+  // Show crash reports regardless of |crash_reporting_enabled| so that users
+  // can manually uploads those reports.
+  crash::UploadListToValue(upload_list_.get(), &crash_list);
+#else
+  // Don't show crash reports list if crash reporting is disbled for platforms
+  // where manual uploads are not supported.
   if (crash_reporting_enabled)
     crash::UploadListToValue(upload_list_.get(), &crash_list);
+#endif
 
   base::FundamentalValue enabled(crash_reporting_enabled);
   base::FundamentalValue dynamic_backend(system_crash_reporter);
+  base::FundamentalValue manual_uploads(support_manual_uploads);
   base::StringValue version(version_info::GetVersionNumber());
   base::StringValue os_string(base::SysInfo::OperatingSystemName() + " " +
                               base::SysInfo::OperatingSystemVersion());
@@ -173,6 +200,7 @@ void CrashesDOMHandler::UpdateUI() {
   std::vector<const base::Value*> args;
   args.push_back(&enabled);
   args.push_back(&dynamic_backend);
+  args.push_back(&manual_uploads);
   args.push_back(&crash_list);
   args.push_back(&version);
   args.push_back(&os_string);
@@ -180,6 +208,16 @@ void CrashesDOMHandler::UpdateUI() {
                                          args);
 }
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+void CrashesDOMHandler::HandleRequestSingleCrashUpload(

[Mark Mentovai, 2016/08/25 20:25:11]

I think it’s a good idea to check that policy says it’s OK to do the upload
here, once C++ has control again. Otherwise, we’re trusting the
(user-inspectable) JS code to maintain the correct value of
support_manual_uploads. Dangerous.

[gayane -on leave until 09-2017, 2016/08/26 00:12:55]

Done.

+    const base::ListValue* args) {
+  std::string local_id;
+  if (!args || !args->GetString(0, &local_id))
+    return;
+  upload_list_->RequestSingleCrashUploadAsync(local_id);
+}
+#endif
+
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////