Add Now setting and migration logic.

chrome/browser/ui/app_list/google_now_extension.cc

Index: chrome/browser/ui/app_list/google_now_extension.cc
diff --git a/chrome/browser/ui/app_list/google_now_extension.cc b/chrome/browser/ui/app_list/google_now_extension.cc
index 303eb301a5c56ad69f6cf63543060a10c3b41a9d..816016cf03a12ec23ff9af9dcfa5f788b34543e9 100644
--- a/chrome/browser/ui/app_list/google_now_extension.cc
+++ b/chrome/browser/ui/app_list/google_now_extension.cc
@@ -4,10 +4,18 @@
 
 #include "chrome/browser/ui/app_list/google_now_extension.h"
 
+#include "base/prefs/pref_service.h"
+#include "chrome/browser/notifications/desktop_notification_service.h"
+#include "chrome/browser/notifications/desktop_notification_service_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
 #include "components/variations/variations_associated_data.h"
 
 namespace {
 const char kGoogleNowExtensionFieldTrialName[] = "GoogleNowExtension";
+
+// Extension ID for previous Now notifications component.
+const char kNowNotifierId[] = "pafkbggdmjlpgkdkcbjmhmfcdpncadgh";
 };  // namespace
 
 bool GetGoogleNowExtensionId(std::string* extension_id) {
@@ -15,3 +23,28 @@ bool GetGoogleNowExtensionId(std::string* extension_id) {
       kGoogleNowExtensionFieldTrialName, "id");
   return !extension_id->empty();
 }
+
+// TODO(skare): Remove this if/when the Now Notifications component is
+// deprecated. http://crbug.com/459846
+void MigrateGoogleNowPrefs(Profile* profile) {
+  std::string now_extension_id;
+  if (!GetGoogleNowExtensionId(&now_extension_id)) {

[Matt Giuca, 2015/02/20 01:40:30]

nit: No curly braces (one-line condition and one-line body).

[skare_, 2015/02/20 17:54:49]

Done.

+    return;
+  }
+
+  PrefService* prefs = profile->GetPrefs();
+  const PrefService::Preference* enabled_pref =
+      prefs->FindPreference(prefs::kGoogleNowLauncherEnabled);
+
+  // If the pref is not its default value, migration was performed.
+  if (!enabled_pref->IsDefaultValue()) {

[Matt Giuca, 2015/02/20 01:40:30]

Nit: No curly braces.

[Matt Giuca, 2015/02/20 01:40:30]

Can I just clarify the meaning of IsDefaultValue()? The documentation is a
little unclear.

Does this mean:
a) "the pref has not been explicitly set; it is still on its default setting",
or
b) "the pref is currently set to the value equal to its default value".

That is, if I explicitly set the pref to true, in (a), IsDefaultValue() returns
false, but in (b), IsDefaultValue() returns true because true is the default.

We do not want to migrate whenever this pref is true. We only want to migrate if
we have never set this pref before. So I will assume that (a) is the case.
Please let me know if (b) is the case, because then there is a problem!

(And perhaps this method should be called "HasBeenSet" instead of
"IsDefaultValue"!)

[skare_, 2015/02/20 17:54:49]

yes, it's akin to a) the pref has not been explicitly set.

[skare_, 2015/02/20 17:54:49]

Done.

+    return;
+  }
+
+  DesktopNotificationService* const notification_service =
+      DesktopNotificationServiceFactory::GetForProfile(profile);
+  bool notifier_enabled = notification_service->IsNotifierEnabled(
+      message_center::NotifierId(
+          message_center::NotifierId::APPLICATION, kNowNotifierId));
+  prefs->SetBoolean(prefs::kGoogleNowLauncherEnabled, notifier_enabled);

[gab, 2015/02/20 15:11:21]

User prefs should typically reflect explicit user decision, i.e. this is why we
strike a difference between IsDefaultValue() and the user value being the same
as the current default (yes Matt (a.) is correct from your previous question).

This allows us to change the default in a later release and affect only users
that haven't made an explicit choice already.

In this case looks like you're using the IsDefaultValue() check to know if
you've set it and if not unconditionally set it which is counter to this
principle.

I'm not going to block for this if you think this is correct for your feature
but just letting you know that this is slightly abusing the API and will prevent
you from changing the default value later.

[skare_, 2015/02/20 17:54:49]

thanks for verifying Matt's question and the detailed explanation. Yes, this is
the intent despite making the setting explicit on migration. Verified with
kuscher/PM (cced on review) to use this approach similar to a couple other
migration paths (like autofill). We could introduce a new nowHasBeenMigrated
pref if anyone has second thoughts.

+}