Change the frequency of update checks depending on the installed channel of chrome.

Change the frequency of update checks depending on the installed channel of chrome.

BUG=50278
TEST=none

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

[Evan Stade, 2010-09-21 00:01:21]

I just picked these frequencies arbitrarily, let me know what you think they
should be.

[Finnur, 2010-09-21 10:33:44]

Thanks for working on this. There is a bug, which I have assigned to you. It
lists the frequencies requested by Jeff. Make sure to update the CL description
to note the bug also.

http://codereview.chromium.org/3455009/diff/1/2
File chrome/browser/gtk/update_recommended_dialog.cc (right):

http://codereview.chromium.org/3455009/diff/1/2#newcode31
chrome/browser/gtk/update_recommended_dialog.cc:31: GTK_RESPONSE_REJECT,
I assume this reverses the order of the buttons? What is the convention here in
our GTK port - main action button to the right, cancel to the left?

http://codereview.chromium.org/3455009/diff/1/3
File chrome/browser/upgrade_detector.cc (right):

http://codereview.chromium.org/3455009/diff/1/3#newcode39
chrome/browser/upgrade_detector.cc:39: const std::string channel =
platform_util::GetVersionStringModifier();
10 bonus points for adding a function to GoogleUpdateSettings that returns an
enum with this information, instead of parsing the strings.

http://codereview.chromium.org/3455009/diff/1/3#newcode165
chrome/browser/upgrade_detector.cc:165: interval_ms *= 1000;  // Command line
value is in seconds.
Now that we have a function GetCheckForUpgradeEveryMs(), this code should
probably live there. Then you can check for the command line value first and (if
it exists) skip any calculations based on the string.

[Evan Stade, 2010-09-21 19:38:11]

http://codereview.chromium.org/3455009/diff/1/2
File chrome/browser/gtk/update_recommended_dialog.cc (right):

http://codereview.chromium.org/3455009/diff/1/2#newcode31
chrome/browser/gtk/update_recommended_dialog.cc:31: GTK_RESPONSE_REJECT,
On 2010/09/21 10:33:44, Finnur wrote:
> I assume this reverses the order of the buttons? What is the convention here
in
> our GTK port - main action button to the right, cancel to the left?

yes

http://codereview.chromium.org/3455009/diff/1/3
File chrome/browser/upgrade_detector.cc (right):

http://codereview.chromium.org/3455009/diff/1/3#newcode39
chrome/browser/upgrade_detector.cc:39: const std::string channel =
platform_util::GetVersionStringModifier();
On 2010/09/21 10:33:44, Finnur wrote:
> 10 bonus points for adding a function to GoogleUpdateSettings that returns an
> enum with this information, instead of parsing the strings.

I like this idea, but I think it would have to live in
common/version_info.{h,cc} because GoogleUpdateSettings is win-only. And either
way this would require moving a bunch of code from browser/platform_util, so it
seems better to save it for another cl.

http://codereview.chromium.org/3455009/diff/1/3#newcode165
chrome/browser/upgrade_detector.cc:165: interval_ms *= 1000;  // Command line
value is in seconds.
On 2010/09/21 10:33:44, Finnur wrote:
> Now that we have a function GetCheckForUpgradeEveryMs(), this code should
> probably live there. Then you can check for the command line value first and
(if
> it exists) skip any calculations based on the string.

Done.

[Finnur, 2010-09-21 21:16:07]

LGTM

On 2010/09/21 19:38:11, Evan Stade wrote:
> http://codereview.chromium.org/3455009/diff/1/2
> File chrome/browser/gtk/update_recommended_dialog.cc (right):
> 
> http://codereview.chromium.org/3455009/diff/1/2#newcode31
> chrome/browser/gtk/update_recommended_dialog.cc:31: GTK_RESPONSE_REJECT,
> On 2010/09/21 10:33:44, Finnur wrote:
> > I assume this reverses the order of the buttons? What is the convention here
> in
> > our GTK port - main action button to the right, cancel to the left?
> 
> yes
> 
> http://codereview.chromium.org/3455009/diff/1/3
> File chrome/browser/upgrade_detector.cc (right):
> 
> http://codereview.chromium.org/3455009/diff/1/3#newcode39
> chrome/browser/upgrade_detector.cc:39: const std::string channel =
> platform_util::GetVersionStringModifier();
> On 2010/09/21 10:33:44, Finnur wrote:
> > 10 bonus points for adding a function to GoogleUpdateSettings that returns
an
> > enum with this information, instead of parsing the strings.
> 
> I like this idea, but I think it would have to live in
> common/version_info.{h,cc} because GoogleUpdateSettings is win-only. And
either
> way this would require moving a bunch of code from browser/platform_util, so
it
> seems better to save it for another cl.
> 
> http://codereview.chromium.org/3455009/diff/1/3#newcode165
> chrome/browser/upgrade_detector.cc:165: interval_ms *= 1000;  // Command line
> value is in seconds.
> On 2010/09/21 10:33:44, Finnur wrote:
> > Now that we have a function GetCheckForUpgradeEveryMs(), this code should
> > probably live there. Then you can check for the command line value first and
> (if
> > it exists) skip any calculations based on the string.
> 
> Done.