Print too-long command line

Print too-long command line

When RegisterApplicationRestart fails with E_INVALIDARG it is supposed
to indicate that the command line is too long. This error was hit on
a recent test run and it was impossible to do further diagnosis because
the command line is not printed, even though it is printed for other
failure codes. This change prints the command line in the E_INVALIDARG
case.

The failure only happened once and was probably a Windows glitch since
all evidence is that the command line was short enough. But, still.

This change also removes some checks for win::VERSION_VISTA since Chrome
requires Windows 7 or higher, and avoids some redundant calls to
GetCommandLineString.

Review-Url: https://codereview.chromium.org/2756603002
Cr-Commit-Position: refs/heads/master@{#457297}
Committed: https://chromium.googlesource.com/chromium/src/+/fe785a58e31217e1ef0e1c8946a4e853829371f8

[brucedawson, 2017-03-15 22:22:55]

The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-15 22:23:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brucedawson, 2017-03-15 22:24:04]

Description was changed from

==========
Print too-long command line

When RegisterApplicationRestart fails with E_INVALIDARG it is supposed
to indicate that the command line is too long. This error was hit on
a recent test run and it was impossible to do further diagnosis because
the command line is not printed, even though it is printed for other
failure codes. This change prints the command line in the E_INVALIDARG
case.

The failure only happened once and was probably a Windows glitch since
all evidence is that the command line was short enough. But, still.

This change also removes some checks for win::VERSION_VISTA since Chrome
requires Windows 7 or higher, and avoids some redundant calls to
GetCommandLineString.
==========

to

==========
Print too-long command line

When RegisterApplicationRestart fails with E_INVALIDARG it is supposed
to indicate that the command line is too long. This error was hit on
a recent test run and it was impossible to do further diagnosis because
the command line is not printed, even though it is printed for other
failure codes. This change prints the command line in the E_INVALIDARG
case.

The failure only happened once and was probably a Windows glitch since
all evidence is that the command line was short enough. But, still.

This change also removes some checks for win::VERSION_VISTA since Chrome
requires Windows 7 or higher, and avoids some redundant calls to
GetCommandLineString.
==========

[brucedawson, 2017-03-15 22:24:05]

brucedawson@chromium.org changed reviewers:
+ thestig@chromium.org

[commit-bot: I haz the power, 2017-03-15 23:38:50]

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

[commit-bot: I haz the power, 2017-03-15 23:38:51]

Dry run: This issue passed the CQ dry run.

[brucedawson, 2017-03-15 23:41:43]

Minor tweak to print better diagnostics on a failure we saw once - PTAL

[Lei Zhang, 2017-03-15 23:59:29]

lgtm

https://codereview.chromium.org/2756603002/diff/1/chrome/browser/chrome_brows...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/2756603002/diff/1/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main.cc:203: #include "base/win/windows_version.h"
No longer needed?

[brucedawson, 2017-03-16 00:08:25]

I was briefly tempted to expand the change to make the #include in the other .cc
file unnecessary but I resisted. Another day.

I updated the CL to remove the #include that you pointed out.

https://codereview.chromium.org/2756603002/diff/1/chrome/browser/chrome_brows...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/2756603002/diff/1/chrome/browser/chrome_brows...
chrome/browser/chrome_browser_main.cc:203: #include "base/win/windows_version.h"
On 2017/03/15 23:59:29, Lei Zhang (super slow) wrote:
> No longer needed?

Sharp eyes. You are correct.

[brucedawson, 2017-03-16 00:08:35]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2017-03-16 00:08:35]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2756603002/#ps20001
(title: "Remove now unneeded include")

[commit-bot: I haz the power, 2017-03-16 00:09:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-16 01:18:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489622915108900,
"parent_rev": "38424d53080c397f8cc13c6581fe937070922ac0", "commit_rev":
"fe785a58e31217e1ef0e1c8946a4e853829371f8"}

[commit-bot: I haz the power, 2017-03-16 01:19:27]

Description was changed from

==========
Print too-long command line

When RegisterApplicationRestart fails with E_INVALIDARG it is supposed
to indicate that the command line is too long. This error was hit on
a recent test run and it was impossible to do further diagnosis because
the command line is not printed, even though it is printed for other
failure codes. This change prints the command line in the E_INVALIDARG
case.

The failure only happened once and was probably a Windows glitch since
all evidence is that the command line was short enough. But, still.

This change also removes some checks for win::VERSION_VISTA since Chrome
requires Windows 7 or higher, and avoids some redundant calls to
GetCommandLineString.
==========

to

==========
Print too-long command line

When RegisterApplicationRestart fails with E_INVALIDARG it is supposed
to indicate that the command line is too long. This error was hit on
a recent test run and it was impossible to do further diagnosis because
the command line is not printed, even though it is printed for other
failure codes. This change prints the command line in the E_INVALIDARG
case.

The failure only happened once and was probably a Windows glitch since
all evidence is that the command line was short enough. But, still.

This change also removes some checks for win::VERSION_VISTA since Chrome
requires Windows 7 or higher, and avoids some redundant calls to
GetCommandLineString.

Review-Url: https://codereview.chromium.org/2756603002
Cr-Commit-Position: refs/heads/master@{#457297}
Committed:
https://chromium.googlesource.com/chromium/src/+/fe785a58e31217e1ef0e1c8946a4...
==========

[commit-bot: I haz the power, 2017-03-16 01:19:29]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/fe785a58e31217e1ef0e1c8946a4...