Refactor and fix feedback

chrome/browser/feedback/feedback_util.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/bug_report_util.h"
+#include "chrome/browser/feedback/feedback_util.h"
 
 #include <sstream>
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/file_version_info.h"
 #include "base/memory/singleton.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/windows_version.h"
@@ skipping 13 matching lines @@
 #include "net/url_request/url_request_status.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "unicode/locid.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/notifications/system_notification.h"
 #endif
 
 namespace {
 
-const int kBugReportVersion = 1;
+const int kFeedbackVersion = 1;
 
 const char kReportPhishingUrl[] =
     "http://www.google.com/safebrowsing/report_phish/";
 
 // URL to post bug reports to.
-static char const kBugReportPostUrl[] =
+static char const kFeedbackPostUrl[] =
     "https://www.google.com/tools/feedback/chrome/__submit";
 
 static char const kProtBufMimeType[] = "application/x-protobuf";
 static char const kPngMimeType[] = "image/png";
 
 // Tags we use in product specific data
 static char const kChromeVersionTag[] = "CHROME VERSION";
 static char const kOsVersionTag[] = "OS VERSION";
 
-static char const kNotificationId[] = "feedback.chromeos";
-
 static int const kHttpPostSuccessNoContent = 204;
 static int const kHttpPostFailNoConnection = -1;
 static int const kHttpPostFailClientError = 400;
 static int const kHttpPostFailServerError = 500;
 
 #if defined(OS_CHROMEOS)
 static char const kBZip2MimeType[] = "application/x-bzip2";
 static char const kLogsAttachmentName[] = "system_logs.bz2";
 // Maximum number of lines in system info log chunk to be still included
 // in product specific data.
 const size_t kMaxLineCount       = 40;
 // Maximum number of bytes in system info log chunk to be still included
 // in product specific data.
 const size_t kMaxSystemLogLength = 4 * 1024;
 #endif
 
-const int64 kInitialRetryDelay = 900000; // 15 minutes
+const int64 kInitialRetryDelay = 900000;  // 15 minutes
 const int64 kRetryDelayIncreaseFactor = 2;
-const int64 kRetryDelayLimit = 14400000; // 4 hours
+const int64 kRetryDelayLimit = 14400000;  // 4 hours
 
 
 }  // namespace
 
 
 // Simple content::URLFetcherDelegate to clean up URLFetcher on completion.
