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

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

BUG=None

Committed: https://crrev.com/8ff5c840f60d0fef4788a4d4d9747f7e0ace641c
Cr-Commit-Position: refs/heads/master@{#324209}

[knn, 2015-03-31 11:38:01]

knn@chromium.org changed reviewers:
+ newt@chromium.org

[knn, 2015-03-31 11:38:01]

PTAL.

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

https://codereview.chromium.org/1050473002/diff/1/chrome/browser/android/pref...
File chrome/browser/android/preferences/pref_service_bridge.cc (right):

https://codereview.chromium.org/1050473002/diff/1/chrome/browser/android/pref...
chrome/browser/android/preferences/pref_service_bridge.cc:503: if (!pref ||
!pref->HasUserSetting()) {
nit: don't need curly braces for two-line if statements in C++

https://codereview.chromium.org/1050473002/diff/1/chrome/browser/android/pref...
chrome/browser/android/preferences/pref_service_bridge.cc:509: location_enabled
&= GetAllowLocationEnabled(env, obj);
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]

PTAL.

https://codereview.chromium.org/1050473002/diff/1/chrome/browser/android/pref...
File chrome/browser/android/preferences/pref_service_bridge.cc (right):

https://codereview.chromium.org/1050473002/diff/1/chrome/browser/android/pref...
chrome/browser/android/preferences/pref_service_bridge.cc:503: if (!pref ||
!pref->HasUserSetting()) {
On 2015/04/02 04:15:18, newt wrote:
> nit: don't need curly braces for two-line if statements in C++

Done.

https://codereview.chromium.org/1050473002/diff/1/chrome/browser/android/pref...
chrome/browser/android/preferences/pref_service_bridge.cc:509: location_enabled
&= GetAllowLocationEnabled(env, obj);
On 2015/04/02 04:15:18, newt wrote:
> 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?

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.

[newt (away), 2015-04-03 01:12:40]

lgtm after comment

https://codereview.chromium.org/1050473002/diff/20001/chrome/browser/android/...
File chrome/browser/android/preferences/pref_service_bridge.cc (right):

https://codereview.chromium.org/1050473002/diff/20001/chrome/browser/android/...
chrome/browser/android/preferences/pref_service_bridge.cc:524: pmi_enabled &=
GetProtectedMediaIdentifierEnabled(env, obj);
I'd use the same form here that you use in MigrateLocationPreference(), i.e.

    if (!pmi_enabled)
      SetProtectedMediaIdentifierEnabled(env, obj, false);

[knn, 2015-04-07 15:37:52]

Patchset #3 (id:40001) has been deleted

[knn, 2015-04-08 10:13:36]

The CQ bit was checked by knn@chromium.org

[knn, 2015-04-08 10:13:37]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1050473002/#ps60001
(title: "Address Comment")

[commit-bot: I haz the power, 2015-04-08 10:14:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1050473002/60001

[commit-bot: I haz the power, 2015-04-08 10:18:10]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-04-08 10:18:56]

Patchset 3 (id:??) landed as
https://crrev.com/8ff5c840f60d0fef4788a4d4d9747f7e0ace641c
Cr-Commit-Position: refs/heads/master@{#324209}