child processes: wait 60 seconds before sending SIGALRM.

child processes: wait 60 seconds before sending SIGALRM.

This code is run under --child-clean-exit, which is useful when
profiling Chrome to generate data for optimization. Currently 30 seconds
is not enough time for Chrome to write profile data upon exit on slower
ChromeOS machines. Profile data is around 11k files that are written one
one by one.

This CL increases the timeout to 60 seconds.

BUG=239059
TEST=Collected PGO data on ChromeOS machines with --child-clean-exit.
git cl try.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215059

[asharif1, 2013-07-26 22:11:38]

Markus, while my other CL is pending
(https://codereview.chromium.org/19231006/), this CL along with a few PyAutoPerf
CLs is a short-term workaround for collecting PGO data.

Let me know if what you think of this.

[asharif1, 2013-07-30 14:21:45]

On 2013/07/26 22:11:38, asharif1 wrote:
> Markus, while my other CL is pending
> (https://codereview.chromium.org/19231006/), this CL along with a few
PyAutoPerf
> CLs is a short-term workaround for collecting PGO data.
> 
> Let me know if what you think of this.

markus, ping. We need this to collect PGO data to optimize Chrome on ChromeOS.

[Markus (顧孟勤), 2013-07-30 14:36:47]

lgtm

[commit-bot: I haz the power, 2013-07-30 21:02:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asharif@chromium.org/20842002/1

[commit-bot: I haz the power, 2013-07-31 00:01:44]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-07-31 03:39:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asharif@chromium.org/20842002/1

[commit-bot: I haz the power, 2013-07-31 04:17:38]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[asharif1, 2013-07-31 04:28:22]

darin@, you are one of the owners of this file. Can you approve this CL so I can
submit it? Thanks!

[asharif1, 2013-07-31 18:21:10]

Adding avi@ as avi@ is one of the OWNERS of content/. avi@ can you please review
this CL?

Thanks,

[Avi (use Gerrit), 2013-08-01 15:17:28]

LGTM, being nitpicky about the comment.

https://codereview.chromium.org/20842002/diff/1/content/child/child_thread.cc
File content/child/child_thread.cc (right):

https://codereview.chromium.org/20842002/diff/1/content/child/child_thread.cc...
content/child/child_thread.cc:76: // application.
This comment seems a bit untrue. It says "We want to kill this process after
giving it 60 seconds" yet there is an if() in which the false branch it doesn't
wait.

Perhaps a little rewording?

"If a clean exit is requested, we want to kill this process after giving it 60
seconds to run the exit handlers and write whatever it needs to disk."

and move the discussion about SIGALRM to be on the alarm call.

[asharif1, 2013-08-01 15:22:15]

https://codereview.chromium.org/20842002/diff/1/content/child/child_thread.cc
File content/child/child_thread.cc (right):

https://codereview.chromium.org/20842002/diff/1/content/child/child_thread.cc...
content/child/child_thread.cc:76: // application.
On 2013/08/01 15:17:28, Avi wrote:
> This comment seems a bit untrue. It says "We want to kill this process after
> giving it 60 seconds" yet there is an if() in which the false branch it
doesn't
> wait.
> 
> Perhaps a little rewording?
> 
> "If a clean exit is requested, we want to kill this process after giving it 60
> seconds to run the exit handlers and write whatever it needs to disk."
> 
> and move the discussion about SIGALRM to be on the alarm call.

I don't know if I need braces here. Do you?

[Avi (use Gerrit), 2013-08-01 15:29:42]

SLGTM

https://codereview.chromium.org/20842002/diff/21001/content/child/child_threa...
File content/child/child_thread.cc (right):

https://codereview.chromium.org/20842002/diff/21001/content/child/child_threa...
content/child/child_thread.cc:76: // profile data to disk (which happens under
profile collection mode).
Yeah, you need braces here. Even if there's only one line of code, there's more
than one line of stuff.

[asharif1, 2013-08-01 15:33:42]

Also tweaked comment just a little.

https://codereview.chromium.org/20842002/diff/21001/content/child/child_threa...
File content/child/child_thread.cc (right):

https://codereview.chromium.org/20842002/diff/21001/content/child/child_threa...
content/child/child_thread.cc:76: // profile data to disk (which happens under
profile collection mode).
On 2013/08/01 15:29:43, Avi wrote:
> Yeah, you need braces here. Even if there's only one line of code, there's
more
> than one line of stuff.

Done.

[commit-bot: I haz the power, 2013-08-01 15:36:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asharif@chromium.org/20842002/30001

[commit-bot: I haz the power, 2013-08-01 17:44:00]

Change committed as 215059