Revert "Remove histograms from feedback reports"

This reverts commit ffc0bb9.

Reason for revert: crbug.com/942082 There's a project in process to make
use of this data and it was removed just before that started, so we are
putting this data back into the feedback reports.

Original change's description:
> Remove histograms from feedback reports
>
> Histograms data were added to the feedback reports long
> time ago, but they're uncommonly checked and no good use
> for them has been found, hence removed.
>
> BUG=915227
> TEST=File a feedback report and make sure it ends up with
>      no histogram data.
>
> Change-Id: Ie8e3540e7afa700ebf4b770c1cee7df729268081
> Reviewed-on: https://chromium-review.googlesource.com/c/1401563
> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
> Reviewed-by: Toni Baržić <tbarzic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620988}

Bug: 915227, 942082
Change-Id: I68d59b57cc2483430f39c53e92cf8253ba144956
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1532365
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Cr-Commit-Position: refs/heads/master@{#642703}

chrome/app/generated_resources.grd

@@ -5449,12 +5449,17 @@ the Bookmarks menu.">
         <message name="IDS_FEEDBACK_OFFLINE_DIALOG_TEXT" desc="The content of the message box displayed when the user attempts to send a report while offline">
           Thank you for your feedback. You are offline now, and your report will be sent later.
         </message>
-        <message name="IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_CHKBOX" desc="Checkbox for including system information on the bug report dialog box">
-          Send <ph name="BEGIN_LINK1">&lt;a href="#" id="sys-info-url"&gt;</ph>system information<ph name="END_LINK1">&lt;/a&gt;</ph>
-        </message>
+        <if expr="not chromeos">
+          <message name="IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_CHKBOX" desc="Checkbox for including system information on the bug report dialog box">
+            Send <ph name="BEGIN_LINK1">&lt;a href="#" id="sys-info-url"&gt;</ph>system information<ph name="END_LINK1">&lt;/a&gt;</ph>
+          </message>
+        </if>
         <if expr="chromeos">
-          <message name="IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_CHKBOX_ARC" desc="Checkbox for including system, and app information on the bug report dialog box on an ARC enabled device">
-            Send <ph name="BEGIN_LINK1">&lt;a href="#" id="sys-info-url"&gt;</ph>system and app information<ph name="END_LINK1">&lt;/a&gt;</ph>
+          <message name="IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_AND_METRICS_CHKBOX" desc="Checkbox for including system information and metrics on the bug report dialog box">
+            Send <ph name="BEGIN_LINK1">&lt;a href="#" id="sys-info-url"&gt;</ph>system information<ph name="END_LINK1">&lt;/a&gt;</ph> and <ph name="BEGIN_LINK2">&lt;a href="#" id="histograms-url"&gt;</ph>metrics<ph name="END_LINK2">&lt;/a&gt;</ph>
+          </message>
+          <message name="IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_AND_METRICS_CHKBOX_ARC" desc="Checkbox for including system, metrics and app information on the bug report dialog box on an ARC enabled device">
+            Send <ph name="BEGIN_LINK1">&lt;a href="#" id="sys-info-url"&gt;</ph>system and app information<ph name="END_LINK1">&lt;/a&gt;</ph>, and <ph name="BEGIN_LINK2">&lt;a href="#" id="histograms-url"&gt;</ph>metrics<ph name="END_LINK2">&lt;/a&gt;</ph>
           </message>
         </if>
         <message name="IDS_FEEDBACK_INCLUDE_ASSISTANT_INFORMATION_CHKBOX" desc="Checkbox for including Assistant debug information in the feedback report">

chrome/app/generated_resources_grd/IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_AND_METRICS_CHKBOX.png.sha1

@@ -0,0 +1 @@
+bf18f00274d97bbe09707e55fac83c0b80b03ae4

chrome/app/generated_resources_grd/IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_AND_METRICS_CHKBOX_ARC.png.sha1

@@ -0,0 +1 @@
+e895e0c7b1649adc70084e1c97c210f58cc55e3b

chrome/browser/extensions/api/feedback_private/chrome_feedback_private_delegate.cc

@@ -41,11 +41,13 @@ int GetSysInfoCheckboxStringId(content::BrowserContext* browser_context) {
 #if defined(OS_CHROMEOS)
   if (arc::IsArcPlayStoreEnabledForProfile(
           Profile::FromBrowserContext(browser_context))) {
-    return IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_CHKBOX_ARC;
+    return IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_AND_METRICS_CHKBOX_ARC;
+  } else {
+    return IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_AND_METRICS_CHKBOX;
   }
-#endif
-
+#else
   return IDS_FEEDBACK_INCLUDE_SYSTEM_INFORMATION_CHKBOX;
+#endif
 }
 
 }  // namespace

