[Logging] Remove unneeded CheckOpString struct for CHECKs

Using string* directly works.

Made LogMessage delete any passed in string* after it uses it.

Removed suppressions for fixed memory leak.

BUG=57683
TEST=Existing unit tests

Review URL: http://codereview.chromium.org/6413032

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@74100 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 15 insertions, 37 deletions

diff --git a/base/logging.cc b/base/logging.cc
index d66f01b..0a808b4 100644
--- a/base/logging.cc
+++ b/base/logging.cc
@@ -540,17 +540,19 @@ LogMessage::LogMessage(const char* file, int line, LogSeverity severity)
   Init(file, line);
 }
 
-LogMessage::LogMessage(const char* file, int line, const CheckOpString& result)
+LogMessage::LogMessage(const char* file, int line, std::string* result)
     : severity_(LOG_FATAL), file_(file), line_(line) {
   Init(file, line);
-  stream_ << "Check failed: " << (*result.str_);
+  stream_ << "Check failed: " << *result;
+  delete result;
 }
 
 LogMessage::LogMessage(const char* file, int line, LogSeverity severity,
-                       const CheckOpString& result)
+                       std::string* result)
     : severity_(severity), file_(file), line_(line) {
   Init(file, line);
-  stream_ << "Check failed: " << (*result.str_);
+  stream_ << "Check failed: " << *result;
+  delete result;
 }
 
 LogMessage::~LogMessage() {
diff --git a/base/logging.h b/base/logging.h
index 771aa73..b1bc0b0 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -436,19 +436,10 @@ const LogSeverity LOG_0 = LOG_ERROR;
   LAZY_STREAM(PLOG_STREAM(FATAL), !(condition)) \
   << "Check failed: " #condition ". "
 
-// A container for a string pointer which can be evaluated to a bool -
-// true iff the pointer is NULL.
-struct CheckOpString {
-  CheckOpString(std::string* str) : str_(str) { }
-  // No destructor: if str_ is non-NULL, we're about to LOG(FATAL),
-  // so there's no point in cleaning up str_.
-  operator bool() const { return str_ != NULL; }
-  std::string* str_;
-};
-
 // Build the error message string.  This is separate from the "Impl"
 // function template because it is not performance critical and so can
-// be out of line, while the "Impl" code should be inline.
+// be out of line, while the "Impl" code should be inline.  Caller
+// takes ownership of the returned string.
 template<class t1, class t2>
 std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
   std::ostringstream ss;
@@ -479,7 +470,7 @@ extern template std::string* MakeCheckOpString<std::string, std::string>(
 // TODO(akalin): Rewrite this so that constructs like if (...)
 // CHECK_EQ(...) else { ... } work properly.
 #define CHECK_OP(name, op, val1, val2)                          \
-  if (logging::CheckOpString _result =                          \
+  if (std::string* _result =                                    \
       logging::Check##name##Impl((val1), (val2),                \
                                  #val1 " " #op " " #val2))      \
     logging::LogMessage(__FILE__, __LINE__, _result).stream()
@@ -655,7 +646,7 @@ const LogSeverity LOG_DCHECK = LOG_INFO;
 // Don't use this macro directly in your code, use DCHECK_EQ et al below.
 #define DCHECK_OP(name, op, val1, val2)                         \
   if (DCHECK_IS_ON())                                           \
-    if (logging::CheckOpString _result =                        \
+    if (std::string* _result =                                  \
         logging::Check##name##Impl((val1), (val2),              \
                                    #val1 " " #op " " #val2))    \
       logging::LogMessage(                                      \
@@ -723,14 +714,15 @@ class LogMessage {
   // saves a couple of bytes per call site.
   LogMessage(const char* file, int line, LogSeverity severity);
 
-  // A special constructor used for check failures.
+  // A special constructor used for check failures.  Takes ownership
+  // of the given string.
   // Implied severity = LOG_FATAL
-  LogMessage(const char* file, int line, const CheckOpString& result);
+  LogMessage(const char* file, int line, std::string* result);
 
   // A special constructor used for check failures, with the option to
-  // specify severity.
+  // specify severity.  Takes ownership of the given string.
   LogMessage(const char* file, int line, LogSeverity severity,
-             const CheckOpString& result);
+             std::string* result);
 
   ~LogMessage();
 
diff --git a/tools/heapcheck/suppressions.txt b/tools/heapcheck/suppressions.txt
index 3e04c6d..bf4ef65 100644
--- a/tools/heapcheck/suppressions.txt
+++ b/tools/heapcheck/suppressions.txt
@@ -949,14 +949,6 @@
    fun:::MessageLoopRelay::ProcessOnTargetThread
 }
 {
-   bug_57683
-   Heapcheck:Leak
-   ...
-   fun:logging::MakeCheckOpString
-   fun:logging::Check*Impl
-   fun:logging::::LoggingTest_*_Test::TestBody
-}
-{
    bug_57799
    Heapcheck:Leak
    fun:cricket::Transport::OnRemoteCandidate
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index 6e35331..789213a 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -2872,14 +2872,6 @@
    fun:_ZN14RunnableMethodIN3IPC12ChannelProxy7ContextEMS2_FvRKNS0_7MessageEE6Tuple1IS3_EE3RunEv
 }
 {
-  bug_57683
-  Memcheck:Leak
-  fun:_Znw*
-  fun:_ZN7logging17MakeCheckOpString*
-  fun:_ZN7logging11Check*Impl*
-  fun:_ZN7logging*LoggingTest_*_Test8TestBodyEv
-}
-{
    bug_57799
    Memcheck:Leak
    fun:_Znw*