Add memory usage info to SyslogsProvider, and clean up bug report screenshot data.

chrome/browser/bug_report_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 <sstream>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 31 matching lines @@
     "http://www.google.com/safebrowsing/report_phish/";
 
 // URL to post bug reports to.
 static char const kBugReportPostUrl[] =
     "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 kPageTitleTag[] = "PAGE TITLE";
-static char const kProblemTypeIdTag[] = "PROBLEM TYPE ID";
-static char const kProblemTypeTag[] = "PROBLEM TYPE";

[stevenjb, 2011/08/03 01:55:20]

Unused.

 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;
 
@@ skipping 176 matching lines @@
       line_count++;
       if (line_count > kMaxLineCount)
         return false;
     }
   }
   return true;
 }
 #endif
 
 // static
-void BugReportUtil::SendReport(Profile* profile,
-    int problem_type,
-    const std::string& page_url_text,
-    const std::string& description,
-    const char* png_data,
-    int png_data_length,
-    int png_width,
+void BugReportUtil::SendReport(
+    Profile* profile
+    , int problem_type
+    , const std::string& page_url_text
+    , const std::string& description
+    , const char* png_data
+    , int png_data_length
+    , int png_width
+    , int png_height

[James Cook, 2011/08/03 17:06:49]

Missing end ) for non-ChromeOS builds.

 #if defined(OS_CHROMEOS)
-    int png_height,
-    const std::string& user_email_text,
-    const char* zipped_logs_data,
-    int zipped_logs_length,
-    const chromeos::system::LogDictionaryType* const sys_info) {
-#else
-    int png_height) {
+    , 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;
   // 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
@@ skipping 43 matching lines @@
     screenshot.set_binary_content(std::string(png_data, png_data_length));
 
     // Set the screenshot object in feedback
     *(feedback_data.mutable_screenshot()) = screenshot;
   }
 
 #if defined(OS_CHROMEOS)
   if (sys_info) {
     // Add the product specific data
     for (chromeos::system::LogDictionaryType::const_iterator i =
-             sys_info->begin(); i != sys_info->end(); ++i)
+             sys_info->begin(); i != sys_info->end(); ++i) {

[James Cook, 2011/08/03 17:06:49]

Ah, this is much clearer with an explicit block.  Nice.

       if (!CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kCompressSystemFeedback) || ValidFeedbackSize(i->second)) {
         AddFeedbackData(&feedback_data, i->first, i->second);
       }
+    }
 
     // If we have zipped logs, add them here
     if (zipped_logs_data && CommandLine::ForCurrentProcess()->HasSwitch(
         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;
     }
@@ skipping 33 matching lines @@
 void BugReportUtil::ReportPhishing(TabContents* currentTab,
                                    const std::string& phishing_url) {
   currentTab->controller().LoadURL(
       safe_browsing_util::GeneratePhishingReportUrl(
           kReportPhishingUrl, phishing_url,
           false /* not client-side detection */),
       GURL(),
       PageTransition::LINK);
 }
 #endif
+
+// static
+std::vector<unsigned char>* BugReportUtil::GetScreenshotPng() {
+  static std::vector<unsigned char>* last_screenshot_png = NULL;

[James Cook, 2011/08/03 17:06:49]

Do we every free the memory occupied by the screenshot data?  I don't care if we
leak a single std::vector<> object as long as we clear() out the data.

[stevenjb, 2011/08/03 19:11:39]

Yes, we call GetScreenshotPng()->clear() in
BugReportHandler::ClobberScreenshotsSource. I'll wrap that in a function to make
that more clear.

+  if (last_screenshot_png == NULL)
+    last_screenshot_png = new std::vector<unsigned char>;
+  return last_screenshot_png;
+}
+
+// static
+gfx::Rect& BugReportUtil::GetScreenshotSize() {
+  static gfx::Rect* screen_size = NULL;
+  if (screen_size == NULL)
+    screen_size = new gfx::Rect();
+  return *screen_size;
+}
+
+// static
+void BugReportUtil::SetScreenshotSize(const gfx::Rect& rect) {
+  gfx::Rect& screen_size = GetScreenshotSize();
+  screen_size = rect;
+}