Skip to content

Commit

Permalink
Print too-long command line
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
randomascii authored and Commit bot committed Mar 16, 2017
1 parent 38424d5 commit fe785a5
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 10 deletions.
6 changes: 1 addition & 5 deletions chrome/browser/chrome_browser_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@
#include "base/trace_event/trace_event_etw_export_win.h"
#include "base/win/registry.h"
#include "base/win/win_util.h"
#include "base/win/windows_version.h"
#include "chrome/browser/chrome_browser_main_win.h"
#include "chrome/browser/component_updater/sw_reporter_installer_win.h"
#include "chrome/browser/downgrade/user_data_downgrade.h"
Expand Down Expand Up @@ -1706,10 +1705,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
// This could be run as late as WM_QUERYENDSESSION for system update reboots,
// but should run on startup if extended to handle crashes/hangs/patches.
// Also, better to run once here than once for each HWND's WM_QUERYENDSESSION.
if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
ChromeBrowserMainPartsWin::RegisterApplicationRestart(
parsed_command_line());
}
ChromeBrowserMainPartsWin::RegisterApplicationRestart(parsed_command_line());

// Verify that the profile is not on a network share and if so prepare to show
// notification to the user.
Expand Down
11 changes: 6 additions & 5 deletions chrome/browser/chrome_browser_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,6 @@ void ChromeBrowserMainPartsWin::PrepareRestartOnCrashEnviroment(
// static
void ChromeBrowserMainPartsWin::RegisterApplicationRestart(
const base::CommandLine& parsed_command_line) {
DCHECK(base::win::GetVersion() >= base::win::VERSION_VISTA);
base::ScopedNativeLibrary library(base::FilePath(L"kernel32.dll"));
// Get the function pointer for RegisterApplicationRestart.
RegisterApplicationRestartProc register_application_restart =
Expand All @@ -444,15 +443,17 @@ void ChromeBrowserMainPartsWin::RegisterApplicationRestart(

// Restart Chrome if the computer is restarted as the result of an update.
// This could be extended to handle crashes, hangs, and patches.
const auto& command_line_string = command_line.GetCommandLineString();
HRESULT hr = register_application_restart(
command_line.GetCommandLineString().c_str(),
command_line_string.c_str(),
RESTART_NO_CRASH | RESTART_NO_HANG | RESTART_NO_PATCH);
if (FAILED(hr)) {
if (hr == E_INVALIDARG) {
LOG(WARNING) << "Command line too long for RegisterApplicationRestart";
LOG(WARNING) << "Command line too long for RegisterApplicationRestart: "
<< command_line_string;
} else {
NOTREACHED() << "RegisterApplicationRestart failed. hr: " << hr <<
", command_line: " << command_line.GetCommandLineString();
NOTREACHED() << "RegisterApplicationRestart failed. hr: " << hr
<< ", command_line: " << command_line_string;
}
}
}
Expand Down

0 comments on commit fe785a5

Please sign in to comment.