Initial CL for Data Saver (Flywheel) prompt when cellular network detected

chrome/browser/chromeos/status/data_promo_notification.cc

Index: chrome/browser/chromeos/status/data_promo_notification.cc
diff --git a/chrome/browser/chromeos/status/data_promo_notification.cc b/chrome/browser/chromeos/status/data_promo_notification.cc
index 0cf7b9cfaa21517d8720174fcf7db461491b2219..fb88aa0f04a307b6554797a3483a5a75db76cf32 100644
--- a/chrome/browser/chromeos/status/data_promo_notification.cc
+++ b/chrome/browser/chromeos/status/data_promo_notification.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/status/data_promo_notification.h"
 
 #include "ash/system/system_notifier.h"
+#include "base/command_line.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
@@ -17,14 +18,17 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/singleton_tabs.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
+#include "chromeos/chromeos_switches.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/network/device_state.h"
 #include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
+#include "extensions/browser/extension_registry.h"
 #include "grit/ui_chromeos_resources.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -41,8 +45,13 @@ namespace chromeos {
 namespace {
 
 const char kDataPromoNotificationId[] = "chrome://settings/internet/data_promo";
+const char kDataSaverNotificationId[] = "chrome://settings/internet/data_saver";
+const char kDataSaverExtensionUrl[] =
+    "https://chrome.google.com/webstore/detail/"
+    "pfmgfdlgomnbgkofeojodiodmgpgmkac";
 
 const int kNotificationCountPrefDefault = -1;
+const int kTimesToShowDataSaverPrompt = 2;
 
 bool GetBooleanPref(const char* pref_name) {
   Profile* profile = ProfileManager::GetPrimaryUserProfile();
@@ -85,6 +94,41 @@ void SetCarrierDealPromoShown(int value) {
   SetIntegerLocalPref(prefs::kCarrierDealPromoShown, value);
 }
 
+// Returns number of times the Data Saver prompt has been displayed.
+int GetDataSaverPromptsShown() {
+  return ProfileManager::GetActiveUserProfile()->GetPrefs()->GetInteger(
+      prefs::kDataSaverPromptsShown);
+}
+
+// Updates number of times the Data Saver prompt has been displayed.
+void SetDataSaverPromptsShown(int times_shown) {
+  ProfileManager::GetActiveUserProfile()->GetPrefs()->SetInteger(
+      prefs::kDataSaverPromptsShown, times_shown);
+}
+
+// Is command line switch set for Data Saver prompt?
+bool DataSaverSwitchEnabled() {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kDisableDataSaverPrompt))
+    return false;
+
+  return base::CommandLine::ForCurrentProcess()->HasSwitch(
+      chromeos::switches::kEnableDataSaverPrompt);
+}
+
+// Is command line switch set for Data Saver demo mode, where we show the prompt
+// after any change in network properties?
+bool DataSaverSwitchDemoMode() {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kDisableDataSaverPrompt))
+    return false;
+
+  const std::string value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          chromeos::switches::kEnableDataSaverPrompt);
+  return value == chromeos::switches::kDataSaverPromptDemoMode;
+}
+
 const chromeos::MobileConfig::Carrier* GetCarrier(
     const NetworkState* cellular) {
   const DeviceState* device = NetworkHandler::Get()->network_state_handler()->
@@ -138,7 +182,7 @@ void NotificationClicked(const std::string& service_path,
 // DataPromoNotification
 
 DataPromoNotification::DataPromoNotification()
-    : check_for_promo_(true),
+    : notifications_shown_(false),
       weak_ptr_factory_(this) {
   NetworkHandler::Get()->network_state_handler()->AddObserver(this, FROM_HERE);
 }
@@ -157,7 +201,8 @@ void DataPromoNotification::RegisterPrefs(PrefRegistrySimple* registry) {
 
 void DataPromoNotification::NetworkPropertiesUpdated(
     const NetworkState* network) {
-  if (!network || network->type() != shill::kTypeCellular)
+  if (!network ||
+      (network->type() != shill::kTypeCellular && !DataSaverSwitchDemoMode()))
     return;
   ShowOptionalMobileDataPromoNotification();
 }
@@ -169,20 +214,25 @@ void DataPromoNotification::DefaultNetworkChanged(const NetworkState* network) {
 }
 
 void DataPromoNotification::ShowOptionalMobileDataPromoNotification() {
-  // Display a one-time notification for authenticated users on first use
-  // of Mobile Data connection or if there is a carrier deal defined
-  // show that even if user has already seen generic promo.
-  if (!check_for_promo_ || !LoginState::Get()->IsUserAuthenticated())
-    return;
   const NetworkState* default_network =
       NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
-  if (!default_network || default_network->type() != shill::kTypeCellular)
+  // Do not show notifications to unauthenticated users, or when requesting a
+  // network connection, or if no default_network.
+  if (!default_network || !LoginState::Get()->IsUserAuthenticated() ||
+      NetworkHandler::Get()
+          ->network_connection_handler()
+          ->HasPendingConnectRequest())

[stevenjb, 2015/04/03 20:07:26]

nit: I think that keeping these separate is a little more clear. At least maybe
separate out the default_network check after the others.

[Greg Levin, 2015/04/03 22:17:33]

Done.

     return;
-  // When requesting a network connection, do not show the notification.
-  if (NetworkHandler::Get()->network_connection_handler()->
-      HasPendingConnectRequest())
+
+  if (!DataSaverSwitchDemoMode() &&
+      (notifications_shown_ || default_network->type() != shill::kTypeCellular))
     return;
 

[stevenjb, 2015/04/03 20:07:26]

nit: It looks like we can set |notifications_shown_| = true here, since both
return paths below set it to true? (This was true before with |check_for_promo_|
also, I only mention it because it confused me while reviewing this change).

[Greg Levin, 2015/04/03 22:17:33]

Done.

+  bool data_saver_prompt_shown = ShowDataSaverNotification();
+
+  // Display a one-time notification on first use of Mobile Data connection or
+  // if there is a carrier deal defined show that even if user has already seen
+  // generic promo.  Show deal regardless of |data_saver_prompt_shown|.
   int carrier_deal_promo_pref = kNotificationCountPrefDefault;
   const MobileConfig::CarrierDeal* deal = NULL;
   const MobileConfig::Carrier* carrier = GetCarrier(default_network);
@@ -201,8 +251,8 @@ void DataPromoNotification::ShowOptionalMobileDataPromoNotification() {
     info_url = deal->info_url();
     if (info_url.empty() && carrier)
       info_url = carrier->top_up_url();
-  } else if (!ShouldShow3gPromoNotification()) {
-    check_for_promo_ = false;
+  } else if (data_saver_prompt_shown || !ShouldShow3gPromoNotification()) {
+    notifications_shown_ = true;
     return;
   }
 
@@ -220,10 +270,54 @@ void DataPromoNotification::ShowOptionalMobileDataPromoNotification() {
           ui::NetworkStateNotifier::kNotifierNetwork,
           base::Bind(&NotificationClicked, default_network->path(), info_url)));
 
-  check_for_promo_ = false;
+  notifications_shown_ = true;
   SetShow3gPromoNotification(false);
   if (carrier_deal_promo_pref != kNotificationCountPrefDefault)
     SetCarrierDealPromoShown(carrier_deal_promo_pref + 1);
 }
 
+bool DataPromoNotification::ShowDataSaverNotification() {
+  if (!DataSaverSwitchEnabled())
+    return false;
+
+  if (message_center::MessageCenter::Get()->FindVisibleNotificationById(
+          kDataSaverNotificationId))  // already showing.
+    return true;
+
+  int times_shown = GetDataSaverPromptsShown();
+  if (!DataSaverSwitchDemoMode() && times_shown >= kTimesToShowDataSaverPrompt)
+    return false;
+
+  if (extensions::ExtensionRegistry::Get(ProfileManager::GetActiveUserProfile())
+          ->GetExtensionById(extension_misc::kDataSaverExtensionId,
+                             extensions::ExtensionRegistry::EVERYTHING)) {
+    // If extension is installed, disable future prompts.
+    SetDataSaverPromptsShown(kTimesToShowDataSaverPrompt);
+    return false;
+  }
+
+  base::string16 title = l10n_util::GetStringUTF16(IDS_3G_DATASAVER_TITLE);
+  base::string16 message = l10n_util::GetStringUTF16(IDS_3G_DATASAVER_MESSAGE);
+  const gfx::Image& icon =
+      ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+          IDR_AURA_UBER_TRAY_NOTIFICATION_DATASAVER);
+  const std::string path =
+      NetworkHandler::Get()->network_state_handler()->DefaultNetwork()->path();
+
+  message_center::MessageCenter::Get()->AddNotification(
+      message_center::Notification::CreateSystemNotification(
+          kDataSaverNotificationId, title, message, icon,
+          ui::NetworkStateNotifier::kNotifierNetwork,
+          base::Bind(&NotificationClicked, path, kDataSaverExtensionUrl)));

[stevenjb, 2015/04/03 20:07:26]

Is the intended behavior to show the settings for the network if the
notification is clicked? That's probably fine, but not entirely intuitive, I
would confirm with UX that it is the desired behavior.

[Greg Levin, 2015/04/03 22:17:33]

You mean from |path|?  The network settings were never being shown, but it
didn't need to be there either, just leftovers from a copy-paste that I didn't
dig deep enough into.

+
+  if (DataSaverSwitchDemoMode()) {
+    SetDataSaverPromptsShown(0);  // demo mode resets times shown counts.

[stevenjb, 2015/04/03 20:07:26]

If we are resetting the times shown here, do we also need the check on line 288?
Less checks would be better.

[Greg Levin, 2015/04/03 22:17:33]

This is done to accommodate the case where you're switching back and forth
between "enabled" and "demo mode".  The intent is to have demo mode reset the
count, so that if you then switch back to enabled, you can test the system with
a fresh count = 0, check that the notification only appears twice, and then do
this as many times as you want.  I'll document this in the issue description
once this lands.

Does this answer your question, or was I missing your point?

[stevenjb, 2015/04/03 22:27:53]

I see. If we have shown the notification and then switch to demo mode we
wouldn't ever see it without the extra test on line 288. That's fine then.

+    SetShow3gPromoNotification(true);
+  } else {
+    SetDataSaverPromptsShown(times_shown + 1);
+  }
+
+  return true;
+}
+
 }  // namespace chromeos