Policy providers aren't loaded early enough, resulting in some policies being missed. |
||||||
Issue descriptionThis is a regression from a recent patch attempting to move loading of local state earlier.
,
Jan 28 2018
I'm requesting a merge to 65 as this breaks enterprise policies.
,
Jan 29 2018
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e6e8f4bdc7b84c018386185282efa5be4029c02 commit 4e6e8f4bdc7b84c018386185282efa5be4029c02 Author: Scott Violet <sky@chromium.org> Date: Mon Jan 29 20:12:01 2018 merge: Changes ordering so that ConfigurationPolicyProviders are set earlier Specifically this converts back to setting the ConfigurationPolicyProviders *before* the PolicyService is created. There is a subtle difference between this code and the way the code was before my patch. Specifically previously the code did the following: 1. ChromeBrowserPolicyConnector created, which added a bunch of providers from constructor. 2. BrowserPolicyConnectorBase::GetPolicyService is called, which creates the service and pushes the providers to the PolicyServiceImpl 3. local state/field-trials created. 4. BrowserPolicyConnector::InitInternal was called, which in turn called Init() on the providers. With this patch, order is now: 1. ChromeBrowserPolicyConnector constructor does very little 2. ChromeBrowserPolicyConnector::InitPolicyProviders is called, which creates the providers. 3. BrowserPolicyConnectorBase::SetPolicyProviders is called, which creates the PolicyService and calls Init() on all the providers and sets is_initialized_. 4. BrowserPolicyConnectorBase::GetPolicyService is called, creating the PolicyService. 5. local state/field-trials created. 6. BrowserPolicyConnector::InitInternal is called. Key differences: . ConfigurationPolicyProvider::Init() is called earlier. . Had to remove DCHECK in BrowserPolicyConnector::InitInternal() because SetProviders sets is_initialized_. BUG= 806538 TEST=none Change-Id: I0981419cb1734208789639cc1713e49ac8e901c7 Reviewed-on: https://chromium-review.googlesource.com/889500 Commit-Queue: Scott Violet <sky@chromium.org> Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#532253}(cherry picked from commit 615e458c5a11aa5c351c31875b91b0adf80d8577) Reviewed-on: https://chromium-review.googlesource.com/891563 Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#152} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/4e6e8f4bdc7b84c018386185282efa5be4029c02/chrome/browser/browser_process_impl.cc [modify] https://crrev.com/4e6e8f4bdc7b84c018386185282efa5be4029c02/chrome/browser/policy/chrome_browser_policy_connector.cc [modify] https://crrev.com/4e6e8f4bdc7b84c018386185282efa5be4029c02/chrome/browser/policy/chrome_browser_policy_connector.h [modify] https://crrev.com/4e6e8f4bdc7b84c018386185282efa5be4029c02/components/policy/core/browser/browser_policy_connector.cc
,
Jan 30 2018
Issue 807367 has been merged into this issue.
,
Jan 30 2018
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42947e87ce8c115820aa91d7915dc70ee2821721 commit 42947e87ce8c115820aa91d7915dc70ee2821721 Author: Owen Min <zmin@chromium.org> Date: Tue Jan 30 23:36:45 2018 Add a browser test to verify the policy value in prefs. This test verifies the policy value is read and copied to local_state properly in the early stage of browser launch on Windows. It's a test for the patch: crrev.com/615e458c5a11aa5c351c31875b91b0adf80d8577. Bug: 806538 Change-Id: I5b150f7429f968312692123d5d40525c3ddbd9c6 Reviewed-on: https://chromium-review.googlesource.com/892043 Commit-Queue: Owen Min <zmin@chromium.org> Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Cr-Commit-Position: refs/heads/master@{#533083} [add] https://crrev.com/42947e87ce8c115820aa91d7915dc70ee2821721/chrome/browser/policy/policy_initialization_browsertest.cc [modify] https://crrev.com/42947e87ce8c115820aa91d7915dc70ee2821721/chrome/test/BUILD.gn
,
Apr 3 2018
Verified no issue as tested in ChromeOS M67.0.3383.0 10539.0.0 dev paine. Need to be verified in other platforms.
,
Apr 3 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Jan 27 2018