Handled nullptr argument in WaitForExit() and WaitForExitWithTimeout() methods.

Handled nullptr argument passing in WaitForExit() and WaitForExitWithTimeout() methods. No need to pass exit_code in above methods if exit_code is not required.

BUG=417532
Committed: https://crrev.com/fd19e23a8a1e0d390706ad53470afb6d77f88847
Cr-Commit-Position: refs/heads/master@{#331936}

[g.mehndiratt, 2015-04-24 10:28:59]

Please review changes for Point 2.

[Peter Kasting, 2015-04-24 21:00:32]

On 2015/04/24 10:28:59, g.mehndiratt wrote:
> Please review changes for Point 2.

You didn't set any reviewers for your change.

[g.mehndiratt, 2015-04-27 04:44:54]

g.mehndiratt@samsung.com changed reviewers:
+ rvargas@chromium.org

[g.mehndiratt, 2015-04-27 04:44:54]

Please review the patch.

[g.mehndiratt, 2015-04-27 09:06:33]

g.mehndiratt@samsung.com changed reviewers:
+ danakj@chromium.org
- rvargas@chromium.org

[g.mehndiratt, 2015-04-27 09:06:34]

Please review the patch.

[danakj, 2015-04-27 16:52:51]

danakj@chromium.org changed reviewers:
+ rvargas@chromium.org
- danakj@chromium.org

[danakj, 2015-04-27 16:52:51]

+rvargas

[g.mehndiratt, 2015-04-29 06:35:04]

g.mehndiratt@samsung.com changed reviewers:
+ thestig@chromium.org

[g.mehndiratt, 2015-04-29 06:35:04]

Added another reviewer as rvargas is not available.

[Lei Zhang, 2015-04-29 07:52:33]

https://codereview.chromium.org/1086363003/diff/1/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/1/base/process/process.h#newc...
base/process/process.h:105: bool WaitForExit(int* exit_code = NULL);
This is against the style guide.
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Default_Argu...

You may want to just wait for rvargas to get back to work out what to do here.

[rvargas (doing something else), 2015-04-30 23:28:27]

https://codereview.chromium.org/1086363003/diff/1/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/1/base/process/process.h#newc...
base/process/process.h:105: bool WaitForExit(int* exit_code = NULL);
On 2015/04/29 07:52:32, Lei Zhang wrote:
> This is against the style guide.
>
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Default_Argu...
> 
> You may want to just wait for rvargas to get back to work out what to do here.

Right. I didn't mean to have a default argument, just to accept nullptr as a
valid input.

It's going to be a while before I go back so it's better not to wait for me :).

[g.mehndiratt, 2015-05-04 06:40:21]

Review comments incorporated.

https://codereview.chromium.org/1086363003/diff/1/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/1/base/process/process.h#newc...
base/process/process.h:105: bool WaitForExit(int* exit_code = NULL);
On 2015/04/30 23:28:26, rvargas (out of office) wrote:

I will corporate comments.

If u r not available can u tell whom to add for review?

[g.mehndiratt, 2015-05-04 07:25:43]

g.mehndiratt@samsung.com changed reviewers:
- rvargas@chromium.org

[g.mehndiratt, 2015-05-04 07:25:44]

Please review.

[Lei Zhang, 2015-05-05 00:44:00]

- You should update the header file to indicate these parameters are options.
- Can you find the callers that would actually benefit from this change, and fix
them?

https://codereview.chromium.org/1086363003/diff/20001/base/process/process_wi...
File base/process/process_win.cc (right):

https://codereview.chromium.org/1086363003/diff/20001/base/process/process_wi...
base/process/process_win.cc:157: *exit_code = temp_code;
bad indentation; same problem in process_posix.cc

[g.mehndiratt, 2015-05-06 07:18:01]

Please review.

https://codereview.chromium.org/1086363003/diff/20001/base/process/process_wi...
File base/process/process_win.cc (right):

https://codereview.chromium.org/1086363003/diff/20001/base/process/process_wi...
base/process/process_win.cc:157: *exit_code = temp_code;
On 2015/05/05 00:44:00, Lei Zhang wrote:

Acknowledged.

