Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2207003002:
Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled (Closed)
DescriptionClears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled.
MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy.
This logic can be removed once all users' prefs are cleared
BUG=630100
Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214
Committed: https://crrev.com/131e08f78c7d001d959dce36bcbcbc3f988395ab
Cr-Original-Commit-Position: refs/heads/master@{#410105}
Cr-Commit-Position: refs/heads/master@{#410144}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Addressed comments #
Total comments: 4
Patch Set 3 : Addressed comments #
Total comments: 2
Patch Set 4 : Addressed comment #Patch Set 5 : Fixed compilation error #
Messages
Total messages: 37 (20 generated)
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared ==========
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 ==========
mahmadi@chromium.org changed reviewers: + michaelpg@chromium.org, rkaplow@chromium.org, rogerta@chromium.org
Hi, Please review this CL.
lgtm actions lg https://codereview.chromium.org/2207003002/diff/1/tools/metrics/actions/actio... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2207003002/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:14798: <action name="UserManager_Cleared_Guest_Mode_Enabled_Add_Person_Enabled_Prefs"> this is ok, but I would prefer a shorter name, if possibkle https://codereview.chromium.org/2207003002/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:14802: prefs::kBrowserAddPersonEnabled are cleared b/c they are no longer exist in nit, write out because
https://codereview.chromium.org/2207003002/diff/1/tools/metrics/actions/actio... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2207003002/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:14798: <action name="UserManager_Cleared_Guest_Mode_Enabled_Add_Person_Enabled_Prefs"> On 2016/08/03 17:05:29, rkaplow wrote: > this is ok, but I would prefer a shorter name, if possibkle Done. https://codereview.chromium.org/2207003002/diff/1/tools/metrics/actions/actio... tools/metrics/actions/actions.xml:14802: prefs::kBrowserAddPersonEnabled are cleared b/c they are no longer exist in On 2016/08/03 17:05:29, rkaplow wrote: > nit, write out because Done.
https://codereview.chromium.org/2207003002/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/signin/user_manager_screen_handler.cc (right): https://codereview.chromium.org/2207003002/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/signin/user_manager_screen_handler.cc:302: const PrefService::Preference* guestModeEnabledPref = var_names_like_this https://codereview.chromium.org/2207003002/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/signin/user_manager_screen_handler.cc:308: (guestModeEnabledPref->IsUserControlled() || if a pref is policy-controlled, don't we still want to clear the value in the user pref store?
https://codereview.chromium.org/2207003002/diff/20001/chrome/browser/ui/webui... File chrome/browser/ui/webui/signin/user_manager_screen_handler.cc (right): https://codereview.chromium.org/2207003002/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/signin/user_manager_screen_handler.cc:302: const PrefService::Preference* guestModeEnabledPref = On 2016/08/04 17:59:45, michaelpg wrote: > var_names_like_this Done. https://codereview.chromium.org/2207003002/diff/20001/chrome/browser/ui/webui... chrome/browser/ui/webui/signin/user_manager_screen_handler.cc:308: (guestModeEnabledPref->IsUserControlled() || On 2016/08/04 17:59:46, michaelpg wrote: > if a pref is policy-controlled, don't we still want to clear the value in the > user pref store? you're right. I should be using HasUserSetting()
lgtm so this completely fixes the bug, then? https://codereview.chromium.org/2207003002/diff/40001/tools/metrics/actions/a... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2207003002/diff/40001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:14850: prefs::kBrowserAddPersonEnabled are cleared because they are no longer exist nit: grammar ("because they no longer exist")
The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Let's hope so :) https://codereview.chromium.org/2207003002/diff/40001/tools/metrics/actions/a... File tools/metrics/actions/actions.xml (right): https://codereview.chromium.org/2207003002/diff/40001/tools/metrics/actions/a... tools/metrics/actions/actions.xml:14850: prefs::kBrowserAddPersonEnabled are cleared because they are no longer exist On 2016/08/04 19:23:41, michaelpg wrote: > nit: grammar ("because they no longer exist") Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
On 2016/08/05 15:29:15, Roger Tawa wrote: > lgtm Thank you.
The CQ bit was checked by mahmadi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org, michaelpg@chromium.org Link to the patchset: https://codereview.chromium.org/2207003002/#ps60001 (title: "Addressed comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2218173002/ by anthonyvd@chromium.org. The reason for reverting is: Reverting this CL because it breaks compile on the following bot: https://build.chromium.org/p/chromium.gpu/builders/GPU%20Mac%20Builder/builds....
Message was sent while issue was closed.
FYI: Findit try jobs (rerunning failed compile or tests) identified this CL at revision 410105 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...
Message was sent while issue was closed.
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105} ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105} ==========
The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mahmadi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rogerta@chromium.org, rkaplow@chromium.org, michaelpg@chromium.org Link to the patchset: https://codereview.chromium.org/2207003002/#ps80001 (title: "Fixed compilation error")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105} ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105} ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Cr-Commit-Position: refs/heads/master@{#410105} ========== to ========== Clears GuestModeEnabled and AddPersonEnabled user prefs if MD settings is enabled. MD settings gets rid of the checkboxes that enabled browsing as guest and adding new profiles. This CL clears the two prefs previously set by users. This should have no effect if the prefs are set by policy. This logic can be removed once all users' prefs are cleared BUG=630100 Committed: https://crrev.com/8c440d2c549779305eb5439ae451158e694b5214 Committed: https://crrev.com/131e08f78c7d001d959dce36bcbcbc3f988395ab Cr-Original-Commit-Position: refs/heads/master@{#410105} Cr-Commit-Position: refs/heads/master@{#410144} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/131e08f78c7d001d959dce36bcbcbc3f988395ab Cr-Commit-Position: refs/heads/master@{#410144} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
