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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 53923004: policy: Make UserNetworkConfigurationUpdater a BCKS. (Closed)

Created:
7 years, 1 month ago by pneubeck (no reviews)
Modified:
7 years, 1 month ago
CC:
chromium-reviews, dbeam+watch-options_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, oshima+watch_chromium.org, nkostylev+watch_chromium.org
Visibility:
Public.

Description

policy: Make UserNetworkConfigurationUpdater a BCKS. UserNetworkConfigurationUpdater is responsible to push network user policies to different backends (PolicyCertVerifier and ManagedNetworkConfigurationHandler). ProfilePolicyConnector is responsible to provide policies for the profile's PrefService. The tight coupling of these two classes is resolved with this commit. Depends on: https://codereview.chromium.org/54483004/ BUG=312660 R=dbeam@chromium.org, joaodasilva@chromium.org, willchan@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233559

Patch Set 1 : #

Patch Set 2 : Rebased. #

Total comments: 6

Patch Set 3 : Rebased. #

Patch Set 4 : Addressed comments. #

Patch Set 5 : Rebased. #

Total comments: 2

Patch Set 6 : Removed forward declaration. #

Patch Set 7 : Rebased. #

Patch Set 8 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+174 lines, -61 lines) Patch
M chrome/browser/chromeos/policy/user_network_configuration_updater.h View 2 chunks +3 lines, -1 line 0 comments Download
A chrome/browser/chromeos/policy/user_network_configuration_updater_factory.h View 1 2 3 1 chunk +51 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc View 1 2 3 1 chunk +103 lines, -0 lines 0 comments Download
M chrome/browser/policy/profile_policy_connector.h View 1 2 3 4 5 5 chunks +0 lines, -21 lines 0 comments Download
M chrome/browser/policy/profile_policy_connector.cc View 1 2 5 chunks +0 lines, -33 lines 0 comments Download
M chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc View 1 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/profiles/profile_io_data.cc View 1 2 3 4 2 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/options/certificate_manager_handler.cc View 1 2 3 4 2 chunks +7 lines, -5 lines 0 comments Download
M chrome/chrome_browser_chromeos.gypi View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
pneubeck (no reviews)
PTAL
7 years, 1 month ago (2013-11-04 08:43:44 UTC) #1
Joao da Silva
lgtm with nits https://codereview.chromium.org/53923004/diff/350001/chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc File chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc (right): https://codereview.chromium.org/53923004/diff/350001/chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc#newcode70 chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc:70: return NULL; Document why this is ...
7 years, 1 month ago (2013-11-04 10:19:57 UTC) #2
pneubeck (no reviews)
https://codereview.chromium.org/53923004/diff/350001/chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc File chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc (right): https://codereview.chromium.org/53923004/diff/350001/chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc#newcode70 chrome/browser/chromeos/policy/user_network_configuration_updater_factory.cc:70: return NULL; On 2013/11/04 10:19:57, Joao da Silva wrote: ...
7 years, 1 month ago (2013-11-04 14:15:30 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pneubeck@chromium.org/53923004/540002
7 years, 1 month ago (2013-11-05 15:12:52 UTC) #4
pneubeck (no reviews)
@William, please take a look at profiles/ . Note, that the PolicyCertVerifier creation code in ...
7 years, 1 month ago (2013-11-05 15:31:04 UTC) #5
Dan Beam
lgtm
7 years, 1 month ago (2013-11-05 20:44:19 UTC) #6
willchan no longer on Chromium
LGTM https://codereview.chromium.org/53923004/diff/740001/chrome/browser/policy/profile_policy_connector.h File chrome/browser/policy/profile_policy_connector.h (right): https://codereview.chromium.org/53923004/diff/740001/chrome/browser/policy/profile_policy_connector.h#newcode32 chrome/browser/policy/profile_policy_connector.h:32: class PolicyCertVerifier; Delete?
7 years, 1 month ago (2013-11-06 18:01:49 UTC) #7
pneubeck (no reviews)
https://codereview.chromium.org/53923004/diff/740001/chrome/browser/policy/profile_policy_connector.h File chrome/browser/policy/profile_policy_connector.h (right): https://codereview.chromium.org/53923004/diff/740001/chrome/browser/policy/profile_policy_connector.h#newcode32 chrome/browser/policy/profile_policy_connector.h:32: class PolicyCertVerifier; On 2013/11/06 18:01:50, willchan wrote: > Delete? ...
7 years, 1 month ago (2013-11-06 20:07:13 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pneubeck@chromium.org/53923004/890001
7 years, 1 month ago (2013-11-06 20:07:59 UTC) #9
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 1 month ago (2013-11-06 23:29:11 UTC) #10
pneubeck (no reviews)
7 years, 1 month ago (2013-11-07 08:51:36 UTC) #11
Message was sent while issue was closed. 
Committed patchset #8 manually as r233559 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698