Changing minidump process generation to be in-process on Android.

Because of UID isolation on Android, crash dump generation has to happen
in-process for renderers as well (as the browser cannot access all the
necessary states of the renderer process).
Breakpad has support for generating minidumps to a passed FD (as the renderer
process on Android does not have permission to create file), so the flow on
Android is:
- when a render process is created the browser creates a file and passes its FD
  to the process
- the renderer process initializes Breakpad with that FD
- if there is a crash, Breakpad generates the minidump to that FD.
- when the browser process detects a renderer stopped it checks the minidump
  file. If it's empty it deletes the file. If it's not empty, it means there
  was a crasher in which case it moves it to the crash dump folder for it to be
  picked up and uploaded by the Java side.
    
BUG=None
TEST=Test that minidumps are generated and uploaded when visiting about:crash
     and about:crashbrowserforrealz on Android and desktop Chrome.


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

[Jay Civelli, 2012-10-19 01:12:47]

Lei, could you look at the chrome part?
John, could you look at the content part?

[jam, 2012-10-19 16:39:17]

https://codereview.chromium.org/11189068/diff/5020/content/browser/child_proc...
File content/browser/child_process_launcher.cc (right):

https://codereview.chromium.org/11189068/diff/5020/content/browser/child_proc...
content/browser/child_process_launcher.cc:209: for
(content::FDInfoList::const_iterator i = files_to_register.begin();
nit: no need for const_iterator when iterating a vector; a
size_t i = 0; i < files_to_register.size(); ++i 
is more readable..

https://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_h...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.cc:1567: Details<const
content::FDInfoList>(&mapped_files));
this seems awkward for two reasons:
-it adds seemingly unrelated data to that notification (even though I understand
why you're doing it this way)
-it gives the embedder information (file mapping) that the embedder originally
gave content.

it seems that the ContentBrowserClient::GetAdditionalMappedFilesForChildProcess
call should just be changed to take the child_process_id so that it can
associate the mapping to the process that way.

https://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_h...
File content/browser/renderer_host/render_process_host_impl.h (right):

https://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.h:119: const
content::FDInfoList& mapped_files) OVERRIDE;
nit: you're in the content namespace already

https://codereview.chromium.org/11189068/diff/5020/content/public/browser/fil...
File content/public/browser/file_descriptor_info.h (right):

https://codereview.chromium.org/11189068/diff/5020/content/public/browser/fil...
content/public/browser/file_descriptor_info.h:27: typedef
std::vector<FileDescriptorInfo> FDInfoList;
nit: this is personal style, but it seems unnecessary to add this. i found the
old content code more readable when I see an std::vector<FileDescriptorInfo>
instead of FDInfoList (which really should be FileDescriptorInfoList)

[Lei Zhang, 2012-10-19 23:22:34]

Jay, do you think breakpad_linux.cc is getting sufficiently hairy that we should
separate Linux/CrOS from Android?

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/app/breakpad...
File chrome/app/breakpad_linux.h (right):

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/app/breakpad...
chrome/app/breakpad_linux.h:12: extern void InitNonBrowserCrashReporter(int
minidump_fd);
Rename to InitNonBrowserCrashReporterForAndroid to avoid confusion?

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/app/breakpad...
chrome/app/breakpad_linux.h:14: extern void InitNonBrowserCrashReporter();
Not needed - Linux/CrOS does not call this.

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/browser/chro...
File chrome/browser/chrome_content_browser_client.cc (right):

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/browser/chro...
chrome/browser/chrome_content_browser_client.cc:1802: #endif
nit: #endif  //defined(USE_LINUX_BREAKPAD)

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/browser/chro...
File chrome/browser/chrome_content_browser_client.h (right):

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/browser/chro...
chrome/browser/chrome_content_browser_client.h:26: #if defined(OS_ANDROID)
nit: this goes above the forward declarations.

https://chromiumcodereview.appspot.com/11189068/diff/5020/chrome/browser/chro...
chrome/browser/chrome_content_browser_client.h:244: scoped_ptr<CrashDumpManager>
crash_dump_manager_;
jam: is this the right place to put this?

[Jay Civelli, 2012-10-22 22:09:39]

On 2012/10/19 23:22:34, Lei Zhang wrote:
> Jay, do you think breakpad_linux.cc is getting sufficiently hairy that we
should
> separate Linux/CrOS from Android?
I am not sure yet. It is hairy but a lot of code is shared and overall that
class could use some TLC (some methods are super long).
Maybe a subsequent patch?

[Jay Civelli, 2012-10-22 22:09:48]

http://codereview.chromium.org/11189068/diff/5020/chrome/app/breakpad_linux.h
File chrome/app/breakpad_linux.h (right):

