Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(27)

Issue 2410113002: Add migration code for the EnabledPlugins and DisabledPlugins policies. (Closed)

Created:
4 years, 2 months ago by pastarmovj
Modified:
4 years, 2 months ago
Reviewers:
Bernhard Bauer
CC:
chromium-reviews, jam
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add migration code for the EnabledPlugins and DisabledPlugins policies. These policies are deprecated in favor of: DefaultPluginSettings (aka. Flash BLOCK/ALLOW) AlwaysOpenPdfExternally (aka. PDF plugin disabled) BUG=654731 TEST=browser_tests:PolicyPrefIndicator Committed: https://crrev.com/c6c75ce329c9b4f9c925056d583dd9f1ae24a9d7 Cr-Commit-Position: refs/heads/master@{#425743}

Patch Set 1 #

Patch Set 2 : Reduce code duplication. #

Patch Set 3 : Remove the direct policy handling for the deprecated policies. #

Patch Set 4 : Add handling for the Exceptions policy. #

Patch Set 5 : Make "*" entries work as expected. #

Patch Set 6 : Rebased on ToT. #

Patch Set 7 : Fix copy/paste thingy that VC doesn't catch. #

Patch Set 8 : Fix clang stuff. #

Patch Set 9 : Fix platforms that has no plugins support. #

Patch Set 10 : Adjust policy tests. #

Total comments: 7

Patch Set 11 : Address comments. #

Patch Set 12 : Invert the matching. #

Total comments: 15

Patch Set 13 : Address comments. #

Patch Set 14 : Remove case insensitivity. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+219 lines, -18 lines) Patch
M chrome/browser/BUILD.gn View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/browser/plugins/plugin_policy_handler.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +44 lines, -0 lines 0 comments Download
A chrome/browser/plugins/plugin_policy_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +119 lines, -0 lines 0 comments Download
M chrome/browser/policy/configuration_policy_handler_list_factory.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +8 lines, -9 lines 0 comments Download
M chrome/test/data/policy/policy_test_cases.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +46 lines, -9 lines 0 comments Download

Messages

Total messages: 35 (22 generated)
pastarmovj
Hi Bernhard, Please take a look at this CL. Thanks! :) Cheers, Julian
4 years, 2 months ago (2016-10-12 13:59:17 UTC) #18
Bernhard Bauer
https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.cc File chrome/browser/plugins/plugin_policy_handler.cc (right): https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.cc#newcode26 chrome/browser/plugins/plugin_policy_handler.cc:26: return NULL; nullptr https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.cc#newcode94 chrome/browser/plugins/plugin_policy_handler.cc:94: // Finally check if ...
4 years, 2 months ago (2016-10-12 15:28:49 UTC) #21
pastarmovj
Thanks for the review! PTAL. https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.cc File chrome/browser/plugins/plugin_policy_handler.cc (right): https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.cc#newcode26 chrome/browser/plugins/plugin_policy_handler.cc:26: return NULL; On 2016/10/12 ...
4 years, 2 months ago (2016-10-13 13:29:00 UTC) #22
Bernhard Bauer
https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.h File chrome/browser/plugins/plugin_policy_handler.h (right): https://codereview.chromium.org/2410113002/diff/200001/chrome/browser/plugins/plugin_policy_handler.h#newcode25 chrome/browser/plugins/plugin_policy_handler.h:25: // the new policies. On 2016/10/13 13:29:00, pastarmovj wrote: ...
4 years, 2 months ago (2016-10-13 14:33:08 UTC) #23
pastarmovj
Inverted the logic as discussed yesterday. PTAL.
4 years, 2 months ago (2016-10-14 10:29:48 UTC) #24
Bernhard Bauer
https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc File chrome/browser/plugins/plugin_policy_handler.cc (right): https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc#newcode56 chrome/browser/plugins/plugin_policy_handler.cc:56: plugin = base::ToUpperASCII(plugin); I think this is also not ...
4 years, 2 months ago (2016-10-14 11:49:46 UTC) #25
pastarmovj
https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc File chrome/browser/plugins/plugin_policy_handler.cc (right): https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc#newcode56 chrome/browser/plugins/plugin_policy_handler.cc:56: plugin = base::ToUpperASCII(plugin); On 2016/10/14 11:49:45, Bernhard Bauer wrote: ...
4 years, 2 months ago (2016-10-14 12:38:59 UTC) #26
Bernhard Bauer
https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc File chrome/browser/plugins/plugin_policy_handler.cc (right): https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc#newcode56 chrome/browser/plugins/plugin_policy_handler.cc:56: plugin = base::ToUpperASCII(plugin); On 2016/10/14 12:38:59, pastarmovj wrote: > ...
4 years, 2 months ago (2016-10-14 15:27:05 UTC) #27
pastarmovj
https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc File chrome/browser/plugins/plugin_policy_handler.cc (right): https://codereview.chromium.org/2410113002/diff/240001/chrome/browser/plugins/plugin_policy_handler.cc#newcode56 chrome/browser/plugins/plugin_policy_handler.cc:56: plugin = base::ToUpperASCII(plugin); On 2016/10/14 15:27:05, Bernhard Bauer wrote: ...
4 years, 2 months ago (2016-10-17 09:07:09 UTC) #28
Bernhard Bauer
LGTM, thanks!
4 years, 2 months ago (2016-10-17 16:07:20 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2410113002/280001
4 years, 2 months ago (2016-10-17 18:18:47 UTC) #31
commit-bot: I haz the power
Committed patchset #14 (id:280001)
4 years, 2 months ago (2016-10-17 19:21:39 UTC) #33
commit-bot: I haz the power
4 years, 2 months ago (2016-10-17 19:25:20 UTC) #35
Message was sent while issue was closed.
Patchset 14 (id:??) landed as
https://crrev.com/c6c75ce329c9b4f9c925056d583dd9f1ae24a9d7
Cr-Commit-Position: refs/heads/master@{#425743}

Powered by Google App Engine
This is Rietveld 408576698