Cherrypick of r34036 to make merge at r34474 build....

Cherrypick of r34036 to make merge at r34474 build.

Merging only changes to base/logging.* (for RAW_LOG()/RAW_CHECK()), and NOT changes to chrome/browser/browser_main.cc.

Original CL description:

Make POSIX SIGTERM/SIGINT/SIGHUP handler async signal safe.
* Don't use LOG/CHECK.  Replace with RAW_LOG/DCHECK (newly added to logging.h)
* Don't directly post a task to the UI loop.  Write to a magic pipe.  Read this from a separate thread which will post to a task to the UI loop.
BUG=http://crbug.com/29240

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

TBR=viettrungluu@chromium.org,mark@chromium.org,willchan@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=34499

[willchan no longer on Chromium, 2009-12-15 05:04:42]

I don't really understand this.  The logging changes weren't made
separately from others.  Why is there just these two files?  Was a
previous merge done incorrectly?

On Mon, Dec 14, 2009 at 1:38 PM,  <viettrungluu@chromium.org> wrote:
> Reviewers: willchan,
>
> Description:
> Cherrypick of r34036 to make merge at r34474 build.
>
> Original CL description:
>
> Make POSIX SIGTERM/SIGINT/SIGHUP handler async signal safe.
> * Don't use LOG/CHECK.  Replace with RAW_LOG/DCHECK (newly added to
> logging.h)
> * Don't directly post a task to the UI loop.  Write to a magic pipe.  Read
> this
> from a separate thread which will post to a task to the UI loop.
> BUG=http://crbug.com/29240
>
> Review URL: http://codereview.chromium.org/460094
>
> TBR=viettrungluu@chromium.org,mark@chromium.org,willchan@chromium.org
>
> Please review this at http://codereview.chromium.org/501004
>
> SVN Base: svn://svn.chromium.org/chrome/branches/249/src/
>
> Affected files:
>  M     base/logging.h
>  M     base/logging.cc
>
>
> Index: base/logging.cc
> ===================================================================
> --- base/logging.cc     (revision 34497)
> +++ base/logging.cc     (working copy)
> @@ -5,9 +5,14 @@
>  #include "base/logging.h"
>
>  #if defined(OS_WIN)
> +#include <io.h>
>  #include <windows.h>
>  typedef HANDLE FileHandle;
>  typedef HANDLE MutexHandle;
> +// Windows warns on using write().  It prefers _write().
> +#define write(fd, buf, count) _write(fd, buf, static_cast<unsigned
> int>(count))
> +// Windows doesn't define STDERR_FILENO.  Define it here.
> +#define STDERR_FILENO 2
>  #elif defined(OS_MACOSX)
>  #include <CoreFoundation/CoreFoundation.h>
>  #include <mach/mach.h>
> @@ -37,6 +42,7 @@
>  #include "base/base_switches.h"
>  #include "base/command_line.h"
>  #include "base/debug_util.h"
> +#include "base/eintr_wrapper.h"
>  #include "base/lock_impl.h"
>  #if defined(OS_POSIX)
>  #include "base/safe_strerror_posix.h"
> @@ -667,6 +673,37 @@
>   log_file = NULL;
>  }
>
> +void RawLog(int level, const char* message) {
> +  if (level >= min_log_level) {
> +    size_t bytes_written = 0;
> +    const size_t message_len = strlen(message);
> +    int rv;
> +    while (bytes_written < message_len) {
> +      rv = HANDLE_EINTR(
> +          write(STDERR_FILENO, message + bytes_written,
> +                message_len - bytes_written));
> +      if (rv < 0) {
> +        // Give up, nothing we can do now.
> +        break;
> +      }
> +      bytes_written += rv;
> +    }
> +
> +    if (message_len > 0 && message[message_len - 1] != '\n') {
> +      do {
> +        rv = HANDLE_EINTR(write(STDERR_FILENO, "\n", 1));
> +        if (rv < 0) {
> +          // Give up, nothing we can do now.
> +          break;
> +        }
> +      } while (rv != 1);
> +    }
> +  }
> +
> +  if (level == LOG_FATAL)
> +    DebugUtil::BreakDebugger();
> +}
> +
>  }  // namespace logging
>
>  std::ostream& operator<<(std::ostream& out, const wchar_t* wstr) {
> Index: base/logging.h
> ===================================================================
> --- base/logging.h      (revision 34497)
> +++ base/logging.h      (working copy)
> @@ -765,6 +765,17 @@
>  //       after this call.
>  void CloseLogFile();
>
> +// Async signal safe logging mechanism.
> +void RawLog(int level, const char* message);
> +
> +#define RAW_LOG(level, message) logging::RawLog(logging::LOG_ ## level,
> message)
> +
> +#define RAW_CHECK(condition)
>     \
> +  do {
>     \
> +    if (!(condition))
>    \
> +      logging::RawLog(logging::LOG_FATAL, "Check failed: " #condition
> "\n");   \
> +  } while (0)
> +
>  }  // namespace logging
>
>  // These functions are provided as a convenience for logging, which is
> where we
>
>
>

