From 3bed26a4bf5a41484a002b337d8643741592e3cc Mon Sep 17 00:00:00 2001 From: "finnur@chromium.org" Date: Thu, 14 Apr 2011 16:05:04 +0000 Subject: Change the upgrade reminder as time passes without user action. Dev channel will show the green arrow badge on the Wrench menu after 1 hour, as before. For stable and beta, I'm changing it so that: After 2 days you get a green arrow. After 4 days you get an orange arrow. After 7 days you get a red arrow. After 14 days you get an exlamation point. Also make it use wall-clock time instead of ticks when calculating when to show the reminder. BUG=78406, 71202. TEST=Pass in --check-for-update-interval=10 as a command line into Chrome and when a new build is available the numbers above change into minutes (instead of hours). Note the part about the dev build above (it won't get more than a green arrow). Review URL: http://codereview.chromium.org/6840038 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81598 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/ui/toolbar/wrench_menu_model.cc | 48 +++++++++++-- chrome/browser/ui/toolbar/wrench_menu_model.h | 37 +++++----- chrome/browser/ui/views/toolbar_view.cc | 19 +++++- chrome/browser/ui/views/toolbar_view.h | 3 + chrome/browser/upgrade_detector.cc | 93 ++++++++++++++++++++------ chrome/browser/upgrade_detector.h | 29 +++++++- 6 files changed, 183 insertions(+), 46 deletions(-) (limited to 'chrome/browser') diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.cc b/chrome/browser/ui/toolbar/wrench_menu_model.cc index f17cabc..e8dd206 100644 --- a/chrome/browser/ui/toolbar/wrench_menu_model.cc +++ b/chrome/browser/ui/toolbar/wrench_menu_model.cc @@ -225,7 +225,8 @@ bool WrenchMenuModel::IsItemForCommandIdDynamic(int command_id) const { command_id == IDC_FULLSCREEN || #endif command_id == IDC_SYNC_BOOKMARKS || - command_id == IDC_VIEW_BACKGROUND_PAGES; + command_id == IDC_VIEW_BACKGROUND_PAGES || + command_id == IDC_UPGRADE_DIALOG; } string16 WrenchMenuModel::GetLabelForCommandId(int command_id) const { @@ -249,12 +250,53 @@ string16 WrenchMenuModel::GetLabelForCommandId(int command_id) const { return l10n_util::GetStringFUTF16(IDS_VIEW_BACKGROUND_PAGES, num_background_pages); } + case IDC_UPGRADE_DIALOG: { +#if defined(OS_CHROMEOS) + const string16 product_name = + l10n_util::GetStringUTF16(IDS_PRODUCT_OS_NAME); +#else + const string16 product_name = l10n_util::GetStringUTF16(IDS_PRODUCT_NAME); +#endif + + return l10n_util::GetStringFUTF16(IDS_UPDATE_NOW, product_name); + } default: NOTREACHED(); return string16(); } } +bool WrenchMenuModel::GetIconForCommandId(int command_id, + SkBitmap* icon) const { + switch (command_id) { + case IDC_UPGRADE_DIALOG: { + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + int resource_id; + UpgradeDetector::UpgradeNotificationAnnoyanceLevel stage = + UpgradeDetector::GetInstance()->upgrade_notification_stage(); + switch (stage) { + case UpgradeDetector::UPGRADE_ANNOYANCE_SEVERE: + resource_id = IDR_UPDATE_MENU4; + break; + case UpgradeDetector::UPGRADE_ANNOYANCE_HIGH: + resource_id = IDR_UPDATE_MENU3; + break; + case UpgradeDetector::UPGRADE_ANNOYANCE_ELEVATED: + resource_id = IDR_UPDATE_MENU2; + break; + default: + resource_id = IDR_UPDATE_MENU; + break; + } + *icon = *rb.GetBitmapNamed(resource_id); + break; + } + default: + break; + } + return false; +} + void WrenchMenuModel::ExecuteCommand(int command_id) { browser_->ExecuteCommand(command_id); } @@ -442,10 +484,8 @@ void WrenchMenuModel::Build() { AddItem(IDC_VIEW_INCOMPATIBILITIES, l10n_util::GetStringUTF16( IDS_VIEW_INCOMPATIBILITIES)); - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - SetIcon(GetIndexOfCommandId(IDC_UPGRADE_DIALOG), - *rb.GetBitmapNamed(IDR_UPDATE_MENU)); #if defined(OS_WIN) + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); SetIcon(GetIndexOfCommandId(IDC_VIEW_INCOMPATIBILITIES), *rb.GetBitmapNamed(IDR_CONFLICT_MENU)); #endif diff --git a/chrome/browser/ui/toolbar/wrench_menu_model.h b/chrome/browser/ui/toolbar/wrench_menu_model.h index dd8f597..7e12dcf 100644 --- a/chrome/browser/ui/toolbar/wrench_menu_model.h +++ b/chrome/browser/ui/toolbar/wrench_menu_model.h @@ -29,11 +29,12 @@ class EncodingMenuModel : public ui::SimpleMenuModel, virtual ~EncodingMenuModel(); // Overridden from ui::SimpleMenuModel::Delegate: - virtual bool IsCommandIdChecked(int command_id) const; - virtual bool IsCommandIdEnabled(int command_id) const; - virtual bool GetAcceleratorForCommandId(int command_id, - ui::Accelerator* accelerator); - virtual void ExecuteCommand(int command_id); + virtual bool IsCommandIdChecked(int command_id) const OVERRIDE; + virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE; + virtual bool GetAcceleratorForCommandId( + int command_id, + ui::Accelerator* accelerator) OVERRIDE; + virtual void ExecuteCommand(int command_id) OVERRIDE; private: void Build(); @@ -79,34 +80,36 @@ class WrenchMenuModel : public ui::SimpleMenuModel, virtual ~WrenchMenuModel(); // Overridden for ButtonMenuItemModel::Delegate: - virtual bool DoesCommandIdDismissMenu(int command_id) const; + virtual bool DoesCommandIdDismissMenu(int command_id) const OVERRIDE; // Overridden for both ButtonMenuItemModel::Delegate and SimpleMenuModel: - virtual bool IsItemForCommandIdDynamic(int command_id) const; - virtual string16 GetLabelForCommandId(int command_id) const; - virtual void ExecuteCommand(int command_id); - virtual bool IsCommandIdChecked(int command_id) const; - virtual bool IsCommandIdEnabled(int command_id) const; - virtual bool IsCommandIdVisible(int command_id) const; + virtual bool IsItemForCommandIdDynamic(int command_id) const OVERRIDE; + virtual string16 GetLabelForCommandId(int command_id) const OVERRIDE; + virtual bool GetIconForCommandId(int command_id, + SkBitmap* icon) const OVERRIDE; + virtual void ExecuteCommand(int command_id) OVERRIDE; + virtual bool IsCommandIdChecked(int command_id) const OVERRIDE; + virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE; + virtual bool IsCommandIdVisible(int command_id) const OVERRIDE; virtual bool GetAcceleratorForCommandId( int command_id, - ui::Accelerator* accelerator); + ui::Accelerator* accelerator) OVERRIDE; // Overridden from TabStripModelObserver: virtual void TabSelectedAt(TabContentsWrapper* old_contents, TabContentsWrapper* new_contents, int index, - bool user_gesture); + bool user_gesture) OVERRIDE; virtual void TabReplacedAt(TabStripModel* tab_strip_model, TabContentsWrapper* old_contents, TabContentsWrapper* new_contents, - int index); - virtual void TabStripModelDeleted(); + int index) OVERRIDE; + virtual void TabStripModelDeleted() OVERRIDE; // Overridden from NotificationObserver: virtual void Observe(NotificationType type, const NotificationSource& source, - const NotificationDetails& details); + const NotificationDetails& details) OVERRIDE; // Getters. Browser* browser() const { return browser_; } diff --git a/chrome/browser/ui/views/toolbar_view.cc b/chrome/browser/ui/views/toolbar_view.cc index 18bdf87..8cf2bca 100644 --- a/chrome/browser/ui/views/toolbar_view.cc +++ b/chrome/browser/ui/views/toolbar_view.cc @@ -618,6 +618,23 @@ bool ToolbarView::IsUpgradeRecommended() { #endif } +int ToolbarView::GetUpgradeRecommendedBadge() const { +#if defined(OS_CHROMEOS) + return IDR_UPDATE_BADGE; +#else + switch (UpgradeDetector::GetInstance()->upgrade_notification_stage()) { + case UpgradeDetector::UPGRADE_ANNOYANCE_SEVERE: + return IDR_UPDATE_BADGE4; + case UpgradeDetector::UPGRADE_ANNOYANCE_HIGH: + return IDR_UPDATE_BADGE3; + case UpgradeDetector::UPGRADE_ANNOYANCE_ELEVATED: + return IDR_UPDATE_BADGE2; + default: + return IDR_UPDATE_BADGE; + } +#endif +} + bool ToolbarView::ShouldShowIncompatibilityWarning() { #if defined(OS_WIN) EnumerateModulesModel* loaded_modules = EnumerateModulesModel::GetInstance(); @@ -733,7 +750,7 @@ SkBitmap ToolbarView::GetAppMenuIcon(views::CustomButton::ButtonState state) { // Only one badge can be active at any given time. The Upgrade notification // is deemed most important, then the DLL conflict badge. if (IsUpgradeRecommended()) { - badge = *tp->GetBitmapNamed(IDR_UPDATE_BADGE); + badge = *tp->GetBitmapNamed(GetUpgradeRecommendedBadge()); } else if (ShouldShowIncompatibilityWarning()) { #if defined(OS_WIN) if (!was_showing) diff --git a/chrome/browser/ui/views/toolbar_view.h b/chrome/browser/ui/views/toolbar_view.h index 199a2f0..53616fc 100644 --- a/chrome/browser/ui/views/toolbar_view.h +++ b/chrome/browser/ui/views/toolbar_view.h @@ -144,6 +144,9 @@ class ToolbarView : public AccessiblePaneView, // Returns true if we should show the upgrade recommended dot. bool IsUpgradeRecommended(); + // Retrieve which badge we should show when recommending an upgrade. + int GetUpgradeRecommendedBadge() const; + // Returns true if we should show the background page badge. bool ShouldShowBackgroundPageBadge(); diff --git a/chrome/browser/upgrade_detector.cc b/chrome/browser/upgrade_detector.cc index 8d8d60d..19e9703 100644 --- a/chrome/browser/upgrade_detector.cc +++ b/chrome/browser/upgrade_detector.cc @@ -35,38 +35,42 @@ namespace { +// How long (in milliseconds) to wait (each cycle) before checking whether +// Chrome's been upgraded behind our back. +const int kCheckForUpgradeMs = 2 * 60 * 60 * 1000; // 2 hours. + +// How long to wait (each cycle) before checking which severity level we should +// be at. Once we reach the highest severity, the timer will stop. +const int kNotifyCycleTimeMs = 20 * 60 * 1000; // 20 minutes. + +// Same as kNotifyCycleTimeMs but only used during testing. +const int kNotifyCycleTimeForTestingMs = 5000; // 5 seconds. + +std::string CmdLineInterval() { + const CommandLine& cmd_line = *CommandLine::ForCurrentProcess(); + return cmd_line.GetSwitchValueASCII(switches::kCheckForUpdateIntervalSec); +} + // How often to check for an upgrade. int GetCheckForUpgradeEveryMs() { // Check for a value passed via the command line. int interval_ms; - const CommandLine& cmd_line = *CommandLine::ForCurrentProcess(); - std::string interval = - cmd_line.GetSwitchValueASCII(switches::kCheckForUpdateIntervalSec); + std::string interval = CmdLineInterval(); if (!interval.empty() && base::StringToInt(interval, &interval_ms)) return interval_ms * 1000; // Command line value is in seconds. - // Otherwise check once an hour for dev channel and once a day for all other - // channels/builds. - const std::string channel = platform_util::GetVersionStringModifier(); - int hours; - if (channel == "dev") - hours = 1; - else - hours = 24; - - return hours * 60 * 60 * 1000; + return kCheckForUpgradeMs; } -// How long to wait before notifying the user about the upgrade. -const int kNotifyUserAfterMs = 0; - // This task checks the currently running version of Chrome against the // installed version. If the installed version is newer, it runs the passed // callback task. Otherwise it just deletes the task. class DetectUpgradeTask : public Task { public: - explicit DetectUpgradeTask(Task* upgrade_detected_task) - : upgrade_detected_task_(upgrade_detected_task) { + explicit DetectUpgradeTask(Task* upgrade_detected_task, + bool* is_dev_channel) + : upgrade_detected_task_(upgrade_detected_task), + is_dev_channel_(is_dev_channel) { } virtual ~DetectUpgradeTask() { @@ -111,6 +115,9 @@ class DetectUpgradeTask : public Task { installed_version.reset(Version::GetVersionFromString(reply)); #endif + const std::string channel = platform_util::GetVersionStringModifier(); + *is_dev_channel_ = channel == "dev"; + // Get the version of the currently *running* instance of Chrome. chrome::VersionInfo version_info; if (!version_info.is_valid()) { @@ -137,6 +144,7 @@ class DetectUpgradeTask : public Task { private: Task* upgrade_detected_task_; + bool* is_dev_channel_; }; } // namespace @@ -148,6 +156,8 @@ void UpgradeDetector::RegisterPrefs(PrefService* prefs) { UpgradeDetector::UpgradeDetector() : ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)), + is_dev_channel_(false), + upgrade_notification_stage_(UPGRADE_ANNOYANCE_NONE), notify_upgrade_(false) { CommandLine command_line(*CommandLine::ForCurrentProcess()); if (command_line.HasSwitch(switches::kDisableBackgroundNetworking)) @@ -184,7 +194,8 @@ void UpgradeDetector::CheckForUpgrade() { // while launching a background process and reading its output; on the Mac and // on Windows checking for an upgrade requires reading a file. BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, - new DetectUpgradeTask(callback_task)); + new DetectUpgradeTask(callback_task, + &is_dev_channel_)); } void UpgradeDetector::UpgradeDetected() { @@ -193,18 +204,58 @@ void UpgradeDetector::UpgradeDetected() { // Stop the recurring timer (that is checking for changes). detect_upgrade_timer_.Stop(); + upgrade_detected_time_ = base::Time::Now(); + NotificationService::current()->Notify( NotificationType::UPGRADE_DETECTED, Source(this), NotificationService::NoDetails()); - // Start the OneShot timer for notifying the user after a certain period. + // Start the repeating timer for notifying the user after a certain period. + // The called function will eventually figure out that enough time has passed + // and stop the timer. + int cycle_time = CmdLineInterval().empty() ? kNotifyCycleTimeMs : + kNotifyCycleTimeForTestingMs; upgrade_notification_timer_.Start( - base::TimeDelta::FromMilliseconds(kNotifyUserAfterMs), + base::TimeDelta::FromMilliseconds(cycle_time), this, &UpgradeDetector::NotifyOnUpgrade); } void UpgradeDetector::NotifyOnUpgrade() { + base::TimeDelta delta = base::Time::Now() - upgrade_detected_time_; + std::string interval = CmdLineInterval(); + // A command line interval implies testing, which we'll make more convenient + // by switching to minutes of waiting instead of hours between flipping + // severity. + int time_passed = interval.empty() ? delta.InHours() : delta.InMinutes(); + const int kSevereThreshold = 14 * (interval.empty() ? 24 : 1); + const int kHighThreshold = 7 * (interval.empty() ? 24 : 1); + const int kElevatedThreshold = 4 * (interval.empty() ? 24 : 1); + // Dev channel is fixed at lowest severity after 1 hour. For other channels + // it is after 2 hours. And, as before, if a command line is passed in we + // drastically reduce the wait time. + const int multiplier = is_dev_channel_ ? 1 : 2; + const int kLowThreshold = multiplier * (interval.empty() ? 24 : 1); + + // These if statements (except for the first one) must be sorted (highest + // interval first). + if (time_passed >= kSevereThreshold) + upgrade_notification_stage_ = UPGRADE_ANNOYANCE_SEVERE; + else if (time_passed >= kHighThreshold) + upgrade_notification_stage_ = UPGRADE_ANNOYANCE_HIGH; + else if (time_passed >= kElevatedThreshold) + upgrade_notification_stage_ = UPGRADE_ANNOYANCE_ELEVATED; + else if (time_passed >= kLowThreshold) + upgrade_notification_stage_ = UPGRADE_ANNOYANCE_LOW; + else + return; // Not ready to recommend upgrade. + + if (is_dev_channel_ || + upgrade_notification_stage_ == UPGRADE_ANNOYANCE_SEVERE) { + // We can't get any higher, baby. + upgrade_notification_timer_.Stop(); + } + notify_upgrade_ = true; NotificationService::current()->Notify( diff --git a/chrome/browser/upgrade_detector.h b/chrome/browser/upgrade_detector.h index e6aa6a6..659ffb8 100644 --- a/chrome/browser/upgrade_detector.h +++ b/chrome/browser/upgrade_detector.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -23,6 +23,15 @@ class PrefService; // class UpgradeDetector { public: + // The Homeland Security Upgrade Advisory System. + enum UpgradeNotificationAnnoyanceLevel { + UPGRADE_ANNOYANCE_NONE = 0, // What? Me worry? + UPGRADE_ANNOYANCE_LOW, // Green. + UPGRADE_ANNOYANCE_ELEVATED, // Yellow. + UPGRADE_ANNOYANCE_HIGH, // Red. + UPGRADE_ANNOYANCE_SEVERE, // Orange. + }; + // Returns the singleton instance. static UpgradeDetector* GetInstance(); @@ -32,6 +41,10 @@ class UpgradeDetector { bool notify_upgrade() { return notify_upgrade_; } + UpgradeNotificationAnnoyanceLevel upgrade_notification_stage() const { + return upgrade_notification_stage_; + } + private: friend struct DefaultSingletonTraits; @@ -52,14 +65,24 @@ class UpgradeDetector { // We periodically check to see if Chrome has been upgraded. base::RepeatingTimer detect_upgrade_timer_; - // After we detect an upgrade we wait a set time before notifying the user. - base::OneShotTimer upgrade_notification_timer_; + // After we detect an upgrade we start a recurring timer to see if enough time + // has passed and we should start notifying the user. + base::RepeatingTimer upgrade_notification_timer_; // We use this factory to create callback tasks for UpgradeDetected. We pass // the task to the actual upgrade detection code, which is in // DetectUpgradeTask. ScopedRunnableMethodFactory method_factory_; + // When the upgrade was detected. + base::Time upgrade_detected_time_; + + // Whether this build is a dev channel build or not. + bool is_dev_channel_; + + // The stage at which the annoyance level for upgrade notifications is at. + UpgradeNotificationAnnoyanceLevel upgrade_notification_stage_; + // Whether we have waited long enough after detecting an upgrade (to see // is we should start nagging about upgrading). bool notify_upgrade_; -- cgit v1.1