[Lei Zhang, 2015-05-06 08:05:15]

Please change the CL description so it makes sense.

https://codereview.chromium.org/1086363003/diff/40001/DEPS
File DEPS (right):

https://codereview.chromium.org/1086363003/diff/40001/DEPS#newcode37
DEPS:37: 'webkit_revision': 'c127d37233c824ba96d9fdba5bf4db8f1311fd0d', # from
svn revision 194850
Umm, should this be here?

https://codereview.chromium.org/1086363003/diff/40001/ash/test/test_metro_vie...
File ash/test/test_metro_viewer_process_host.cc (right):

https://codereview.chromium.org/1086363003/diff/40001/ash/test/test_metro_vie...
ash/test/test_metro_viewer_process_host.cc:33: viewer_process.WaitForExit(NULL);
NULL -> nullptr

Search for "nullptr" on https://chromium-cpp.appspot.com/

https://codereview.chromium.org/1086363003/diff/40001/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/40001/base/process/process.h#...
base/process/process.h:104: // any process. exit_code is optional if exit_code
is not required NULL can be
We refer to variables as |variable_name|. See line 98 for example.

https://codereview.chromium.org/1086363003/diff/40001/base/process/process.h#...
base/process/process.h:104: // any process. exit_code is optional if exit_code
is not required NULL can be
The grammar here needs some work.

[g.mehndiratt, 2015-05-06 10:52:35]

Review comments incorporated.Please review.

https://codereview.chromium.org/1086363003/diff/40001/DEPS
File DEPS (right):

https://codereview.chromium.org/1086363003/diff/40001/DEPS#newcode37
DEPS:37: 'webkit_revision': 'c127d37233c824ba96d9fdba5bf4db8f1311fd0d', # from
svn revision 194850
On 2015/05/06 08:05:14, Lei Zhang wrote:

Acknowledged.

https://codereview.chromium.org/1086363003/diff/40001/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/40001/base/process/process.h#...
base/process/process.h:104: // any process. exit_code is optional if exit_code
is not required NULL can be
On 2015/05/06 08:05:15, Lei Zhang wrote:

Acknowledged.

https://codereview.chromium.org/1086363003/diff/40001/base/process/process.h#...
base/process/process.h:104: // any process. exit_code is optional if exit_code
is not required NULL can be
On 2015/05/06 08:05:15, Lei Zhang wrote:

Acknowledged.

[Lei Zhang, 2015-05-06 21:38:00]