-class BugReportUtil::PostCleanup : public content::URLFetcherDelegate {
+class FeedbackUtil::PostCleanup : public content::URLFetcherDelegate {
  public:
   PostCleanup(Profile* profile, std::string* post_body,
               int64 previous_delay) : profile_(profile),
                                       post_body_(post_body),
                                       previous_delay_(previous_delay) { }
   // Overridden from content::URLFetcherDelegate.
   virtual void OnURLFetchComplete(const content::URLFetcher* source);
 
  protected:
   virtual ~PostCleanup() {}
 
  private:
   Profile* profile_;
   std::string* post_body_;
   int64 previous_delay_;
 
   DISALLOW_COPY_AND_ASSIGN(PostCleanup);
 };
 
 // Don't use the data parameter, instead use the pointer we pass into every
 // post cleanup object - that pointer will be deleted and deleted only on a
 // successful post to the feedback server.
-void BugReportUtil::PostCleanup::OnURLFetchComplete(
+void FeedbackUtil::PostCleanup::OnURLFetchComplete(
     const content::URLFetcher* source) {
   std::stringstream error_stream;
   int response_code = source->GetResponseCode();
   if (response_code == kHttpPostSuccessNoContent) {
     // We've sent our report, delete the report data
     delete post_body_;
 
     error_stream << "Success";
   } else {
     // Uh oh, feedback failed, send it off to retry
     if (previous_delay_) {
       if (previous_delay_ < kRetryDelayLimit)
         previous_delay_ *= kRetryDelayIncreaseFactor;
     } else {
       previous_delay_ = kInitialRetryDelay;
     }
-    BugReportUtil::DispatchFeedback(profile_, post_body_, previous_delay_);
+    FeedbackUtil::DispatchFeedback(profile_, post_body_, previous_delay_);
 
     // Process the error for debug output
     if (response_code == kHttpPostFailNoConnection) {
       error_stream << "No connection to server.";
     } else if ((response_code > kHttpPostFailClientError) &&
         (response_code < kHttpPostFailServerError)) {
       error_stream << "Client error: HTTP response code " << response_code;
     } else if (response_code > kHttpPostFailServerError) {
       error_stream << "Server error: HTTP response code " << response_code;
     } else {
       error_stream << "Unknown error: HTTP response code " << response_code;
     }
   }
 
   LOG(WARNING) << "FEEDBACK: Submission to feedback server (" <<
                source->GetURL() << ") status: " << error_stream.str();
 
   // Delete the URLFetcher.
   delete source;
   // And then delete ourselves.
   delete this;
 }
 
 // static
-void BugReportUtil::SetOSVersion(std::string* os_version) {
+void FeedbackUtil::SetOSVersion(std::string* os_version) {
 #if defined(OS_WIN)
   base::win::OSInfo* os_info = base::win::OSInfo::GetInstance();
   base::win::OSInfo::VersionNumber version_number = os_info->version_number();
   *os_version = base::StringPrintf("%d.%d.%d",
                                    version_number.major,
                                    version_number.minor,
                                    version_number.build);
   int service_pack = os_info->service_pack().major;
   if (service_pack > 0)
     os_version->append(base::StringPrintf("Service Pack %d", service_pack));
 #elif defined(OS_MACOSX)
   *os_version = base::SysInfo::OperatingSystemVersion();
 #else
   *os_version = "unknown";
 #endif
 }
 
 // static
-void BugReportUtil::DispatchFeedback(Profile* profile,
+void FeedbackUtil::DispatchFeedback(Profile* profile,
                                      std::string* post_body,
                                      int64 delay) {
   DCHECK(post_body);
 
   MessageLoop::current()->PostDelayedTask(FROM_HERE, base::Bind(
-      &BugReportUtil::SendFeedback, profile, post_body, delay), delay);
+      &FeedbackUtil::SendFeedback, profile, post_body, delay), delay);
 }
 
 // static
-void BugReportUtil::SendFeedback(Profile* profile,
+void FeedbackUtil::SendFeedback(Profile* profile,
                                  std::string* post_body,
                                  int64 previous_delay) {
   DCHECK(post_body);
 
   GURL post_url;
   if (CommandLine::ForCurrentProcess()->
       HasSwitch(switches::kFeedbackServer))
     post_url = GURL(CommandLine::ForCurrentProcess()->
         GetSwitchValueASCII(switches::kFeedbackServer));
   else
-    post_url = GURL(kBugReportPostUrl);
+    post_url = GURL(kFeedbackPostUrl);
 
   content::URLFetcher* fetcher = content::URLFetcher::Create(
       post_url, content::URLFetcher::POST,
-      new BugReportUtil::PostCleanup(profile, post_body, previous_delay));
+      new FeedbackUtil::PostCleanup(profile, post_body, previous_delay));
   fetcher->SetRequestContext(profile->GetRequestContext());
 
   fetcher->SetUploadData(std::string(kProtBufMimeType), *post_body);
   fetcher->Start();
 }
 
 
 // static
-void BugReportUtil::AddFeedbackData(
-    userfeedback::ExternalExtensionSubmit* feedback_data,
+void FeedbackUtil::AddFeedbackData(
+    userfeedback::ExtensionSubmit* feedback_data,
     const std::string& key, const std::string& value) {
   // Don't bother with empty keys or values
-  if (key=="" || value == "") return;
+  if (key == "" || value == "") return;
   // Create log_value object and add it to the web_data object
   userfeedback::ProductSpecificData log_value;
   log_value.set_key(key);
   log_value.set_value(value);
   userfeedback::WebData* web_data = feedback_data->mutable_web_data();
   *(web_data->add_product_specific_data()) = log_value;
 }
 
 #if defined(OS_CHROMEOS)
-bool BugReportUtil::ValidFeedbackSize(const std::string& content) {
+bool FeedbackUtil::ValidFeedbackSize(const std::string& content) {
   if (content.length() > kMaxSystemLogLength)
     return false;
   size_t line_count = 0;
   const char* text = content.c_str();
   for (size_t i = 0; i < content.length(); i++) {
     if (*(text + i) == '\n') {
       line_count++;
       if (line_count > kMaxLineCount)
         return false;
     }
   }
   return true;
 }
 #endif
 
 // static
-void BugReportUtil::SendReport(
+void FeedbackUtil::SendReport(
     Profile* profile
-    , int problem_type
+    , const std::string& category_tag
     , const std::string& page_url_text
     , const std::string& description
     , ScreenshotDataPtr image_data_ptr
     , int png_width
     , int png_height
 #if defined(OS_CHROMEOS)
     , const std::string& user_email_text
     , const char* zipped_logs_data
     , int zipped_logs_length
     , const chromeos::system::LogDictionaryType* const sys_info
 #endif
     ) {
   // Create google feedback protocol buffer objects
-  userfeedback::ExternalExtensionSubmit feedback_data;
+  userfeedback::ExtensionSubmit feedback_data;
   // type id set to 0, unused field but needs to be initialized to 0
   feedback_data.set_type_id(0);
 
   userfeedback::CommonData* common_data = feedback_data.mutable_common_data();
   userfeedback::WebData* web_data = feedback_data.mutable_web_data();
 
   // Set GAIA id to 0. We're not using gaia id's for recording
   // use feedback - we're using the e-mail field, allows users to
   // submit feedback from incognito mode and specify any mail id
   // they wish
@@ skipping 17 matching lines @@
   // Add the Chrome version
   chrome::VersionInfo version_info;
   if (version_info.is_valid()) {
     std::string chrome_version = version_info.Name() + " - " +
         version_info.Version() +
         " (" + version_info.LastChange() + ")";
     AddFeedbackData(&feedback_data, std::string(kChromeVersionTag),
                     chrome_version);
   }
 
+  // We don't need the OS version for ChromeOS since we get it in
+  // CHROMEOS_RELEASE_VERSION from /etc/lsb-release
+#if !defined(OS_CHROMEOS)
   // Add OS version (eg, for WinXP SP2: "5.1.2600 Service Pack 2").
   std::string os_version = "";
   SetOSVersion(&os_version);
   AddFeedbackData(&feedback_data, std::string(kOsVersionTag), os_version);
+#endif
 
   // Include the page image if we have one.
   if (image_data_ptr.get() && image_data_ptr->size()) {
     userfeedback::PostedScreenshot screenshot;
     screenshot.set_mime_type(kPngMimeType);
     // Set the dimensions of the screenshot
     userfeedback::Dimensions dimensions;
     dimensions.set_width(static_cast<float>(png_width));
     dimensions.set_height(static_cast<float>(png_height));
     *(screenshot.mutable_dimensions()) = dimensions;
@@ skipping 22 matching lines @@
         switches::kCompressSystemFeedback)) {
       userfeedback::ProductSpecificBinaryData attachment;
       attachment.set_mime_type(kBZip2MimeType);
       attachment.set_name(kLogsAttachmentName);
       attachment.set_data(std::string(zipped_logs_data, zipped_logs_length));
       *(feedback_data.add_product_specific_binary_data()) = attachment;
     }
   }
 #endif
 
+  // Set our category tag
+//  feedback_data.set_category_tag(category_tag);

[xiyuan, 2011/12/22 02:18:23]

Is this line needed?

[rkc, 2011/12/22 22:41:46]

Oops. Yes, just need to check to see if a category tag was specified. Category
tags are still needed for things like Autofill and crash feedback.

Done.

+
   // Set our Chrome specific data
   userfeedback::ChromeData chrome_data;
+  chrome_data.set_chrome_platform(
 #if defined(OS_CHROMEOS)
-  chrome_data.set_chrome_platform(
       userfeedback::ChromeData_ChromePlatform_CHROME_OS);
   userfeedback::ChromeOsData chrome_os_data;
   chrome_os_data.set_category(
-      (userfeedback::ChromeOsData_ChromeOsCategory) problem_type);
+      userfeedback::ChromeOsData_ChromeOsCategory_OTHER);
   *(chrome_data.mutable_chrome_os_data()) = chrome_os_data;
 #else
-  chrome_data.set_chrome_platform(
       userfeedback::ChromeData_ChromePlatform_CHROME_BROWSER);
   userfeedback::ChromeBrowserData chrome_browser_data;
   chrome_browser_data.set_category(
-      (userfeedback::ChromeBrowserData_ChromeBrowserCategory) problem_type);
+      userfeedback::ChromeBrowserData_ChromeBrowserCategory_OTHER);
   *(chrome_data.mutable_chrome_browser_data()) = chrome_browser_data;
 #endif
 
   *(feedback_data.mutable_chrome_data()) = chrome_data;
 
   // Serialize our report to a string pointer we can pass around
   std::string* post_body = new std::string;
   feedback_data.SerializeToString(post_body);
 
   // We have the body of our POST, so send it off to the server with 0 delay
   DispatchFeedback(profile, post_body, 0);
 }
 
 #if defined(ENABLE_SAFE_BROWSING)
 // static
-void BugReportUtil::ReportPhishing(TabContents* currentTab,
+void FeedbackUtil::ReportPhishing(TabContents* currentTab,
                                    const std::string& phishing_url) {
   currentTab->controller().LoadURL(
       safe_browsing_util::GeneratePhishingReportUrl(
           kReportPhishingUrl, phishing_url,
           false /* not client-side detection */),
       content::Referrer(),
       content::PAGE_TRANSITION_LINK,
       std::string());
 }
 #endif
 
 static std::vector<unsigned char>* screenshot_png = NULL;
 static gfx::Rect* screenshot_size = NULL;
 
 // static
-std::vector<unsigned char>* BugReportUtil::GetScreenshotPng() {
+std::vector<unsigned char>* FeedbackUtil::GetScreenshotPng() {
   if (screenshot_png == NULL)
     screenshot_png = new std::vector<unsigned char>;
   return screenshot_png;
 }
 
 // static
-void BugReportUtil::ClearScreenshotPng() {
+void FeedbackUtil::ClearScreenshotPng() {
   if (screenshot_png)
     screenshot_png->clear();
 }
 
 // static
-gfx::Rect& BugReportUtil::GetScreenshotSize() {
+gfx::Rect& FeedbackUtil::GetScreenshotSize() {
   if (screenshot_size == NULL)
     screenshot_size = new gfx::Rect();
   return *screenshot_size;
 }
 
 // static
-void BugReportUtil::SetScreenshotSize(const gfx::Rect& rect) {
+void FeedbackUtil::SetScreenshotSize(const gfx::Rect& rect) {
   gfx::Rect& screen_size = GetScreenshotSize();
   screen_size = rect;
 }