chrome/browser/resources/feedback/js/feedback.js

@@ -225,8 +225,10 @@ function sendReport() {
   feedbackInfo.email = $('user-email-drop-down').value;
 
   let useSystemInfo = false;
+  let useHistograms = false;
   if ($('sys-info-checkbox') != null && $('sys-info-checkbox').checked) {
-    useSystemInfo = true;
+    // Send histograms along with system info.
+    useSystemInfo = useHistograms = true;
   }
 
   // <if expr="chromeos">
@@ -251,6 +253,8 @@ function sendReport() {
   }
   // </if>
 
+  feedbackInfo.sendHistograms = useHistograms;
+
   // If the user doesn't want to send the screenshot.
   if (!$('screenshot-checkbox').checked) {
     feedbackInfo.screenshot = null;
@@ -535,6 +539,14 @@ function initialize() {
           };
         }
 
+        const histogramUrlElement = $('histograms-url');
+        if (histogramUrlElement) {
+          // Opens a new window showing the histogram metrics.
+          setupLinkHandlers(
+              histogramUrlElement, 'chrome://histograms',
+              true /* useAppWindow */);
+        }
+
         const legalHelpPageUrlElement = $('legal-help-page-url');
         if (legalHelpPageUrlElement) {
           setupLinkHandlers(

components/feedback/feedback_data.cc

@@ -27,6 +27,11 @@ namespace {
 const char kTraceFilename[] = "tracing.zip";
 const char kPerformanceCategoryTag[] = "Performance";
 
+const base::FilePath::CharType kHistogramsFilename[] =
+    FILE_PATH_LITERAL("histograms.txt");
+
+const char kHistogramsAttachmentName[] = "histograms.zip";
+
 }  // namespace
 
 FeedbackData::FeedbackData(feedback::FeedbackUploader* uploader)
@@ -74,6 +79,21 @@ void FeedbackData::SetAndCompressSystemInfo(
   }
 }
 
+void FeedbackData::SetAndCompressHistograms(
+    std::unique_ptr<std::string> histograms) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  if (!histograms)
+    return;
+  ++pending_op_count_;
+  base::PostTaskWithTraitsAndReply(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
+      base::BindOnce(&FeedbackData::CompressFile, this,
+                     base::FilePath(kHistogramsFilename),
+                     kHistogramsAttachmentName, std::move(histograms)),
+      base::BindOnce(&FeedbackData::OnCompressComplete, this));
+}
+
 void FeedbackData::AttachAndCompressFileData(
     std::unique_ptr<std::string> attached_filedata) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);

components/feedback/feedback_data.h

