Allow Chromium Linux to be built with Breakpad. Enable Linux CHROME_HEADLESS ...

Allow Chromium Linux to be built with Breakpad. Enable Linux CHROME_HEADLESS support.

TEST=none
BUG=19663
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=24220

[Lei Zhang, 2009-08-19 22:59:05]

agl: primary reviewer
mmoss: any thoughts on the linux_chromium_breakpad gyp variable?

I probably still need to enable the dump_syms target on Chromium.

http://codereview.chromium.org/173095/diff/1/2
File build/common.gypi (right):

http://codereview.chromium.org/173095/diff/1/2#newcode120
Line 120: 'linux_chromium_breakpad%': 1,
whoops, this should default to 0.

[agl, 2009-08-19 23:05:22]

Code LGTM. I'll defer on the GYP changes.

http://codereview.chromium.org/173095/diff/1/4
File chrome/app/breakpad_linux.cc (right):

http://codereview.chromium.org/173095/diff/1/4#newcode116
Line 116: if (fd == -1) {
"if < 0" (then it matches below)

http://codereview.chromium.org/173095/diff/1/4#newcode124
Line 124: fd = sys_open(info.filename, O_WRONLY | O_EXCL, 0600);
You either want O_CREAT | O_EXCL or neither. Just EXCL is undefined.

http://codereview.chromium.org/173095/diff/1/4#newcode127
Line 127: sys_write(2, msg, sizeof(msg) - 1);
sys_close(ufd);

[Lei Zhang, 2009-08-20 19:06:55]

Added the symupload and dump_syms target, and simplified chrome.gyp.

http://codereview.chromium.org/173095/diff/1/4
File chrome/app/breakpad_linux.cc (right):

http://codereview.chromium.org/173095/diff/1/4#newcode116
Line 116: if (fd == -1) {
On 2009/08/19 23:05:22, agl wrote:
> "if < 0" (then it matches below)

Done.

http://codereview.chromium.org/173095/diff/1/4#newcode124
Line 124: fd = sys_open(info.filename, O_WRONLY | O_EXCL, 0600);
On 2009/08/19 23:05:22, agl wrote:
> You either want O_CREAT | O_EXCL or neither. Just EXCL is undefined.

Whoops, fixed.

http://codereview.chromium.org/173095/diff/1/4#newcode127
Line 127: sys_write(2, msg, sizeof(msg) - 1);
On 2009/08/19 23:05:22, agl wrote:
> sys_close(ufd);

Done.

[Michael Moss, 2009-08-20 19:17:38]

http://codereview.chromium.org/173095/diff/1005/1007
File build/common.gypi (right):

http://codereview.chromium.org/173095/diff/1005/1007#newcode120
Line 120: 'linux_chromium_breakpad%': 0,
chrome/chrome.gyp already creates a 'linux_breakpad' variable. Maybe that should
be moved to here instead of creating a new one.

[Lei Zhang, 2009-08-20 20:50:40]

http://codereview.chromium.org/173095/diff/1005/1007
File build/common.gypi (right):

http://codereview.chromium.org/173095/diff/1005/1007#newcode120
Line 120: 'linux_chromium_breakpad%': 0,
On 2009/08/20 19:17:39, mmoss wrote:
> chrome/chrome.gyp already creates a 'linux_breakpad' variable. Maybe that
should
> be moved to here instead of creating a new one.

That feels weird because we would then be able to define linux_breakpad=0 here,
but if we're doing an official build, that would override this.

[Lei Zhang, 2009-08-21 00:12:32]

On 2009/08/20 19:17:38, mmoss wrote:
> http://codereview.chromium.org/173095/diff/1005/1007
> File build/common.gypi (right):
> 
> http://codereview.chromium.org/173095/diff/1005/1007#newcode120
> Line 120: 'linux_chromium_breakpad%': 0,
> chrome/chrome.gyp already creates a 'linux_breakpad' variable. Maybe that
should
> be moved to here instead of creating a new one.

Ok, please take a look at patch set 4.

[Michael Moss, 2009-08-24 18:16:30]

lgtm

[laktar523, 2015-01-17 05:38:34]

On 2009/08/24 18:16:30, Michael Moss wrote:
> lgtm

[laktar523, 2015-01-17 05:39:18]

On 2009/08/19 22:59:05, Lei Zhang wrote:
> agl: primary reviewer
> mmoss: any thoughts on the linux_chromium_breakpad gyp variable?
> 
> I probably still need to enable the dump_syms target on Chromium.
> 
> http://codereview.chromium.org/173095/diff/1/2
> File build/common.gypi (right):
> 
> http://codereview.chromium.org/173095/diff/1/2#newcode120
> Line 120: 'linux_chromium_breakpad%': 1,
> whoops, this should default to 0.