[viettrungluu, 2009-12-15 05:15:49]

I borrowed some code to add a signal handler to the renderer (on Mac), 
to fix a critical problem we're having on the current beta channel. 
(This is a stopgap, with the real fix to fix Breakpad.) The code I 
borrowed required RAW_LOG/RAW_CHECK, so I had to bring that into 249 also.

- Trung

William Chan (陈智昌) wrote:
> I don't really understand this.  The logging changes weren't made
> separately from others.  Why is there just these two files?  Was a
> previous merge done incorrectly?
> 
> On Mon, Dec 14, 2009 at 1:38 PM,  <viettrungluu@chromium.org> wrote:
>> Reviewers: willchan,
>>
>> Description:
>> Cherrypick of r34036 to make merge at r34474 build.
>>
>> Original CL description:
>>
>> Make POSIX SIGTERM/SIGINT/SIGHUP handler async signal safe.
>> * Don't use LOG/CHECK.  Replace with RAW_LOG/DCHECK (newly added to
>> logging.h)
>> * Don't directly post a task to the UI loop.  Write to a magic pipe.  Read
>> this
>> from a separate thread which will post to a task to the UI loop.
>> BUG=http://crbug.com/29240
>>
>> Review URL: http://codereview.chromium.org/460094
>>
>> TBR=viettrungluu@chromium.org,mark@chromium.org,willchan@chromium.org
>>
>> Please review this at http://codereview.chromium.org/501004
>>
>> SVN Base: svn://svn.chromium.org/chrome/branches/249/src/
>>
>> Affected files:
>>  M     base/logging.h
>>  M     base/logging.cc
>>
>>
>> Index: base/logging.cc
>> ===================================================================
>> --- base/logging.cc     (revision 34497)
>> +++ base/logging.cc     (working copy)
>> @@ -5,9 +5,14 @@
>>  #include "base/logging.h"
>>
>>  #if defined(OS_WIN)
>> +#include <io.h>
>>  #include <windows.h>
>>  typedef HANDLE FileHandle;
>>  typedef HANDLE MutexHandle;
>> +// Windows warns on using write().  It prefers _write().
>> +#define write(fd, buf, count) _write(fd, buf, static_cast<unsigned
>> int>(count))
>> +// Windows doesn't define STDERR_FILENO.  Define it here.
>> +#define STDERR_FILENO 2
>>  #elif defined(OS_MACOSX)
>>  #include <CoreFoundation/CoreFoundation.h>
>>  #include <mach/mach.h>
>> @@ -37,6 +42,7 @@
>>  #include "base/base_switches.h"
>>  #include "base/command_line.h"
>>  #include "base/debug_util.h"
>> +#include "base/eintr_wrapper.h"
>>  #include "base/lock_impl.h"
>>  #if defined(OS_POSIX)
>>  #include "base/safe_strerror_posix.h"
>> @@ -667,6 +673,37 @@
>>   log_file = NULL;
>>  }
>>
>> +void RawLog(int level, const char* message) {
>> +  if (level >= min_log_level) {
>> +    size_t bytes_written = 0;
>> +    const size_t message_len = strlen(message);
>> +    int rv;
>> +    while (bytes_written < message_len) {
>> +      rv = HANDLE_EINTR(
>> +          write(STDERR_FILENO, message + bytes_written,
>> +                message_len - bytes_written));
>> +      if (rv < 0) {
>> +        // Give up, nothing we can do now.
>> +        break;
>> +      }
>> +      bytes_written += rv;
>> +    }
>> +
>> +    if (message_len > 0 && message[message_len - 1] != '\n') {
>> +      do {
>> +        rv = HANDLE_EINTR(write(STDERR_FILENO, "\n", 1));
>> +        if (rv < 0) {
>> +          // Give up, nothing we can do now.
>> +          break;
>> +        }
>> +      } while (rv != 1);
>> +    }
>> +  }
>> +
>> +  if (level == LOG_FATAL)
>> +    DebugUtil::BreakDebugger();
>> +}
>> +
>>  }  // namespace logging
>>
>>  std::ostream& operator<<(std::ostream& out, const wchar_t* wstr) {
>> Index: base/logging.h
>> ===================================================================
>> --- base/logging.h      (revision 34497)
>> +++ base/logging.h      (working copy)
>> @@ -765,6 +765,17 @@
>>  //       after this call.
>>  void CloseLogFile();
>>
>> +// Async signal safe logging mechanism.
>> +void RawLog(int level, const char* message);
>> +
>> +#define RAW_LOG(level, message) logging::RawLog(logging::LOG_ ## level,
>> message)
>> +
>> +#define RAW_CHECK(condition)
>>     \
>> +  do {
>>     \
>> +    if (!(condition))
>>    \
>> +      logging::RawLog(logging::LOG_FATAL, "Check failed: " #condition
>> "\n");   \
>> +  } while (0)
>> +
>>  }  // namespace logging
>>
>>  // These functions are provided as a convenience for logging, which is
>> where we
>>
>>
>>
> 

