Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1092)

Issue 3817001: CommandLine: remove wstring-based program() accessor (Closed)

Created:
10 years, 2 months ago by Evan Martin
Modified:
9 years, 7 months ago
Reviewers:
viettrungluu
CC:
chromium-reviews, ben+cc_chromium.org, Paweł Hajdan Jr., brettw-cc_chromium.org
Base URL:
http://src.chromium.org/git/chromium.git
Visibility:
Public.

Description

CommandLine: remove wstring-based program() accessor This was already removed on non-Windows, so this change modifies the remaining Windows-specific usage. In a few places I converted use of wstring paths into FilePath, but in general for Windows-specific code I don't think it's too important to use FilePath everywhere, because it is equivalent on Windows and the current code already works. BUG=23581 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62637

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -62 lines) Patch
M base/command_line.h View 1 chunk +0 lines, -1 line 0 comments Download
M base/command_line.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M base/command_line_unittest.cc View 1 chunk +1 line, -1 line 1 comment Download
M chrome/browser/shell_integration_win.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/sandbox_policy.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/setup/setup_main.cc View 8 chunks +20 lines, -17 lines 1 comment Download
M chrome/installer/setup/setup_util.h View 1 chunk +6 lines, -4 lines 0 comments Download
M chrome/installer/setup/setup_util.cc View 1 chunk +18 lines, -14 lines 0 comments Download
M chrome/installer/setup/setup_util_unittest.cc View 2 chunks +6 lines, -7 lines 0 comments Download
M chrome/installer/setup/uninstall.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/util/shell_util.cc View 2 chunks +10 lines, -8 lines 0 comments Download
M chrome/test/mini_installer_test/run_all_unittests.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Evan Martin
As requested. Please review my logic carefully! Trybot run: http://build.chromium.org/buildbot/try-server/builders/win/builds/54030
10 years, 2 months ago (2010-10-13 21:36:14 UTC) #1
Evan Martin
ping.
10 years, 2 months ago (2010-10-14 18:22:34 UTC) #2
viettrungluu
10 years, 2 months ago (2010-10-14 19:51:27 UTC) #3
LGTM. (I hope I didn't miss anything....)

http://codereview.chromium.org/3817001/diff/1/4
File base/command_line_unittest.cc (right):

http://codereview.chromium.org/3817001/diff/1/4#newcode109
base/command_line_unittest.cc:109: EXPECT_TRUE(cl.GetProgram().value().empty());
Or maybe even "GetProgram().empty()"?

http://codereview.chromium.org/3817001/diff/1/7
File chrome/installer/setup/setup_main.cc (right):

http://codereview.chromium.org/3817001/diff/1/7#newcode231
chrome/installer/setup/setup_main.cc:231: FilePath archive =
Probably the way to break this line (to be consistent with other line breaks in
this file) is:

FilePath archive = cmd_line.GetProgram().DirName().Append(
    installer::kChromeCompressedArchive);

Powered by Google App Engine
This is Rietveld 408576698