Migrate Location and Protected Media Identifier Settings at startup instead of the getter.

chrome/browser/android/preferences/pref_service_bridge.cc

Index: chrome/browser/android/preferences/pref_service_bridge.cc
diff --git a/chrome/browser/android/preferences/pref_service_bridge.cc b/chrome/browser/android/preferences/pref_service_bridge.cc
index 55f74233bdbc398ff5cfd96717f98f305132c2c4..cf7b2b795a4b95158e96fc6cb8d5820155d61e5d 100644
--- a/chrome/browser/android/preferences/pref_service_bridge.cc
+++ b/chrome/browser/android/preferences/pref_service_bridge.cc
@@ -230,13 +230,6 @@ static jboolean GetSearchSuggestManaged(JNIEnv* env, jobject obj) {
 }
 
 static jboolean GetProtectedMediaIdentifierEnabled(JNIEnv* env, jobject obj) {
-  // Migrate Settings from Preferences to the Host Content Settings Map.
-  // TODO (knn): Remove this once users have migrated.
-  PrefService* prefs = GetPrefService();
-  if (!prefs->GetBoolean(prefs::kProtectedMediaIdentifierEnabled)) {
-    prefs->SetBoolean(prefs::kProtectedMediaIdentifierEnabled, true);
-    SetProtectedMediaIdentifierEnabled(env, obj, false);
-  }
   return GetBooleanForContentSetting(
       CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER);
 }
@@ -246,15 +239,6 @@ static jboolean GetPushNotificationsEnabled(JNIEnv* env, jobject obj) {
 }
 
 static jboolean GetAllowLocationEnabled(JNIEnv* env, jobject obj) {
-  // Migrate Settings from Preferences to the Host Content Settings Map.
-  // TODO (knn): Remove this once users have migrated.
-  PrefService* prefs = GetPrefService();
-  // Non-user-modifiable preference levels should have been migrated already.
-  DCHECK(prefs->IsUserModifiablePreference(prefs::kGeolocationEnabled));
-  if (!prefs->GetBoolean(prefs::kGeolocationEnabled)) {
-    prefs->SetBoolean(prefs::kGeolocationEnabled, true);
-    SetAllowLocationEnabled(env, obj, false);
-  }
   return GetBooleanForContentSetting(CONTENT_SETTINGS_TYPE_GEOLOCATION);
 }
 
@@ -513,6 +497,34 @@ static void MigrateJavascriptPreference(JNIEnv* env, jobject obj) {
   GetPrefService()->ClearPref(prefs::kWebKitJavascriptEnabled);
 }
 
+static void MigrateLocationPreference(JNIEnv* env, jobject obj) {
+  const PrefService::Preference* pref =
+      GetPrefService()->FindPreference(prefs::kGeolocationEnabled);
+  if (!pref || !pref->HasUserSetting()) {

[newt (away), 2015/04/02 04:15:18]

nit: don't need curly braces for two-line if statements in C++

[knn, 2015/04/02 11:16:40]

Done.

+    return;
+  }
+  bool location_enabled = false;
+  bool retval = pref->GetValue()->GetAsBoolean(&location_enabled);
+  DCHECK(retval);
+  location_enabled &= GetAllowLocationEnabled(env, obj);

[newt (away), 2015/04/02 04:15:18]

Do we really need to check GetAllowLocationEnabled()? Are there users who've
already set a value for CONTENT_SETTINGS_TYPE_GEOLOCATION that's different than
the value of prefs::kGeolocationEnabled?

[knn, 2015/04/02 11:16:40]

Yes, I was migrating this before by setting the value to true instead of
clearing it (Realize it is a bad idea now).
Also thought this might be an overkill initially but erring on the side of
caution and doing a more restrictive migration.

+  SetAllowLocationEnabled(env, obj, location_enabled);
+  GetPrefService()->ClearPref(prefs::kGeolocationEnabled);
+}
+
+static void MigrateProtectedMediaPreference(JNIEnv* env, jobject obj) {
+  const PrefService::Preference* pref =
+      GetPrefService()->FindPreference(prefs::kProtectedMediaIdentifierEnabled);
+  if (!pref || !pref->HasUserSetting()) {
+    return;
+  }
+  bool pmi_enabled = false;
+  bool retval = pref->GetValue()->GetAsBoolean(&pmi_enabled);
+  DCHECK(retval);
+  pmi_enabled &= GetProtectedMediaIdentifierEnabled(env, obj);
+  SetProtectedMediaIdentifierEnabled(env, obj, pmi_enabled);
+  GetPrefService()->ClearPref(prefs::kProtectedMediaIdentifierEnabled);
+}
+
 static void SetPasswordEchoEnabled(JNIEnv* env,
                                    jobject obj,
                                    jboolean passwordEchoEnabled) {