Show a modified outdated upgrade Bubble for non-enterprise users without auto-update

Show a modified outdated upgrade Bubble for non-enterprise users without auto-update

BUG=164681, 171271
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260289

[MAD, 2014-03-26 02:52:15]

Can you take a first quick look at this?

I was hesitating with something, now we only have two states, but once we start
support the enterprise case, we'll start using an enum instead, and possibly a
single notification with the enum value in the details... Do you think we might
as well do this now, or wait for when we'll add support for enterprise?

Since we're in a hurry for the branch, I went for the simpler solution...
Agreed?

Thanks!

BYE
MAD

[robertshield, 2014-03-26 03:20:11]

Awesome, notes and questions below:

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.cc (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:60:
GetModuleFileName(NULL, exe_path, arraysize(exe_path));
PathService::Get(base::FILE_EXE, &exe_path) is perhaps more conventional

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:258: // auto-update.
please add "and record enable_au bit to local state." to the TODO

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.h (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.h:71: // Button that let
the user accept the proposal, which is to navigate to a
*lets

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.h:72: // Chrome download
page when |auto_update_on_| is true, or turn on auto-update
or attempt to re-enable auto-update otherwise.

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/tool...
File chrome/browser/ui/views/toolbar/toolbar_view.h (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/tool...
chrome/browser/ui/views/toolbar/toolbar_view.h:217: void
ShowOutdatedInstallNotification(bool auto_update_on);
suggest: s/auto_update_on/auto_update_enabled/

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
chrome/browser/upgrade_detector_impl.cc:134: prefs::kFailedTurningOnAutoupdate))
{
nit: indent. 

Moreover, I think we should call this "kAttemptedToEnableAutoupdate" and set it
right before the user tries to re-enable updates rather than have a beacon that
is set on failure (since we may e.g. crash and never set the failure beacon).

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
chrome/browser/upgrade_detector_impl.cc:347: // Don't show the bubble if we have
a brand code that is NOT organic, unless
why is this limited to only organic brand codes?

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
File chrome/browser/upgrade_detector_impl.h (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
chrome/browser/upgrade_detector_impl.h:70: bool is_autoupdate_on_;
suggest on -> enabled

https://codereview.chromium.org/211543008/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/211543008/diff/1/chrome/common/pref_names.cc#...
chrome/common/pref_names.cc:2380: const char kFailedTurningOnAutoupdate[] =
see other comment, suggest calling kAttemptedToReenableAutoupdate and changing
the comment to:

// A boolean where true means that the browser has previously attempted to
enable autoupdate and
// failed, so the next out-of-date browser start should not prompt the user to
enable autoupdate,
// it should offer to reinstall Chrome instead.

[robertshield, 2014-03-26 03:24:44]

Also, any chance you can fix up the other strings I messed up in
https://codereview.chromium.org/209433008/ ?

Here's one, I think it might be the only one left:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

[robertshield, 2014-03-26 03:51:38]

On 2014/03/26 03:24:44, robertshield wrote:
> Also, any chance you can fix up the other strings I messed up in
> https://codereview.chromium.org/209433008/ ?
> 
> Here's one, I think it might be the only one left:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

Also, the bug number on this CL is wrong, it's pointing at the privacy review. I
think it should be 164681 (maybe it could include the privacy review one too).

[MAD, 2014-03-26 19:30:16]

Thanks all addressed/answered...

Adding Alexei for tools\metrics OWNERS review...

Thanks!

BYE
MAD

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.cc (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:60:
GetModuleFileName(NULL, exe_path, arraysize(exe_path));
On 2014/03/26 03:20:11, robertshield wrote:
> PathService::Get(base::FILE_EXE, &exe_path) is perhaps more conventional

Done.

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:258: // auto-update.
On 2014/03/26 03:20:11, robertshield wrote:
> please add "and record enable_au bit to local state." to the TODO

Done.

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.h (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.h:71: // Button that let
the user accept the proposal, which is to navigate to a
On 2014/03/26 03:20:11, robertshield wrote:
> *lets

Done.

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/outd...
chrome/browser/ui/views/outdated_upgrade_bubble_view.h:72: // Chrome download
page when |auto_update_on_| is true, or turn on auto-update
On 2014/03/26 03:20:11, robertshield wrote:
> or attempt to re-enable auto-update otherwise.

Done.

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/tool...
File chrome/browser/ui/views/toolbar/toolbar_view.h (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/ui/views/tool...
chrome/browser/ui/views/toolbar/toolbar_view.h:217: void
ShowOutdatedInstallNotification(bool auto_update_on);
On 2014/03/26 03:20:11, robertshield wrote:
> suggest: s/auto_update_on/auto_update_enabled/

Done.

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
chrome/browser/upgrade_detector_impl.cc:134: prefs::kFailedTurningOnAutoupdate))
{
On 2014/03/26 03:20:11, robertshield wrote:
> nit: indent. 
> 
> Moreover, I think we should call this "kAttemptedToEnableAutoupdate" and set
it
> right before the user tries to re-enable updates rather than have a beacon
that
> is set on failure (since we may e.g. crash and never set the failure beacon). 

Done.
Interestingly, the initial name I had was tried instead of failed... :-)

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
chrome/browser/upgrade_detector_impl.cc:347: // Don't show the bubble if we have
a brand code that is NOT organic, unless
On 2014/03/26 03:20:11, robertshield wrote:
> why is this limited to only organic brand codes?

It was discussed around here:
https://code.google.com/p/chromium/issues/detail?id=151996#c65

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
File chrome/browser/upgrade_detector_impl.h (right):

https://codereview.chromium.org/211543008/diff/1/chrome/browser/upgrade_detec...
chrome/browser/upgrade_detector_impl.h:70: bool is_autoupdate_on_;
On 2014/03/26 03:20:11, robertshield wrote:
> suggest on -> enabled

Done.

https://codereview.chromium.org/211543008/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/211543008/diff/1/chrome/common/pref_names.cc#...
chrome/common/pref_names.cc:2380: const char kFailedTurningOnAutoupdate[] =
On 2014/03/26 03:20:11, robertshield wrote:
> see other comment, suggest calling kAttemptedToReenableAutoupdate and changing
> the comment to:
> 
> // A boolean where true means that the browser has previously attempted to
> enable autoupdate and
> // failed, so the next out-of-date browser start should not prompt the user to
> enable autoupdate,
> // it should offer to reinstall Chrome instead.

Done.

[robertshield, 2014-03-26 20:37:18]

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/ui/views...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/ui/views...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:259: // auto-update, and
record enable_au bit to local state.
Actually since you've added the pref, you could set it here if you like :-)

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/upgrade_...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:163: // - kSimulateOutdated[NoAu] can
work on its own, or with a specified date.
drop the []

https://codereview.chromium.org/211543008/diff/140001/chrome/common/pref_name...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/common/pref_name...
chrome/common/pref_names.cc:2369: "browser.failed_turning_on_autoupdate";
s/failed_turning_on_autoupdate/attempted_to_enable_autoupdate/

[MAD, 2014-03-26 21:21:10]

Answered 2, addressed one... Did I convinced you enough with my answers? :-)

Adding sky for UI OWNER review now to get it primed...

Thanks!

BYE
MAD

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/ui/views...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/ui/views...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:259: // auto-update, and
record enable_au bit to local state.
On 2014/03/26 20:37:18, robertshield wrote:
> Actually since you've added the pref, you could set it here if you like :-)

Actually, I thought it would be better if we added within the function... I'm a
bit ambivalent here... And shouldn't we only set it when the API returned true?
Then I would prefer we wait for the API to be called here...

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/upgrade_...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:163: // - kSimulateOutdated[NoAu] can
work on its own, or with a specified date.
On 2014/03/26 20:37:18, robertshield wrote:
> drop the []

Why? This is intended to say kSimulateOutdated and kSimulateOutdatedNoAu, do you
prefer kSimulateOutdated/NoAu instead?

https://codereview.chromium.org/211543008/diff/140001/chrome/common/pref_name...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/common/pref_name...
chrome/common/pref_names.cc:2369: "browser.failed_turning_on_autoupdate";
On 2014/03/26 20:37:18, robertshield wrote:
> s/failed_turning_on_autoupdate/attempted_to_enable_autoupdate/

Done.

[Alexei Svitkine (slow), 2014-03-26 21:24:41]

(Hmm, somehow I reviewed a different CL than I thought you asked me to review...
Will review the other one in a bit.)

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:66: bool SimulatingOutdated() {
Nit: Add a 1-line comment to provide more clarity to the reader what this is
for.

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:123: // channel.
Nit: Update comment.

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:133:
!g_browser_process->local_state()->GetBoolean(
This function is running on the FILE thread, which I don't think is allowed to
access the pref store (only UI thread is). I think you need to do this check
before you post task and pass the result as a parameter.

[Alexei Svitkine (slow), 2014-03-26 21:26:18]

Nevermind, you did ask me to review this one (I got confused since I wasn't in
the reviewers list when I was posting the above reply).

[robertshield, 2014-03-26 21:45:09]

LGTM, feel free to add the pref if you like or disagree with me otherwise :-)

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/ui/views...
File chrome/browser/ui/views/outdated_upgrade_bubble_view.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/ui/views...
chrome/browser/ui/views/outdated_upgrade_bubble_view.cc:259: // auto-update, and
record enable_au bit to local state.
On 2014/03/26 21:21:10, MAD wrote:
> On 2014/03/26 20:37:18, robertshield wrote:
> > Actually since you've added the pref, you could set it here if you like :-)
> 
> Actually, I thought it would be better if we added within the function... I'm
a
> bit ambivalent here... And shouldn't we only set it when the API returned
true?
> Then I would prefer we wait for the API to be called here...

I think we should set it regardless of whether the API succeeds or not. It does
no harm if the API succeeds and this way we don't have to worry about it not
being set for reasons in cases where the API might fail (picture e.g. a call out
to setup.exe and setup.exe hangs forever).

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/upgrade_...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/140001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:163: // - kSimulateOutdated[NoAu] can
work on its own, or with a specified date.
On 2014/03/26 21:21:10, MAD wrote:
> On 2014/03/26 20:37:18, robertshield wrote:
> > drop the []
> 
> Why? This is intended to say kSimulateOutdated and kSimulateOutdatedNoAu, do
you
> prefer kSimulateOutdated/NoAu instead?

Oh, sorry, for some reason I read it entirely as a typo. No, what you have there
is fine, if my brain had been working I would have understood.

[MAD, 2014-03-26 23:47:35]

Addressed all comments from both Robert and Alexei...

Now just needs OWNER approvals...

Thanks!

BYE
MAD

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
File chrome/browser/upgrade_detector_impl.cc (right):

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:66: bool SimulatingOutdated() {
On 2014/03/26 21:24:42, Alexei Svitkine wrote:
> Nit: Add a 1-line comment to provide more clarity to the reader what this is
> for.

Done... And then some... :-)

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:123: // channel.
On 2014/03/26 21:24:42, Alexei Svitkine wrote:
> Nit: Update comment.

Done.

https://codereview.chromium.org/211543008/diff/160001/chrome/browser/upgrade_...
chrome/browser/upgrade_detector_impl.cc:133:
!g_browser_process->local_state()->GetBoolean(
On 2014/03/26 21:24:42, Alexei Svitkine wrote:
> This function is running on the FILE thread, which I don't think is allowed to
> access the pref store (only UI thread is). I think you need to do this check
> before you post task and pass the result as a parameter.

Done. Good catch... Thanks!

[sky, 2014-03-27 15:38:29]

LGTM

[Alexei Svitkine (slow), 2014-03-27 15:47:18]

LGTM

[MAD, 2014-03-27 15:48:05]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-27 15:48:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/240001

[MAD, 2014-03-27 15:48:21]

Thanks OWNERS...

[commit-bot: I haz the power, 2014-03-27 15:55:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 15:55:30]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[MAD, 2014-03-27 16:55:23]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-27 16:55:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/240001

[commit-bot: I haz the power, 2014-03-27 17:18:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 17:18:35]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg

[MAD, 2014-03-27 17:58:37]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-27 17:59:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/240001

[commit-bot: I haz the power, 2014-03-27 18:04:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 18:04:07]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[MAD, 2014-03-27 18:06:12]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-27 18:08:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/240001

[commit-bot: I haz the power, 2014-03-27 19:45:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 19:45:29]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[MAD, 2014-03-27 19:56:38]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-27 20:00:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/240001

[commit-bot: I haz the power, 2014-03-28 02:29:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:29:41]

Retried try job too often on ios_dbg_simulator for step(s) components_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[MAD, 2014-03-28 02:46:13]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:47:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/240001

[commit-bot: I haz the power, 2014-03-28 02:47:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:47:47]

Failed to apply patch for chrome/browser/upgrade_detector_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/upgrade_detector_impl.cc
  Hunk #3 succeeded at 56 (offset -2 lines).
  Hunk #4 succeeded at 91 (offset -2 lines).
  Hunk #5 succeeded at 109 (offset -2 lines).
  Hunk #6 FAILED at 125.
  Hunk #7 succeeded at 142 (offset -4 lines).
  Hunk #8 succeeded at 155 (offset -4 lines).
  Hunk #9 succeeded at 167 (offset -4 lines).
  Hunk #10 succeeded at 175 (offset -4 lines).
  Hunk #11 succeeded at 191 (offset -4 lines).
  Hunk #12 succeeded at 206 (offset -4 lines).
  Hunk #13 succeeded at 237 (offset -4 lines).
  Hunk #14 succeeded at 346 (offset -4 lines).
  Hunk #15 succeeded at 360 (offset -4 lines).
  Hunk #16 succeeded at 373 (offset -4 lines).
  1 out of 16 hunks FAILED -- saving rejects to file
chrome/browser/upgrade_detector_impl.cc.rej

Patch:       chrome/browser/upgrade_detector_impl.cc
Index: chrome/browser/upgrade_detector_impl.cc
diff --git a/chrome/browser/upgrade_detector_impl.cc
b/chrome/browser/upgrade_detector_impl.cc
index
f2934538ca6ecf1fe10ec032d6c30ab2a444841b..1164e1f43875055de7f74b5d94826880a1768407
100644
--- a/chrome/browser/upgrade_detector_impl.cc
+++ b/chrome/browser/upgrade_detector_impl.cc
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/path_service.h"
+#include "base/prefs/pref_service.h"
 #include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -23,10 +24,12 @@
 #include "chrome/browser/google/google_util.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
+#include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/base/resource/resource_bundle.h"
 
 #if defined(OS_WIN)
+#include "base/win/win_util.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/helper.h"
@@ -55,17 +58,28 @@ const int kNotifyCycleTimeForTestingMs = 500;  // Half a
second.
 // The number of days after which we identify a build/install as outdated.
 const uint64 kOutdatedBuildAgeInDays = 12 * 7;
 
+// Return the string that was passed as a value for the
+// kCheckForUpdateIntervalSec switch.
 std::string CmdLineInterval() {
   const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
   return cmd_line.GetSwitchValueASCII(switches::kCheckForUpdateIntervalSec);
 }
 
+// Check if one of the outdated simulation switches was present on the command
+// line.
+bool SimulatingOutdated() {
+  const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
+  return cmd_line.HasSwitch(switches::kSimulateOutdated) ||
+      cmd_line.HasSwitch(switches::kSimulateOutdatedNoAU);
+}
+
+// Check if any of the testing switches was present on the command line.
 bool IsTesting() {
   const CommandLine& cmd_line = *CommandLine::ForCurrentProcess();
   return cmd_line.HasSwitch(switches::kSimulateUpgrade) ||
       cmd_line.HasSwitch(switches::kCheckForUpdateIntervalSec) ||
       cmd_line.HasSwitch(switches::kSimulateCriticalUpdate) ||
-      cmd_line.HasSwitch(switches::kSimulateOutdated);
+      SimulatingOutdated();
 }
 
 // How often to check for an upgrade.
@@ -79,6 +93,7 @@ int GetCheckForUpgradeEveryMs() {
   return kCheckForUpgradeMs;
 }
 
+// Return true if the current build is one of the unstable channels.
 bool IsUnstableChannel() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
@@ -96,6 +111,7 @@ void CheckForUnstableChannel(const base::Closure&
callback_task,
 }
 
 #if defined(OS_WIN)
+// Return true if the currently running Chrome is a system install.
 bool IsSystemInstall() {
   // Get the version of the currently *installed* instance of Chrome,
   // which might be newer than the *running* instance if we have been
@@ -109,19 +125,25 @@ bool IsSystemInstall() {
   return !InstallUtil::IsPerUserInstall(exe_path.value().c_str());
 }
 
-// This task checks the update policy and calls back the task only if automatic
-// updates are allowed. It also identifies whether we are running an unstable
-// channel.
+// This task checks the update policy and calls back the task only if the
+// system is not enrolled in a domain (i.e., not in an enterprise environment).
+// It also identifies if autoupdate is enabled and whether we are running an
+// unstable channel. |is_auto_update_enabled| can be NULL.
 void DetectUpdatability(const base::Closure& callback_task,
-                        bool* is_unstable_channel) {
+                        bool* is_unstable_channel,
+                        bool* is_auto_update_enabled) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   base::string16 app_guid = installer::GetAppGuidForUpdates(IsSystemInstall());
   DCHECK(!app_guid.empty());
-  if (GoogleUpdateSettings::AUTOMATIC_UPDATES ==
-      GoogleUpdateSettings::GetAppUpdatePolicy(app_guid, NULL)) {
-    CheckForUnstableChannel(callback_task, is_unstable_channel);
+  // Don't try to turn on autoupdate when we failed previously.
+  if (is_auto_update_enabled) {
+    *is_auto_update_enabled = GoogleUpdateSettings::AUTOMATIC_UPDATES ==
+        GoogleUpdateSettings::GetAppUpdatePolicy(app_guid, NULL);
   }
+  // Don't show the update bubbles to entreprise users (i.e., on a domain).
+  if (!base::win::IsEnrolledToDomain())
+    CheckForUnstableChannel(callback_task, is_unstable_channel);
 }
 #endif  // defined(OS_WIN)
 
@@ -130,7 +152,11 @@ void DetectUpdatability(const base::Closure& callback_task,
 UpgradeDetectorImpl::UpgradeDetectorImpl()
     : weak_factory_(this),
       is_unstable_channel_(false),
+      is_auto_update_enabled_(true),
       build_date_(base::GetBuildTime()) {
+  // Start tracking network time updates.
+  network_time_tracker_.Start();
+
   CommandLine command_line(*CommandLine::ForCurrentProcess());
   // The different command line switches that affect testing can't be used
   // simultaneously, if they do, here's the precedence order, based on the
order
@@ -139,7 +165,8 @@ UpgradeDetectorImpl::UpgradeDetectorImpl()
   //   switch from being taken into account.
   // - kSimulateUpgrade supersedes critical or outdated upgrade switches.
   // - kSimulateCriticalUpdate has precedence over kSimulateOutdated.
-  // - kSimulateOutdated can work on its own, or with a specified date.
+  // - kSimulateOutdatedNoAU has precedence over kSimulateOutdated.
+  // - kSimulateOutdated[NoAu] can work on its own, or with a specified date.
   if (command_line.HasSwitch(switches::kDisableBackgroundNetworking))
     return;
   if (command_line.HasSwitch(switches::kSimulateUpgrade)) {
@@ -150,7 +177,7 @@ UpgradeDetectorImpl::UpgradeDetectorImpl()
     UpgradeDetected(UPGRADE_AVAILABLE_CRITICAL);
     return;
   }
-  if (command_line.HasSwitch(switches::kSimulateOutdated)) {
+  if (SimulatingOutdated()) {
     // The outdated simulation can work without a value, which means outdated
     // now, or with a value that must be a well formed date/time string that
     // overrides the build date.
@@ -158,8 +185,14 @@ UpgradeDetectorImpl::UpgradeDetectorImpl()
     // tracking moves off of the VariationsService, the "variations-server-url"
     // command line switch must also be specified for the service to be
     // available on non GOOGLE_CHROME_BUILD.
-    std::string build_date = command_line.GetSwitchValueASCII(
-        switches::kSimulateOutdated);
+    std::string switch_name;
+    if (command_line.HasSwitch(switches::kSimulateOutdatedNoAU)) {
+      is_auto_update_enabled_ = false;
+      switch_name = switches::kSimulateOutdatedNoAU;
+    } else {
+      switch_name = switches::kSimulateOutdated;
+    }
+    std::string build_date = command_line.GetSwitchValueASCII(switch_name);
     base::Time maybe_build_time;
     bool result = base::Time::FromString(build_date.c_str(),
&maybe_build_time);
     if (result && !maybe_build_time.is_null()) {
@@ -168,7 +201,9 @@ UpgradeDetectorImpl::UpgradeDetectorImpl()
       StartTimerForUpgradeCheck();
     } else {
       // Without a valid date, we simulate that we are already outdated...
-      UpgradeDetected(UPGRADE_NEEDED_OUTDATED_INSTALL);
+      UpgradeDetected(
+          is_auto_update_enabled_ ? UPGRADE_NEEDED_OUTDATED_INSTALL
+                                  : UPGRADE_NEEDED_OUTDATED_INSTALL_NO_AU);
     }
     return;
   }
@@ -181,18 +216,27 @@ UpgradeDetectorImpl::UpgradeDetectorImpl()
   // Only enable upgrade notifications for official builds.  Chromium has no
   // upgrade channel.
 #if defined(GOOGLE_CHROME_BUILD)
-  // On Windows, there might be a policy preventing updates, so validate
-  // updatability, and then call StartTimerForUpgradeCheck appropriately.
+  // On Windows, there might be a policy/enterprise environment preventing
+  // updates, so validate updatability, and then call StartTimerForUpgradeCheck
+  // appropriately. And don't check for autoupdate if we already attempted to
+  // enable it in the past.
+  bool attempted_enabling_autoupdate = g_browser_process->local_state() &&
+      g_browser_process->local_state()->GetBoolean(
+          prefs::kAttemptedToEnableAutoupdate);
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                           base::Bind(&DetectUpdatability,
                                      start_upgrade_check_timer_task,
-                                     &is_unstable_channel_));
+                                     &is_unstable_channel_,
+                                     attempted_enabling_autoupdate ?
+                                         NULL : &is_auto_update_enabled_));
 #endif
 #else
 #if defined(OS_MACOSX)
   // Only enable upgrade notifications if the updater (Keystone) is present.
-  if (…
(message too large)

[MAD, 2014-03-28 02:56:45]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-28 02:57:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/260001

[commit-bot: I haz the power, 2014-03-28 03:33:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 03:33:18]

Retried try job too often on ios_dbg_simulator for step(s) crypto_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[robertshield, 2014-03-28 05:16:57]

The CQ bit was checked by robertshield@chromium.org

[commit-bot: I haz the power, 2014-03-28 05:17:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/260001

[commit-bot: I haz the power, 2014-03-28 07:09:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 07:10:00]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[MAD, 2014-03-28 11:14:31]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-28 11:15:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/260001

[commit-bot: I haz the power, 2014-03-28 13:14:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 13:14:28]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[MAD, 2014-03-28 14:05:14]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-28 14:06:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/260001

[commit-bot: I haz the power, 2014-03-28 16:20:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 16:20:36]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[MAD, 2014-03-28 17:41:08]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-28 17:41:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/280001

[commit-bot: I haz the power, 2014-03-28 21:19:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:19:13]

Failed to apply the patch.
svn: Caught signal

[MAD, 2014-03-28 21:40:18]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:41:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mad@chromium.org/211543008/280001

[commit-bot: I haz the power, 2014-03-28 21:46:28]

Change committed as 260289