http://codereview.chromium.org/11189068/diff/5020/chrome/app/breakpad_linux.h...
chrome/app/breakpad_linux.h:12: extern void InitNonBrowserCrashReporter(int
minidump_fd);
On 2012/10/19 23:22:34, Lei Zhang wrote:
> Rename to InitNonBrowserCrashReporterForAndroid to avoid confusion?

Done.

http://codereview.chromium.org/11189068/diff/5020/chrome/app/breakpad_linux.h...
chrome/app/breakpad_linux.h:14: extern void InitNonBrowserCrashReporter();
On 2012/10/19 23:22:34, Lei Zhang wrote:
> Not needed - Linux/CrOS does not call this.

Done.

http://codereview.chromium.org/11189068/diff/5020/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_client.cc (right):

http://codereview.chromium.org/11189068/diff/5020/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_client.cc:1802: #endif
On 2012/10/19 23:22:34, Lei Zhang wrote:
> nit: #endif  //defined(USE_LINUX_BREAKPAD)

Done.

http://codereview.chromium.org/11189068/diff/5020/chrome/browser/chrome_conte...
File chrome/browser/chrome_content_browser_client.h (right):

http://codereview.chromium.org/11189068/diff/5020/chrome/browser/chrome_conte...
chrome/browser/chrome_content_browser_client.h:26: #if defined(OS_ANDROID)
On 2012/10/19 23:22:34, Lei Zhang wrote:
> nit: this goes above the forward declarations.

Done.

http://codereview.chromium.org/11189068/diff/5020/content/browser/child_proce...
File content/browser/child_process_launcher.cc (right):

http://codereview.chromium.org/11189068/diff/5020/content/browser/child_proce...
content/browser/child_process_launcher.cc:209: for
(content::FDInfoList::const_iterator i = files_to_register.begin();
On 2012/10/19 16:39:17, John Abd-El-Malek wrote:
> nit: no need for const_iterator when iterating a vector; a
> size_t i = 0; i < files_to_register.size(); ++i 
> is more readable..

Done.

http://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_ho...
File content/browser/renderer_host/render_process_host_impl.cc (right):

http://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_ho...
content/browser/renderer_host/render_process_host_impl.cc:1567: Details<const
content::FDInfoList>(&mapped_files));
On 2012/10/19 16:39:17, John Abd-El-Malek wrote:
> this seems awkward for two reasons:
> -it adds seemingly unrelated data to that notification (even though I
understand
> why you're doing it this way)
> -it gives the embedder information (file mapping) that the embedder originally
> gave content.
> 
> it seems that the
ContentBrowserClient::GetAdditionalMappedFilesForChildProcess
> call should just be changed to take the child_process_id so that it can
> associate the mapping to the process that way.

Changed per our discussion. Now using the process host ID in the crash handler.

http://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_ho...
File content/browser/renderer_host/render_process_host_impl.h (right):

http://codereview.chromium.org/11189068/diff/5020/content/browser/renderer_ho...
content/browser/renderer_host/render_process_host_impl.h:119: const
content::FDInfoList& mapped_files) OVERRIDE;
On 2012/10/19 16:39:17, John Abd-El-Malek wrote:
> nit: you're in the content namespace already
This change is gone.

http://codereview.chromium.org/11189068/diff/5020/content/public/browser/file...
File content/public/browser/file_descriptor_info.h (right):

http://codereview.chromium.org/11189068/diff/5020/content/public/browser/file...
content/public/browser/file_descriptor_info.h:27: typedef
std::vector<FileDescriptorInfo> FDInfoList;
On 2012/10/19 16:39:17, John Abd-El-Malek wrote:
> nit: this is personal style, but it seems unnecessary to add this. i found the
> old content code more readable when I see an std::vector<FileDescriptorInfo>
> instead of FDInfoList (which really should be FileDescriptorInfoList)

Done.

[jam, 2012-10-23 01:20:47]

content lgtm with nit

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
File chrome/browser/android/crash_dump_manager.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:36: 
nit: extra line

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_android.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_android.cc:30: 
nit: extra line

https://codereview.chromium.org/11189068/diff/14001/content/public/browser/co...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/11189068/diff/14001/content/public/browser/co...
content/public/browser/content_browser_client.h:460: int process_host_id,
nit: here and other places, "child_process_id" for consistency