[willchan no longer on Chromium, 2009-12-15 05:18:08]

Ok, understood.

On Mon, Dec 14, 2009 at 9:15 PM, Viet-Trung Luu
<viettrungluu@chromium.org> wrote:
> I borrowed some code to add a signal handler to the renderer (on Mac), to
> fix a critical problem we're having on the current beta channel. (This is a
> stopgap, with the real fix to fix Breakpad.) The code I borrowed required
> RAW_LOG/RAW_CHECK, so I had to bring that into 249 also.
>
> - Trung
>
> William Chan (陈智昌) wrote:
>>
>> I don't really understand this.  The logging changes weren't made
>> separately from others.  Why is there just these two files?  Was a
>> previous merge done incorrectly?
>>
>> On Mon, Dec 14, 2009 at 1:38 PM,  <viettrungluu@chromium.org> wrote:
>>>
>>> Reviewers: willchan,
>>>
>>> Description:
>>> Cherrypick of r34036 to make merge at r34474 build.
>>>
>>> Original CL description:
>>>
>>> Make POSIX SIGTERM/SIGINT/SIGHUP handler async signal safe.
>>> * Don't use LOG/CHECK.  Replace with RAW_LOG/DCHECK (newly added to
>>> logging.h)
>>> * Don't directly post a task to the UI loop.  Write to a magic pipe.
>>>  Read
>>> this
>>> from a separate thread which will post to a task to the UI loop.
>>> BUG=http://crbug.com/29240
>>>
>>> Review URL: http://codereview.chromium.org/460094
>>>
>>> TBR=viettrungluu@chromium.org,mark@chromium.org,willchan@chromium.org
>>>
>>> Please review this at http://codereview.chromium.org/501004
>>>
>>> SVN Base: svn://svn.chromium.org/chrome/branches/249/src/
>>>
>>> Affected files:
>>>  M     base/logging.h
>>>  M     base/logging.cc
>>>
>>>
>>> Index: base/logging.cc
>>> ===================================================================
>>> --- base/logging.cc     (revision 34497)
>>> +++ base/logging.cc     (working copy)
>>> @@ -5,9 +5,14 @@
>>>  #include "base/logging.h"
>>>
>>>  #if defined(OS_WIN)
>>> +#include <io.h>
>>>  #include <windows.h>
>>>  typedef HANDLE FileHandle;
>>>  typedef HANDLE MutexHandle;
>>> +// Windows warns on using write().  It prefers _write().
>>> +#define write(fd, buf, count) _write(fd, buf, static_cast<unsigned
>>> int>(count))
>>> +// Windows doesn't define STDERR_FILENO.  Define it here.
>>> +#define STDERR_FILENO 2
>>>  #elif defined(OS_MACOSX)
>>>  #include <CoreFoundation/CoreFoundation.h>
>>>  #include <mach/mach.h>
>>> @@ -37,6 +42,7 @@
>>>  #include "base/base_switches.h"
>>>  #include "base/command_line.h"
>>>  #include "base/debug_util.h"
>>> +#include "base/eintr_wrapper.h"
>>>  #include "base/lock_impl.h"
>>>  #if defined(OS_POSIX)
>>>  #include "base/safe_strerror_posix.h"
>>> @@ -667,6 +673,37 @@
>>>  log_file = NULL;
>>>  }
>>>
>>> +void RawLog(int level, const char* message) {
>>> +  if (level >= min_log_level) {
>>> +    size_t bytes_written = 0;
>>> +    const size_t message_len = strlen(message);
>>> +    int rv;
>>> +    while (bytes_written < message_len) {
>>> +      rv = HANDLE_EINTR(
>>> +          write(STDERR_FILENO, message + bytes_written,
>>> +                message_len - bytes_written));
>>> +      if (rv < 0) {
>>> +        // Give up, nothing we can do now.
>>> +        break;
>>> +      }
>>> +      bytes_written += rv;
>>> +    }
>>> +
>>> +    if (message_len > 0 && message[message_len - 1] != '\n') {
>>> +      do {
>>> +        rv = HANDLE_EINTR(write(STDERR_FILENO, "\n", 1));
>>> +        if (rv < 0) {
>>> +          // Give up, nothing we can do now.
>>> +          break;
>>> +        }
>>> +      } while (rv != 1);
>>> +    }
>>> +  }
>>> +
>>> +  if (level == LOG_FATAL)
>>> +    DebugUtil::BreakDebugger();
>>> +}
>>> +
>>>  }  // namespace logging
>>>
>>>  std::ostream& operator<<(std::ostream& out, const wchar_t* wstr) {
>>> Index: base/logging.h
>>> ===================================================================
>>> --- base/logging.h      (revision 34497)
>>> +++ base/logging.h      (working copy)
>>> @@ -765,6 +765,17 @@
>>>  //       after this call.
>>>  void CloseLogFile();
>>>
>>> +// Async signal safe logging mechanism.
>>> +void RawLog(int level, const char* message);
>>> +
>>> +#define RAW_LOG(level, message) logging::RawLog(logging::LOG_ ## level,
>>> message)
>>> +
>>> +#define RAW_CHECK(condition)
>>>    \
>>> +  do {
>>>    \
>>> +    if (!(condition))
>>>   \
>>> +      logging::RawLog(logging::LOG_FATAL, "Check failed: " #condition
>>> "\n");   \
>>> +  } while (0)
>>> +
>>>  }  // namespace logging
>>>
>>>  // These functions are provided as a convenience for logging, which is
>>> where we
>>>
>>>
>>>
>>
>
>

