Add ExtensionManifestV2Availability policy definition [chromium/src : main]
Owen Min (Gerrit)
Attention is currently required from: Enterprise Policy Reviews, Istiaque Ahmed.
Owen Min would like Istiaque Ahmed and Enterprise Policy Reviews to review this change.
Add ExtensionManifestV2Availability policy definition
Add policy yaml definition and policy to pref mapping.
Bug: 1347794
Change-Id: I9dd8c933a08d59cfe3898d7a2317b59ba9563422
---
M chrome/browser/policy/configuration_policy_handler_list_factory.cc
M chrome/test/data/policy/policy_test_cases.json
M components/policy/resources/templates/policies.yaml
A components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml
M components/policy/resources/templates/policy_definitions/Extensions/policy_atomic_groups.yaml
M extensions/browser/pref_names.cc
M extensions/browser/pref_names.h
M tools/metrics/histograms/enums.xml
8 files changed, 106 insertions(+), 1 deletion(-)
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
Attention is currently required from: Enterprise Policy Reviews, Istiaque Ahmed.
gwsq (Gerrit)
Attention is currently required from: Artem Sumaneev, Enterprise Policy Reviews, Istiaque Ahmed.
gwsq would like Artem Sumaneev to review this change authored by Owen Min.
Add ExtensionManifestV2Availability policy definition
Add policy yaml definition and policy to pref mapping.
Bug: 1347794
Change-Id: I9dd8c933a08d59cfe3898d7a2317b59ba9563422
---
M chrome/browser/policy/configuration_policy_handler_list_factory.cc
M chrome/test/data/policy/policy_test_cases.json
M components/policy/resources/templates/policies.yaml
A components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml
M components/policy/resources/templates/policy_definitions/Extensions/policy_atomic_groups.yaml
M extensions/browser/pref_names.cc
M extensions/browser/pref_names.h
M tools/metrics/histograms/enums.xml
8 files changed, 106 insertions(+), 1 deletion(-)
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
gwsq (Gerrit)
Attention is currently required from: Artem Sumaneev, Enterprise Policy Reviews, Istiaque Ahmed, Julian Pastarmov.
gwsq would like Julian Pastarmov to review this change authored by Owen Min.
gwsq (Gerrit)
Attention is currently required from: Artem Sumaneev, Istiaque Ahmed, Julian Pastarmov.
Owen Min has uploaded this change for review.
Add ExtensionManifestV2Availability policy definition
Add policy yaml definition and policy to pref mapping.
Bug: 1347794
Change-Id: I9dd8c933a08d59cfe3898d7a2317b59ba9563422
---
M chrome/browser/policy/configuration_policy_handler_list_factory.cc
M chrome/test/data/policy/policy_test_cases.json
M components/policy/resources/templates/policies.yaml
A components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml
M components/policy/resources/templates/policy_definitions/Extensions/policy_atomic_groups.yaml
M extensions/browser/pref_names.cc
M extensions/browser/pref_names.h
M tools/metrics/histograms/enums.xml
8 files changed, 106 insertions(+), 1 deletion(-)
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
gwsq (Gerrit)
Attention is currently required from: Artem Sumaneev, Istiaque Ahmed, Julian Pastarmov.
From chrome/enterprise/gwsq/enterprise-policy-review.gwsq:
Note: A shadow reviewer was assigned to this CL. go/new-policy-review-process
Shadowed: asumaneev
Reviewer source(s):
asumaneev, pasta...@chromium.org is from context(chrome/enterprise/gwsq/enterprise-policy-review.gwsq)
Artem Sumaneev (Gerrit)
Attention is currently required from: Istiaque Ahmed, Julian Pastarmov.
10 comments:
File chrome/test/data/policy/policy_test_cases.json:
Patch Set #4, Line 3795:
"ExtensionManifestV2Availability": 1Is testcase for value == 0 missing?
File components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml:
Patch Set #4, Line 6:
browsernit: <ph name=\"PRODUCT_NAME\">$1<ex>Google Chrome</ex></ph>
Patch Set #4, Line 8:
supportedremove it? or change to support?
Patch Set #4, Line 12:
Set tahe policy to 0 or leave it unset, .Probably evolved into the next line.
Patch Set #4, Line 13:
browsernit: <ph name=\"PRODUCT_NAME\">$1<ex>Google Chrome</ex></ph>
are
are
are
are
Patch Set #4, Line 26:
int-enumI'd prefer string-enum with corresponding default-disabled-enabled values. It's easier to check such a value on chrome://policy page.
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
Attention is currently required from: Artem Sumaneev, Istiaque Ahmed, Julian Pastarmov.
10 comments:
File chrome/test/data/policy/policy_test_cases.json:
Patch Set #4, Line 3795:
"ExtensionManifestV2Availability": 1Is testcase for value == 0 missing?
I agree it's useful to test pref mapping for different inputs.
However, in the meantime, the policy use `SimplePolicyHandler` which copy policy value to pref without any additional logic. In those cases, I found it's not that useful to go though all possible inputs of a policy.
File components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml:
Patch Set #4, Line 6:
browsernit: <ph name=\"PRODUCT_NAME\">$1<ex>Google Chrome</ex></ph>
I avoid using `Google Chrome` for policies that support both CrOS and other desktop platforms. Also, this is a short work with the same meaning.
Patch Set #4, Line 8:
supportedremove it? or change to support?
Done
Patch Set #4, Line 12:
Set tahe policy to 0 or leave it unset, .Probably evolved into the next line.
Deleted.
Patch Set #4, Line 13:
browsernit: <ph name=\"PRODUCT_NAME\">$1<ex>Google Chrome</ex></ph>
Same comment as above.
are
Done
are
Done
are
Done
are
Done
Patch Set #4, Line 26:
int-enum
I'd prefer string-enum with corresponding default-disabled-enabled values. […]
I agree. However, at the same time, string-enum is also slightly more difficult for admins to setup compared with int-enum. As on many 3rd-party management tool like GPO, admins have to typing the whole word instead of a single digit.
I will choose string-enum for following situations:
1) Chrome OS policy - we can setup better UI on admin console to avoid typing.
2) Complicated policies that have lots of options. It's not the case here a it's still a very basic on/off policy.
For chrome://policy a better approach here is converting int-enum option into C++ string so that we can show more meaning value even for int-enum policies.
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Artem Sumaneev (Gerrit)
Attention is currently required from: Istiaque Ahmed, Julian Pastarmov, Owen Min.
3 comments:
File chrome/test/data/policy/policy_test_cases.json:
Patch Set #4, Line 3795:
"ExtensionManifestV2Availability": 1
I agree it's useful to test pref mapping for different inputs. […]
I see, but I still vote for more coverage, even though it's not the case for some enum policies here.
File components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml:
Patch Set #4, Line 6:
browser
I avoid using `Google Chrome` for policies that support both CrOS and other desktop platforms. […]
Ack
Patch Set #4, Line 26:
int-enum
I agree. […]
With 4 values, the policy is not that simple. But I see your point.
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Julian Pastarmov (Gerrit)
Attention is currently required from: Istiaque Ahmed, Owen Min.
Patch set 6:Code-Review +1
6 comments:
Patchset:
Lgtm with a bunch of small wording nits to make the text easier for translators.
File components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml:
Patch Set #6, Line 12:
<ph name="DEFAULT">Default</ph>Since this is int-enum maybe add the numeric value in brackets behind each of those so that people can match what they see in chrome://policy.
Patch Set #6, Line 12:
browser, follownit: "the browser, following".
Patch Set #6, Line 13:
no-opthis might be hard to translate. Let's go with "This option is going to be treated the same as if the policy is not set"
Patch Set #6, Line 14:
no-opsame as above.
Patch Set #6, Line 15:
force installedmaybe replace this by "installed trough the ExtensionInstallForcelist or ExtensionSettings policy with installation mode "forced"."
please check also the install mode string I put it there from memory.
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
Attention is currently required from: Artem Sumaneev, Istiaque Ahmed.
6 comments:
File chrome/test/data/policy/policy_test_cases.json:
Patch Set #4, Line 3795:
"ExtensionManifestV2Availability": 1
I see, but I still vote for more coverage, even though it's not the case for some enum policies here […]
Done
File components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml:
Patch Set #6, Line 12:
browser, follow
nit: "the browser, following".
Done
Patch Set #6, Line 12:
<ph name="DEFAULT">Default</ph>
Since this is int-enum maybe add the numeric value in brackets behind each of those so that people c […]
Done
Patch Set #6, Line 13:
no-op
this might be hard to translate. […]
Done
Patch Set #6, Line 14:
no-opsame as above.
Done
Patch Set #6, Line 15:
force installed
maybe replace this by "installed trough the ExtensionInstallForcelist or ExtensionSettings policy wi […]
Done
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Istiaque Ahmed (Gerrit)
Attention is currently required from: Artem Sumaneev, Owen Min.
2 comments:
Commit Message:
Also describe (in short) why we need this and how this can be leveraged.
File extensions/browser/pref_names.h:
Patch Set #8, Line 88:
// An integer indicate the support of manifest v2 extensions.We'd need clearer documentation for this. This integer is used as an `enum`, right? References to where more details can be found would be helpful too.
I could find [1] from codesearch. You might want to use sth similar here.
https://source.chromium.org/chromium/chromium/src/+/main:ash/constants/ash_pref_names.cc;l=943;drc=fb406c65738a42757aed8c7702906f258e303867
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
Attention is currently required from: Artem Sumaneev, Istiaque Ahmed.
Patch set 10:-Commit-Queue
2 comments:
Commit Message:
Also describe (in short) why we need this and how this can be leveraged.
Done
File extensions/browser/pref_names.h:
Patch Set #8, Line 88:
// An integer indicate the support of manifest v2 extensions.
We'd need clearer documentation for this. […]
Done
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Istiaque Ahmed (Gerrit)
Attention is currently required from: Artem Sumaneev, Owen Min.
Patch set 10:Code-Review +1
2 comments:
Commit Message:
It will
also extend thenit:
It will also `allow for` extendingOr similar. Otherwise, it could be (mis)read as "this CL will extend mv2 support".
Patchset:
//extensions/browser/*pref* changes lgtm
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Artem Sumaneev (Gerrit)
Attention is currently required from: Owen Min.
Patch set 10:Code-Review +1
1 comment:
Patchset:
LGTM, thanks!
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
Attention is currently required from: Owen Min.
Owen Min uploaded patch set #11 to this change.
Add ExtensionManifestV2Availability policy definition
Add policy yaml definition and policy to pref mapping. This policy
will allow admin test manifest v2 deprecation ahead of time. It will
also allow for extend the manifest v2 support after its full deprecation
in the future.
Bug: 1347794
Change-Id: I9dd8c933a08d59cfe3898d7a2317b59ba9563422
---
M chrome/browser/policy/configuration_policy_handler_list_factory.cc
M chrome/test/data/policy/policy_test_cases.json
M components/policy/resources/templates/policies.yaml
A components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml
M components/policy/resources/templates/policy_definitions/Extensions/policy_atomic_groups.yaml
M extensions/browser/extension_prefs.cc
M extensions/browser/pref_names.cc
M extensions/browser/pref_names.h
M tools/metrics/histograms/enums.xml
9 files changed, 141 insertions(+), 0 deletions(-)
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
1 comment:
Commit Message:
It will
also extend the
nit: […]
Done
To view, visit change 4004453. To unsubscribe, or for help writing mail filters, visit settings.
Owen Min (Gerrit)
Patch set 12:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
10 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: components/policy/resources/templates/policies.yaml
Insertions: 2, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: tools/metrics/histograms/enums.xml
Insertions: 2, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Add ExtensionManifestV2Availability policy definition
Add policy yaml definition and policy to pref mapping. This policy
will allow admin test manifest v2 deprecation ahead of time. It will
also allow for extend the manifest v2 support after its full deprecation
in the future.
Bug: 1347794
Change-Id: I9dd8c933a08d59cfe3898d7a2317b59ba9563422
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004453
Commit-Queue: Owen Min <zm...@chromium.org>
Reviewed-by: Artem Sumaneev <asum...@google.com>
Reviewed-by: Julian Pastarmov <pasta...@chromium.org>
Reviewed-by: Istiaque Ahmed <laz...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1070527}
---
M chrome/browser/policy/configuration_policy_handler_list_factory.cc
M chrome/test/data/policy/policy_test_cases.json
M components/policy/resources/templates/policies.yaml
A components/policy/resources/templates/policy_definitions/Extensions/ExtensionManifestV2Availability.yaml
M components/policy/resources/templates/policy_definitions/Extensions/policy_atomic_groups.yaml
M extensions/browser/extension_prefs.cc
M extensions/browser/pref_names.cc
M extensions/browser/pref_names.h
M tools/metrics/histograms/enums.xml
9 files changed, 147 insertions(+), 0 deletions(-)