[Lei Zhang, 2012-10-23 02:57:33]

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/breakpa...
File chrome/app/breakpad_linux.cc (left):

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/breakpa...
chrome/app/breakpad_linux.cc:1281: // We might be chrooted in a zygote or
renderer process so we cannot call
I don't think this block should be removed for non-Android?

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/breakpa...
File chrome/app/breakpad_linux.cc (right):

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/breakpa...
chrome/app/breakpad_linux.cc:1258: const char* failed_msg = "Failed to rename:";
There's a few instances of this. It's wrong. sizeof(failed_msg) below will
return the size of a char* and not the size of the string.

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/breakpa...
chrome/app/breakpad_linux.cc:1479:
base::SetLinuxDistro(switch_value.substr(separator + 1));
This seems silly. What does Android pass as the value to kEnableCrashReporter?

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/chrome_...
File chrome/app/chrome_main_delegate.cc (right):

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/app/chrome_...
chrome/app/chrome_main_delegate.cc:647: // generated as the renderer and browser
run  with different UIDs
nit: "run with"

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
File chrome/browser/android/crash_dump_manager.cc (right):

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.cc:7: #include <inttypes.h>
why do we need this?

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.cc:53: base::PlatformFile f =
nit: can we name the variable to something that's easier to search for? pf,
even.

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.cc:65:
DCHECK(process_host_id_to_minidump_info_.find(process_host_id) ==
nit: more readable with ContainsKey().

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.cc:80: DCHECK(r) << "Failed to
retrieve size for minidump " <<
nit: put the trailing << on the next line and line it up. Ditto below.

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.cc:128: content::ChildProcessData*
chil_process_data =
nit: chil_ -> child_ ?

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
File chrome/browser/android/crash_dump_manager.h (right):

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.h:11: #include
"base/memory/singleton.h"
nit: unused

https://chromiumcodereview.appspot.com/11189068/diff/14001/chrome/browser/and...
chrome/browser/android/crash_dump_manager.h:60: typedef std::map<int,
MinidumpInfo> ProcessHostIDToMinidumpInfo;
nit: goes near the top of the private section, not right before the first use.

[Jay Civelli, 2012-10-23 20:39:55]

https://codereview.chromium.org/11189068/diff/14001/chrome/app/breakpad_linux.cc
File chrome/app/breakpad_linux.cc (left):

https://codereview.chromium.org/11189068/diff/14001/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1281: // We might be chrooted in a zygote or
renderer process so we cannot call
On 2012/10/23 02:57:34, Lei Zhang wrote:
> I don't think this block should be removed for non-Android?
Oops, it's now back.

https://codereview.chromium.org/11189068/diff/14001/chrome/app/breakpad_linux.cc
File chrome/app/breakpad_linux.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1258: const char* failed_msg = "Failed to rename:";
On 2012/10/23 02:57:34, Lei Zhang wrote:
> There's a few instances of this. It's wrong. sizeof(failed_msg) below will
> return the size of a char* and not the size of the string.
Good point, changed the char* to char[] in all the places so sizeof returns the
correct sizes.

https://codereview.chromium.org/11189068/diff/14001/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1479:
base::SetLinuxDistro(switch_value.substr(separator + 1));
On 2012/10/23 02:57:34, Lei Zhang wrote:
> This seems silly. What does Android pass as the value to kEnableCrashReporter?
You are right, this is silly, we know it's Android and we set that in
EnableNonBrowserCrashDumping.
Simplified all this.

https://codereview.chromium.org/11189068/diff/14001/chrome/app/chrome_main_de...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/app/chrome_main_de...
chrome/app/chrome_main_delegate.cc:647: // generated as the renderer and browser
run  with different UIDs
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: "run with"

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
File chrome/browser/android/crash_dump_manager.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:7: #include <inttypes.h>
On 2012/10/23 02:57:34, Lei Zhang wrote:
> why do we need this?
We need it for the PRIx64.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:36: 
On 2012/10/23 01:20:47, John Abd-El-Malek wrote:
> nit: extra line

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:53: base::PlatformFile f =
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: can we name the variable to something that's easier to search for? pf,
> even.

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:53: base::PlatformFile f =
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: can we name the variable to something that's easier to search for? pf,
> even.

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:65:
DCHECK(process_host_id_to_minidump_info_.find(process_host_id) ==
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: more readable with ContainsKey().

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:80: DCHECK(r) << "Failed to
retrieve size for minidump " <<
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: put the trailing << on the next line and line it up. Ditto below.

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:128: content::ChildProcessData*
chil_process_data =
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: chil_ -> child_ ?

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
File chrome/browser/android/crash_dump_manager.h (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.h:11: #include
"base/memory/singleton.h"
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: unused

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.h:60: typedef std::map<int,
MinidumpInfo> ProcessHostIDToMinidumpInfo;
On 2012/10/23 02:57:34, Lei Zhang wrote:
> nit: goes near the top of the private section, not right before the first use.

Done.

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main_android.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main_android.cc:30: 
On 2012/10/23 01:20:47, John Abd-El-Malek wrote:
> nit: extra line

Done.

https://codereview.chromium.org/11189068/diff/14001/content/public/browser/co...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/11189068/diff/14001/content/public/browser/co...
content/public/browser/content_browser_client.h:460: int process_host_id,
On 2012/10/23 01:20:47, John Abd-El-Malek wrote:
> nit: here and other places, "child_process_id" for consistency

Done.

[Lei Zhang, 2012-10-23 21:34:57]

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
File chrome/browser/android/crash_dump_manager.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:7: #include <inttypes.h>
On 2012/10/23 20:39:55, Jay Civelli wrote:
> On 2012/10/23 02:57:34, Lei Zhang wrote:
> > why do we need this?
> We need it for the PRIx64.

Can we use base/format_macros.h instead?

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux.cc
File chrome/app/breakpad_linux.cc (right):

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:509: #if defined(OS_ANDROID)
can we put this closer to NonBrowserCrashHandler() below?

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:689: #if defined(OS_ANDROID)
This is now a defined(OS_ANDROID) inside a !defined(OS_ANDROID) block.

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1446: "InitNonBrowserCrashReporter in non browser
process.";
slightly better to say "... in " << process_type << " process.";

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1467: LOG(INFO) << "Non Browser crash dumping
enabled for: " << process_type;
VLOG() ?

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux.h
File chrome/app/breakpad_linux.h (right):

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.h:24: int fd;                          // File
descriptor to the Breakpad dump data.
Can you add a comment to explain the relationship between fd and filename?

[Jay Civelli, 2012-10-24 00:12:53]

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
File chrome/browser/android/crash_dump_manager.cc (right):

https://codereview.chromium.org/11189068/diff/14001/chrome/browser/android/cr...
chrome/browser/android/crash_dump_manager.cc:7: #include <inttypes.h>
On 2012/10/23 21:34:57, Lei Zhang wrote:
> On 2012/10/23 20:39:55, Jay Civelli wrote:
> > On 2012/10/23 02:57:34, Lei Zhang wrote:
> > > why do we need this?
> > We need it for the PRIx64.
> 
> Can we use base/format_macros.h instead?

Done.

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux.cc
File chrome/app/breakpad_linux.cc (right):

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:509: #if defined(OS_ANDROID)
On 2012/10/23 21:34:58, Lei Zhang wrote:
> can we put this closer to NonBrowserCrashHandler() below?

Done.

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:689: #if defined(OS_ANDROID)
On 2012/10/23 21:34:58, Lei Zhang wrote:
> This is now a defined(OS_ANDROID) inside a !defined(OS_ANDROID) block.
Removed. Also moved that function next to EnableNonBrowserCrashDumping where it
is used.

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1446: "InitNonBrowserCrashReporter in non browser
process.";
On 2012/10/23 21:34:58, Lei Zhang wrote:
> slightly better to say "... in " << process_type << " process.";

Done.

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.cc:1467: LOG(INFO) << "Non Browser crash dumping
enabled for: " << process_type;
On 2012/10/23 21:34:58, Lei Zhang wrote:
> VLOG() ?

Done.

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux.h
File chrome/app/breakpad_linux.h (right):

https://codereview.chromium.org/11189068/diff/11003/chrome/app/breakpad_linux...
chrome/app/breakpad_linux.h:24: int fd;                          // File
descriptor to the Breakpad dump data.
On 2012/10/23 21:34:58, Lei Zhang wrote:
> Can you add a comment to explain the relationship between fd and filename?

Done.

[Lei Zhang, 2012-10-24 05:20:47]

lgtm with some nits.

https://chromiumcodereview.appspot.com/11189068/diff/17023/chrome/app/breakpa...
File chrome/app/breakpad_linux.cc (right):

https://chromiumcodereview.appspot.com/11189068/diff/17023/chrome/app/breakpa...
chrome/app/breakpad_linux.cc:132: static void SetProcessStartTime() {
nit: doesn't need to be static inside of an anonymous namespace.

https://chromiumcodereview.appspot.com/11189068/diff/17023/chrome/app/breakpa...
chrome/app/breakpad_linux.cc:738: #endif
nit: #endif  // defined(OS_ANDROID)

https://chromiumcodereview.appspot.com/11189068/diff/17023/chrome/app/breakpa...
chrome/app/breakpad_linux.cc:1228: unsigned done_filename_len = filename_length
-1 + pid_str_length;
nit: -1 -> - 1

[commit-bot: I haz the power, 2012-10-24 20:09:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jcivelli@chromium.org/11189068/45001

[commit-bot: I haz the power, 2012-10-24 22:03:40]

Change committed as 163917