Fixes a bug in the criteria checker that caused the 6-month flag to not be de...

Fixes a bug in the criteria checker that caused the 6-month flag to not be deleted.

BUG=8217
R=kuchhal


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=11038

[Glenn Wilson, 2009-03-03 18:23:22]

Here's a revised version of the criteria checker fix, which cleans up the
original version of this method.

[kuchhal, 2009-03-03 18:33:18]

I think the new for loop can get stuck in infinite loop since we are only
checking ERROR_NO_MORE_ITEMS return as exit condition. If there is any other
error the loop will keep getting the same error again and again.

We probably do need ERROR_SUCCESS check to make sure registry functions are
working as expected.

http://codereview.chromium.org/27307/diff/2002/2003
File chrome/installer/gcapi/gcapi.cc (right):

http://codereview.chromium.org/27307/diff/2002/2003#newcode34
Line 34: wchar_t value_name[260];
Move this also inside for loop (line 41)?

http://codereview.chromium.org/27307/diff/2002/2003#newcode47
Line 47: &value_len) == ERROR_NO_MORE_ITEMS)
reduce indentation of lines 46 and 47 by one.

[Glenn Wilson, 2009-03-03 22:22:55]

On 2009/03/03 18:33:18, kuchhal wrote:
> I think the new for loop can get stuck in infinite loop since we are only
> checking ERROR_NO_MORE_ITEMS return as exit condition. If there is any other
> error the loop will keep getting the same error again and again.
> 
> We probably do need ERROR_SUCCESS check to make sure registry functions are
> working as expected.
> 

I changed this to be != ERROR_SUCCESS instead of == ERROR_NO_MORE_ITEMS.  This
means that this loop exits on any type of failure at all now.

> http://codereview.chromium.org/27307/diff/2002/2003
> File chrome/installer/gcapi/gcapi.cc (right):
> 
> http://codereview.chromium.org/27307/diff/2002/2003#newcode34
> Line 34: wchar_t value_name[260];
> Move this also inside for loop (line 41)?
> 

Done.

> http://codereview.chromium.org/27307/diff/2002/2003#newcode47
> Line 47: &value_len) == ERROR_NO_MORE_ITEMS)
> reduce indentation of lines 46 and 47 by one.

Done.

[kuchhal, 2009-03-03 22:25:41]

lgtm

[Glenn Wilson, 2009-03-05 17:59:28]

Sorry for the second set of changes on this, but we have an improved patch for
this issue.

Jean-Luc and I decided to cut the CleanUp..() method out entirely, since it
seemed unnecessary, and added its behavior into the CanReOffer...() method.

Also, I cleaned up some of the indentation in the main
GoogleChromeCompatibilityCheck method.

[kuchhal, 2009-03-05 18:20:43]

lgtm. with one nit below.

http://codereview.chromium.org/27307/diff/2009/2010
File chrome/installer/gcapi/gcapi.cc (right):

http://codereview.chromium.org/27307/diff/2009/2010#newcode123
Line 123: ::RegSetValueEx(key, company, 0, REG_DWORD, (LPBYTE)&value,
sizeof(DWORD));
More than 80 chars