@@ -34,6 +34,10 @@ class FeedbackData : public FeedbackCommon {
   // compression.
   void SetAndCompressSystemInfo(std::unique_ptr<SystemLogsMap> sys_info);
 
+  // Sets the histograms for this instance and kicks off its
+  // compression.
+  void SetAndCompressHistograms(std::unique_ptr<std::string> histograms);
+
   // Sets the attached file data and kicks off its compression.
   void AttachAndCompressFileData(
       std::unique_ptr<std::string> attached_filedata);

components/feedback/feedback_data_unittest.cc

@@ -24,6 +24,7 @@ namespace feedback {
 
 namespace {
 
+constexpr char kHistograms[] = "Histogram Data";
 constexpr char kImageData[] = "Image Data";
 constexpr char kFileData[] = "File Data";
 
@@ -97,6 +98,7 @@ class FeedbackDataTest : public testing::Test {
 };
 
 TEST_F(FeedbackDataTest, ReportSending) {
+  data_->SetAndCompressHistograms(MakeScoped(kHistograms));
   data_->set_image(MakeScoped(kImageData));
   data_->AttachAndCompressFileData(MakeScoped(kFileData));
   Send();

extensions/browser/api/feedback_private/feedback_private_api.cc

@@ -11,6 +11,8 @@
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/statistics_recorder.h"
 #include "base/metrics/user_metrics.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -331,26 +333,37 @@ ExtensionFunction::ResponseAction FeedbackPrivateSendFeedbackFunction::Run() {
 
   delegate->FetchAndMergeIwlwifiDumpLogsIfPresent(
       std::move(sys_logs), browser_context(),
-      base::Bind(&FeedbackPrivateSendFeedbackFunction::OnAllLogsFetched, this,
-                 feedback_data,
-                 feedback_info.send_bluetooth_logs &&
-                     *feedback_info.send_bluetooth_logs));
+      base::BindOnce(
+          &FeedbackPrivateSendFeedbackFunction::OnAllLogsFetched, this,
+          feedback_data,
+          feedback_info.send_histograms && *feedback_info.send_histograms,
+          feedback_info.send_bluetooth_logs &&
+              *feedback_info.send_bluetooth_logs));
 #else
-  OnAllLogsFetched(feedback_data, false /* send_bluetooth_logs */,
-                   std::move(sys_logs));
+  OnAllLogsFetched(feedback_data, false /* send_histograms */,
+                   false /* send_bluetooth_logs */, std::move(sys_logs));
 #endif  // defined(OS_CHROMEOS)
 
   return RespondLater();
 }
 
 void FeedbackPrivateSendFeedbackFunction::OnAllLogsFetched(
     scoped_refptr<FeedbackData> feedback_data,
+    bool send_histograms,
     bool send_bluetooth_logs,
     std::unique_ptr<system_logs::SystemLogsResponse> sys_logs) {
   VLOG(1) << "All logs have been fetched. Proceeding with sending the report.";
 
   feedback_data->SetAndCompressSystemInfo(std::move(sys_logs));
 
+  if (send_histograms) {
+    auto histograms = std::make_unique<std::string>();
+    *histograms =
+        base::StatisticsRecorder::ToJSON(base::JSON_VERBOSITY_LEVEL_FULL);
+    if (!histograms->empty())
+      feedback_data->SetAndCompressHistograms(std::move(histograms));
+  }
+
   if (send_bluetooth_logs) {
     std::unique_ptr<std::string> bluetooth_logs =
         std::make_unique<std::string>();

extensions/browser/api/feedback_private/feedback_private_api.h

@@ -142,6 +142,7 @@ class FeedbackPrivateSendFeedbackFunction : public UIThreadExtensionFunction {
  private:
   void OnAllLogsFetched(
       scoped_refptr<feedback::FeedbackData> feedback_data,
+      bool send_histograms,
       bool send_bluetooth_logs,
       std::unique_ptr<FeedbackCommon::SystemLogsMap> sys_logs);
   void OnCompleted(api::feedback_private::LandingPageType type, bool success);

extensions/common/api/feedback_private.idl

@@ -73,7 +73,6 @@ namespace feedbackPrivate {
     SystemInformation[]? systemInformation;
 
     // True if we have permission to add histograms to this feedback report.
-    // Deprecated and will be ignored.
     boolean? sendHistograms;
 
     // Optional feedback UI flow. Default is the regular user flow.