Cause new_chrome.exe to always be written if Chrome Frame is in use, resulting in updates being d...

Cause new_chrome.exe to always be written if Chrome Frame is in use, resulting in updates being deferred.

BUG=88589
TEST=NONE
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=92706

[grt (UTC plus 2), 2011-07-14 14:25:29]

Awesome fix.  LGTM with some nits.

http://codereview.chromium.org/7353030/diff/9001/chrome/browser/first_run/upg...
File chrome/browser/first_run/upgrade_util_win.cc (right):

http://codereview.chromium.org/7353030/diff/9001/chrome/browser/first_run/upg...
chrome/browser/first_run/upgrade_util_win.cc:97: KEY_READ) == ERROR_SUCCESS) {
fix indentation, and use KEY_QUERY_VALUE rather than KEY_READ.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
File chrome/installer/util/installer_state.cc (right):

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:349: bool
InstallerState::is_chrome_frame_running() const {
move this trivial method into the header.  (the two methods above aren't since
they do DCHECKs.)

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:429: 
add "// static" comment

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:434: base::win::ScopedHandle
file_handle(CreateFile(file.value().c_str(),
this function could be reduced to:

return !base::win::ScopedHandle(CreateFile(...)).IsValid();

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:442: 
consider adding a brief comment here explaining that it's only necessary to
check for the "current" version and not versions between that and the newest
version.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
File chrome/installer/util/installer_state.h (right):

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.h:198: bool IsFileInUse(const FilePath&
file);
this can be static.

[robertshield, 2011-07-15 13:49:17]

Thanks PTAL!

http://codereview.chromium.org/7353030/diff/9001/chrome/browser/first_run/upg...
File chrome/browser/first_run/upgrade_util_win.cc (right):

http://codereview.chromium.org/7353030/diff/9001/chrome/browser/first_run/upg...
chrome/browser/first_run/upgrade_util_win.cc:97: KEY_READ) == ERROR_SUCCESS) {
On 2011/07/14 14:25:29, grt wrote:
> fix indentation, and use KEY_QUERY_VALUE rather than KEY_READ.

Done.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
File chrome/installer/util/installer_state.cc (right):

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:349: bool
InstallerState::is_chrome_frame_running() const {
On 2011/07/14 14:25:29, grt wrote:
> move this trivial method into the header.  (the two methods above aren't since
> they do DCHECKs.)

Done.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:429: 
On 2011/07/14 14:25:29, grt wrote:
> add "// static" comment

Done.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:434: base::win::ScopedHandle
file_handle(CreateFile(file.value().c_str(),
On 2011/07/14 14:25:29, grt wrote:
> this function could be reduced to:
> 
> return !base::win::ScopedHandle(CreateFile(...)).IsValid();

Done.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.cc:442: 
On 2011/07/14 14:25:29, grt wrote:
> consider adding a brief comment here explaining that it's only necessary to
> check for the "current" version and not versions between that and the newest
> version.

Done.

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
File chrome/installer/util/installer_state.h (right):

http://codereview.chromium.org/7353030/diff/9001/chrome/installer/util/instal...
chrome/installer/util/installer_state.h:198: bool IsFileInUse(const FilePath&
file);
On 2011/07/14 14:25:29, grt wrote:
> this can be static.

Done.

[grt (UTC plus 2), 2011-07-15 13:58:08]

Again greatness.

http://codereview.chromium.org/7353030/diff/15001/chrome/installer/util/insta...
File chrome/installer/util/installer_state_unittest.cc (right):

http://codereview.chromium.org/7353030/diff/15001/chrome/installer/util/insta...
chrome/installer/util/installer_state_unittest.cc:50: bool IsFileInUse(const
FilePath& file) {
nit: what do you think about making this static, too?

[robertshield, 2011-07-15 17:19:12]

Thanks!

http://codereview.chromium.org/7353030/diff/15001/chrome/installer/util/insta...
File chrome/installer/util/installer_state_unittest.cc (right):

http://codereview.chromium.org/7353030/diff/15001/chrome/installer/util/insta...
chrome/installer/util/installer_state_unittest.cc:50: bool IsFileInUse(const
FilePath& file) {
On 2011/07/15 13:58:09, grt wrote:
> nit: what do you think about making this static, too?

Done.

[grt (UTC plus 2), 2011-07-15 17:22:25]

LGTM++