[willchan no longer on Chromium, 2009-12-15 05:18:24]

lgtm

On Mon, Dec 14, 2009 at 9:17 PM, William Chan (陈智昌)
<willchan@chromium.org> wrote:
> Ok, understood.
>
> On Mon, Dec 14, 2009 at 9:15 PM, Viet-Trung Luu
> <viettrungluu@chromium.org> wrote:
>> I borrowed some code to add a signal handler to the renderer (on Mac), to
>> fix a critical problem we're having on the current beta channel. (This is a
>> stopgap, with the real fix to fix Breakpad.) The code I borrowed required
>> RAW_LOG/RAW_CHECK, so I had to bring that into 249 also.
>>
>> - Trung
>>
>> William Chan (陈智昌) wrote:
>>>
>>> I don't really understand this.  The logging changes weren't made
>>> separately from others.  Why is there just these two files?  Was a
>>> previous merge done incorrectly?
>>>
>>> On Mon, Dec 14, 2009 at 1:38 PM,  <viettrungluu@chromium.org> wrote:
>>>>
>>>> Reviewers: willchan,
>>>>
>>>> Description:
>>>> Cherrypick of r34036 to make merge at r34474 build.
>>>>
>>>> Original CL description:
>>>>
>>>> Make POSIX SIGTERM/SIGINT/SIGHUP handler async signal safe.
>>>> * Don't use LOG/CHECK.  Replace with RAW_LOG/DCHECK (newly added to
>>>> logging.h)
>>>> * Don't directly post a task to the UI loop.  Write to a magic pipe.
>>>>  Read
>>>> this
>>>> from a separate thread which will post to a task to the UI loop.
>>>> BUG=http://crbug.com/29240
>>>>
>>>> Review URL: http://codereview.chromium.org/460094
>>>>
>>>> TBR=viettrungluu@chromium.org,mark@chromium.org,willchan@chromium.org
>>>>
>>>> Please review this at http://codereview.chromium.org/501004
>>>>
>>>> SVN Base: svn://svn.chromium.org/chrome/branches/249/src/
>>>>
>>>> Affected files:
>>>>  M     base/logging.h
>>>>  M     base/logging.cc
>>>>
>>>>
>>>> Index: base/logging.cc
>>>> ===================================================================
>>>> --- base/logging.cc     (revision 34497)
>>>> +++ base/logging.cc     (working copy)
>>>> @@ -5,9 +5,14 @@
>>>>  #include "base/logging.h"
>>>>
>>>>  #if defined(OS_WIN)
>>>> +#include <io.h>
>>>>  #include <windows.h>
>>>>  typedef HANDLE FileHandle;
>>>>  typedef HANDLE MutexHandle;
>>>> +// Windows warns on using write().  It prefers _write().
>>>> +#define write(fd, buf, count) _write(fd, buf, static_cast<unsigned
>>>> int>(count))
>>>> +// Windows doesn't define STDERR_FILENO.  Define it here.
>>>> +#define STDERR_FILENO 2
>>>>  #elif defined(OS_MACOSX)
>>>>  #include <CoreFoundation/CoreFoundation.h>
>>>>  #include <mach/mach.h>
>>>> @@ -37,6 +42,7 @@
>>>>  #include "base/base_switches.h"
>>>>  #include "base/command_line.h"
>>>>  #include "base/debug_util.h"
>>>> +#include "base/eintr_wrapper.h"
>>>>  #include "base/lock_impl.h"
>>>>  #if defined(OS_POSIX)
>>>>  #include "base/safe_strerror_posix.h"
>>>> @@ -667,6 +673,37 @@
>>>>  log_file = NULL;
>>>>  }
>>>>
>>>> +void RawLog(int level, const char* message) {
>>>> +  if (level >= min_log_level) {
>>>> +    size_t bytes_written = 0;
>>>> +    const size_t message_len = strlen(message);
>>>> +    int rv;
>>>> +    while (bytes_written < message_len) {
>>>> +      rv = HANDLE_EINTR(
>>>> +          write(STDERR_FILENO, message + bytes_written,
>>>> +                message_len - bytes_written));
>>>> +      if (rv < 0) {
>>>> +        // Give up, nothing we can do now.
>>>> +        break;
>>>> +      }
>>>> +      bytes_written += rv;
>>>> +    }
>>>> +
>>>> +    if (message_len > 0 && message[message_len - 1] != '\n') {
>>>> +      do {
>>>> +        rv = HANDLE_EINTR(write(STDERR_FILENO, "\n", 1));
>>>> +        if (rv < 0) {
>>>> +          // Give up, nothing we can do now.
>>>> +          break;
>>>> +        }
>>>> +      } while (rv != 1);
>>>> +    }
>>>> +  }
>>>> +
>>>> +  if (level == LOG_FATAL)
>>>> +    DebugUtil::BreakDebugger();
>>>> +}
>>>> +
>>>>  }  // namespace logging
>>>>
>>>>  std::ostream& operator<<(std::ostream& out, const wchar_t* wstr) {
>>>> Index: base/logging.h
>>>> ===================================================================
>>>> --- base/logging.h      (revision 34497)
>>>> +++ base/logging.h      (working copy)
>>>> @@ -765,6 +765,17 @@
>>>>  //       after this call.
>>>>  void CloseLogFile();
>>>>
>>>> +// Async signal safe logging mechanism.
>>>> +void RawLog(int level, const char* message);
>>>> +
>>>> +#define RAW_LOG(level, message) logging::RawLog(logging::LOG_ ## level,
>>>> message)
>>>> +
>>>> +#define RAW_CHECK(condition)
>>>>    \
>>>> +  do {
>>>>    \
>>>> +    if (!(condition))
>>>>   \
>>>> +      logging::RawLog(logging::LOG_FATAL, "Check failed: " #condition
>>>> "\n");   \
>>>> +  } while (0)
>>>> +
>>>>  }  // namespace logging
>>>>
>>>>  // These functions are provided as a convenience for logging, which is
>>>> where we
>>>>
>>>>
>>>>
>>>
>>
>>
>