Again, [lease change the CL description so it makes sense.

https://codereview.chromium.org/1086363003/diff/60001/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/60001/base/process/process.h#...
base/process/process.h:105: // NOTE: |exit_code| is optional, nullptr can be
passed if exit code is not
"if the exit code"

https://codereview.chromium.org/1086363003/diff/60001/base/process/process.h#...
base/process/process.h:106: // required
Please end the sentence with a period.

[Lei Zhang, 2015-05-06 21:38:14]

On 2015/05/06 21:38:00, Lei Zhang wrote:
> Again, [lease change the CL description so it makes sense.

*please* change...

[g.mehndiratt, 2015-05-21 09:29:52]

Review comments incorporated.I have changed CL description and it is updated in
description section.Please review.

https://codereview.chromium.org/1086363003/diff/60001/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/60001/base/process/process.h#...
base/process/process.h:105: // NOTE: |exit_code| is optional, nullptr can be
passed if exit code is not
On 2015/05/06 21:38:00, Lei Zhang wrote:

Acknowledged.

https://codereview.chromium.org/1086363003/diff/60001/base/process/process.h#...
base/process/process.h:106: // required
On 2015/05/06 21:38:00, Lei Zhang wrote:

Acknowledged.

[Lei Zhang, 2015-05-21 19:53:15]

For the 3rd time, please change the CL description so it makes sense.

https://codereview.chromium.org/1086363003/diff/80001/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/80001/base/process/process.h#...
base/process/process.h:106: //if the exit code is not required.
Put a space after //, to be consistent with the rest of the file.

There's still room on the previous line... use it?

[g.mehndiratt, 2015-05-27 05:30:21]

On 2015/05/21 19:53:15, Lei Zhang wrote:

Hello Mr. Lei Zhang,

Apology for delay in updation of CL description. Hope this description is
relevant.

[g.mehndiratt, 2015-05-27 05:30:43]

Review comments incorporated.

Please review.

https://codereview.chromium.org/1086363003/diff/80001/base/process/process.h
File base/process/process.h (right):

https://codereview.chromium.org/1086363003/diff/80001/base/process/process.h#...
base/process/process.h:106: //if the exit code is not required.
On 2015/05/21 19:53:15, Lei Zhang wrote:

Acknowledged.

[Lei Zhang, 2015-05-27 06:42:49]

You have ".." in the CL description, but it should be a period and a space. I
have a small nit below. Otherwise lgtm.

https://codereview.chromium.org/1086363003/diff/100001/base/process/process_w...
File base/process/process_win.cc (right):

https://codereview.chromium.org/1086363003/diff/100001/base/process/process_w...
base/process/process_win.cc:156: if (exit_code)
Can you add a blank line above the if statement?

[g.mehndiratt, 2015-05-27 07:28:35]

Review comments incorporated.

https://codereview.chromium.org/1086363003/diff/100001/base/process/process_w...
File base/process/process_win.cc (right):

https://codereview.chromium.org/1086363003/diff/100001/base/process/process_w...
base/process/process_win.cc:156: if (exit_code)
On 2015/05/27 06:42:49, Lei Zhang wrote:

Acknowledged.

[Lei Zhang, 2015-05-27 07:38:42]

lgtm

[g.mehndiratt, 2015-05-27 08:18:36]

The CQ bit was checked by g.mehndiratt@samsung.com

[commit-bot: I haz the power, 2015-05-27 08:18:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1086363003/120001

[commit-bot: I haz the power, 2015-05-27 08:25:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-27 08:25:30]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[g.mehndiratt, 2015-05-27 08:29:59]

g.mehndiratt@samsung.com changed reviewers:
+ siggi@chromium.org, sky@chromium.org

[g.mehndiratt, 2015-05-27 08:30:25]

Added Reviewers for remaining files review.

[Sigurður Ásgeirsson, 2015-05-27 14:11:39]

browser_watcher lgtm

[g.mehndiratt, 2015-05-28 05:07:14]

g.mehndiratt@samsung.com changed reviewers:
+ oshima@chromium.org

[g.mehndiratt, 2015-05-28 05:07:39]

Added another review for ash/test files review.

[oshima, 2015-05-28 06:19:19]

ash/ lgtm

[g.mehndiratt, 2015-05-29 06:23:03]

The CQ bit was checked by g.mehndiratt@samsung.com

[commit-bot: I haz the power, 2015-05-29 06:23:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1086363003/120001

[commit-bot: I haz the power, 2015-05-29 08:17:29]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-05-29 08:18:25]

Patchset 7 (id:??) landed as
https://crrev.com/fd19e23a8a1e0d390706ad53470afb6d77f88847
Cr-Commit-Position: refs/heads/master@{#331936}

[vabr (Chromium), 2015-05-29 09:47:25]

On 2015/05/29 08:18:25, commit-bot: I haz the power wrote:
> Patchset 7 (id:??) landed as
> https://crrev.com/fd19e23a8a1e0d390706ad53470afb6d77f88847
> Cr-Commit-Position: refs/heads/master@{#331936}

FYI: Currently suspecting this CL of breaking
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%2...
and
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....
Will probably speculatively revert soon.

Cheers,
today's sheriff

[vabr (Chromium), 2015-05-29 11:15:23]

On 2015/05/29 09:47:25, vabr (Chromium) wrote:
> On 2015/05/29 08:18:25, commit-bot: I haz the power wrote:
> > Patchset 7 (id:??) landed as
> > https://crrev.com/fd19e23a8a1e0d390706ad53470afb6d77f88847
> > Cr-Commit-Position: refs/heads/master@{#331936}
> 
> FYI: Currently suspecting this CL of breaking
>
https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%2...
> and
>
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....
> Will probably speculatively revert soon.
> 
> Cheers,
> today's sheriff

The next build is green, no reverting.