New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 446937 link

Starred by 6 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 433840

Blocking:
issue 705713



Sign in to add a comment

Make CrosSettings available outside src/chrome

Project Member Reported by steve...@chromium.org, Jan 7 2015

Issue description

We should move CrosSettings to src/chromeos/settings where it can be accessed by code in src/ash and src/ui without the use of delegates.

CrosSettings has the following src/chrome dependencies:

DeviceSettingsProvider
* This isn't a true dependency, the use in the AddSettingsProvider() calls could (and probably should) be removed from the constructor

DeviceSettingService
* This could be an interface defined in src/chromeos/settings, and the existing class renamed to DeviceSettingServiceImpl. CrosSettings should own DeviceSettingService with calls to DeviceSettingService::Get() replaced with CrosSettings::Get()->device_settings_servce()

SystemSettingsProvider
* This has no src/chrome dependencies and should be moved to src/chromeos


 
Actually, the DeviceSettingService dependency is also unnecessary when DeviceSettingsProvider is added outside of the CrosSettings constructor.

Status: Started
Blocking: chromium:447001
Cc: satorux@chromium.org
Moving makes sense, but I don't agree just moving CrosSettings is a good idea, cause that is essentially just creating another delegate interface while leaving the main device settings code within chrome/ for no good reasons.

After skimming the list of current chrome/ dependencies I concluded that neither of DeviceSettingsService, SessionManagerOperation, DeviceSettingsProvider have any good reason to depend on stuff in chrome/. The proper solution here should be to fix up stray chrome dependencies and then move all of the device settings stuff to chromeos/ instead of splitting things across source tree locations.

Cc: steve...@chromium.org
Owner: mnissler@chromium.org
Status: Assigned
I agree that DeviceSettingsService/Provider should be moved also, but I am not familiar enough with that code to feel comfortable doing that re-factoring myself. I was hoping that we would be OK with an incremental step so we could avoid introducing yet more delegate methods in https://codereview.chromium.org/811033002/, but we can live with the addition and clean it up later.

I am going to hand this off to you to delegate moving everything since it sounds like that would be preferred. I recognize that it may not happen any time soon.

Labels: Cr-Enterprise Enterprise-Triaged
Owner: ----
Status: Available
I don't have cycles right now, so releasing to the pool of open enterprise bugs - happy to provide guidance to somebody interested picking this up.
Project Member

Comment 8 by sheriffbot@chromium.org, Mar 16 2016

Labels: Hotlist-Recharge-Cold
This issue has been available for more than 365 days, and should be re-evaluated. Hotlist-Recharge-Cold label is added for tracking. Please re-triage this issue.

For more details visit https://sites.google.com/a/chromium.org/dev/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: steve...@chromium.org
Labels: -Hotlist-Recharge-Cold
Still something we should do.

Blocking: 628792
Cc: st...@chromium.org
Labels: -Enterprise-triaged Enterprise-Triaged
This would be useful for mustash. We would still have to ensure that CrosSettings either:

1. Can have separate instances in the chrome and ash processes, or
2. Have ash own the instance and have chrome communicate to it via mojo

Labels: Proj-Mustash-Chrome
The current CrosSettings code should allow multiple instances in separate processes, with setting changes propagating via session_manager (at least for device settings, the time zone setting might be more difficult). The code to support this already exists, but given it has not been exercised much, there might be hidden bugs :)

Comment 15 by e...@chromium.org, Nov 1 2016

Moving CrosSettings out of c/b/chromeos/settings/ might be a challenge.

CrosSettings itself depends on DeviceSettingsProvider, which brings in a bunch
of dependencies.

Starting from DeviceSettingsProvider.

- The policy stuff that DeviceSettingsProvider depends on in turn depends on ownership and policy stuff. DeviceLocalAccount depends back on CrosSettings.

- The ownership stuff that DeviceSettingsProvider (and DeviceLocalAccount) depend on in turn depends on CrosSettings, policy protobufs, content notifications, and chrome Profile. (I'm unsure if the Profile pointer is actually being used as anything other than a primary key; either way it's using the profile content notifications.)

- The BrowserPolicyConnectorChromeos, which DeviceSettingsProvider depends on, in turn depends on various other pieces of c/b/c/settings/, most of c/b/c/policy/, content notifications, and chrome Profile.

- Several of the above things depend on c/b/browser_process.h for reasons I don't fully get. They're using different things that hang off it.

It seems like this has some nontrivial entanglements with chrome proper.
Some comments on the dependencies brought up in comment 15:

- DeviceSettingsProvider needs to move as well, as should DeviceSettingsService and SessionManagerOperation.

- DeviceSettingsProvider shouldn't depend on any policy stuff. If it does, that's arguably a layering violation as the dependency direction is policy -> DeviceSettignsProvider.

- Ownership stuff is only required for the write path of device settings. There was an effort to separate the write path out of CrosSettings and DeviceSettingsService, but the person working on this disappeared on short notice. I think it's fine for the ownership stuff to remain within Chrome, as long as that's the only place where device settings get written (not sure whether this is compatible with longer-term plans?)

- The BrowserPolicyConnectorChromeOS dependency should really not be present in DeviceSettingsProvider (per my earlier layering violation point). The pieces that rely on it should be changed or moved out. I agree this is non-trivial surgery.

- One other dependency that I didn't have on my radar is the local_state PrefService. DeviceSettingsProvider stores a cached version of the device settings there, which it uses to satisfy requests that happen early while still waiting for IPC to session_manager to complete to obtain the source of truth for device settings.

Splitting CrosSettings out indeed looks a bit more involved than I had hoped purely from a conceptual perspective :-/
Cc: r...@chromium.org
Cc: -st...@chromium.org
Cc: jamescook@chromium.org
Labels: Proj-Mustash-Mash
Status: Started (was: Available)
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5e2e45ccac32440d413e8212e45c03bf6d56830

commit b5e2e45ccac32440d413e8212e45c03bf6d56830
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Mon Dec 18 18:57:25 2017

Elim NOTIFICATION_OWNERSHIP_STATUS_CHANGED

DeviceSettingsServce is initialized in DBusServices() which is
constructed in PostMainMessageLoopStart().

LocaleChangeGuard is owned by ProfileImpl which is constructed
after PostMainMessageLoopStart.

ChromeUserManagerImpl is constructed in PreProfileInit(), also
called after PostMainMessageLoopStart.

Bug: 446937
Change-Id: I2cf1c226a68306d40d047d60d6858fe0abd00e9d
Reviewed-on: https://chromium-review.googlesource.com/830656
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524755}
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chrome_notification_types.h
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chromeos/locale_change_guard.cc
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chromeos/locale_change_guard.h
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chromeos/login/users/chrome_user_manager_impl.h
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chromeos/settings/device_settings_service.cc
[modify] https://crrev.com/b5e2e45ccac32440d413e8212e45c03bf6d56830/chrome/browser/chromeos/settings/device_settings_service.h

Status: Assigned (was: Started)
The primary issue with migrating CroSettings out of src/chrome at this point is migrating the policy code. This is going to be a requirement for Mash at some point regardless so I think we should at least design a plan for that before moving forward with this effort.

Components: Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Blocking: -447001 -628792
Owner: atwilson@chromium.org
Summary: Make CrosSettings available outside src/chrome (was: Move CrosSettings (cros_settings.cc) to src/chromeos/settings)
Renaming / repurposing and assigning to atwilson@.

Feel free to close this and open a new issue instead.


SystemSettingsProvider is only used for time zone. It reports "trusted" status immediately. I'm not sure why it's built into CrosSettings. One possible first step is to refactor out the timezone stuff into Local State or some other generic pref store.

Cc: atwilson@chromium.org olsen@chromium.org
Owner: ljusten@chromium.org
Lutz, this is a tracking bug for the "Move CrosSettings to a service" effort.
Blocking: 705713
Labels: -Pri-2 Pri-1
Owner: olsen@chromium.org
Status: Started (was: Assigned)
I have started looking into this.

https://docs.google.com/drawings/d/19CxLJf8n3Q0M0Ak2pqLLPgT1N72RYVisnrlHfQ31sX0/edit shows a graph of dependencies from cros_settings.cc

I think to make any progress, I will have to break off smaller pieces and move them one by one. A good place to start seems to be SystemSettingsProvider - it has few dependencies in chrome/browser, and there seems to be some certainty in the discussion above that it is one of the things that should be moved.

Unfortunately it no longer has no dependencies in chrome/browser (this bug has gotten less fixed) - it now depends on "chrome/browser/chromeos/system/timezone_util.h". I will look into this more - find out what services this provides, why they are needed and why they are not provided by "chromeos/settings/timezone_settings.h"
Cc: ljusten@chromium.org
I added a comment to the doc, but see crrev.com/830620 for some discussion around an effort to refactor DeviceOffHoursController and DeviceSettingsService.

I haven't looked at this in a while, but I seem to recall that ultimately we are going to need to move some or all of the code in c/b/chroemos/policy if we want to make real headway on this effort.

Cc: alemate@chromium.org
+alemate

Per comment #24, it might be possible to eliminate SystemSettingsProvider entirely. It's not clear to me why it's part of CrosSettings, since it doesn't use the "trusted" state. It seems like it could just be some prefs. Then the entire "provider" mechanism could be eliminated from CrosSettings, since there would only be one provider.

Dunno if that would help with dependencies, but it might simplify the code.

More dependency resources:

- Various dependency graphs: https://docs.google.com/presentation/d/1vdiTfe6iqVeJK7ngtakD0veLH4MLsDU6VSSJCVHTGGA/edit

- Code for generating many dependency graphs:
https://docs.google.com/document/d/1wxZ1YE1nypz71pftHU8SbecGbYo-8rLInqG2CHemx4M/edit

- Dependencies as a document - better for commenting on. Could be the start of a design doc, except, will be hard to make a design that accounts for all these dependencies:
https://docs.google.com/document/d/1qxt0VmOCnZZmp55LkSITJH1XW8z4Z2tK2RBb9CscojI/edit#
FYI - I have a hacky demonstration CL with one possible impl of the design in comment 34: https://chromium-review.googlesource.com/c/chromium/src/+/1042880 - it doesn't directly address CrosSettings, but does show how ash could own more prefs.

See also design sketch at go/ash-settings

My first actual design doc tracking my attempts so far:

https://docs.google.com/document/d/1Zon4KPTvUARbvB66NFlyAyV-WDxo2o5b5QyM17uX06Y/edit

Another graph of dependencies - probably most useful one so far:

https://docs.google.com/drawings/d/1Gm7F8KQfsgpFIFSfhBh9-g4niGmER100rl6-gBeCdhc/edit


Comment 37 Deleted

So, I have found (I hope) a more useful place to start -

- device_settings_provider has a dependency on browser_policy_connector_chromeos, which is a concrete implementation of browser_policy_connector.

It only uses two things members from this class: the InstallAttributes and a Schema known as the "chrome schema".

This is a layering violation - there would be far fewer dependencies if a ChromeOS class (DeviceSettingsProvider) depended directly on the other ChromeOS class (InstallAttributes) without using either the Chrome browser global-store (BrowserProcess) or the Chrome based connector class (BrowserPolicyConnector) to find it.

I think tidying this up will help no matter where this code ends up - will write a little design.
Wrote a brief design for a clean up based on untangling BrowserPolicyConnectorChromeOS.

https://docs.google.com/document/d/1_HG01QFuqwFwcIJaKbQuYXO7ABgR1g8z5c5xQ7xw6h4/edit#
Project Member

Comment 40 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c63621c586998067fb95cf1e83b5608c464fd31f

commit c63621c586998067fb95cf1e83b5608c464fd31f
Author: A Olsen <olsen@chromium.org>
Date: Mon Aug 13 15:32:17 2018

Remove chrome/browser dep from Chrome OS settings

InstallAttributes and the "Chrome" Schema both have few deps and are
needed in Chrome OS settings code, in code that should be servicified.
See go/ash-settings-split. However, getting a useful instance of
either class currently means depending on chrome/browser to get
browser_process and all of its global services.

This CL makes InstallAttributes and the Chrome Schema singletons that
can also be obtained directly, and so removes one of the dependencies
on chrome/browser from the Chrome OS settings code.

Bug: 446937
Change-Id: If8b1d8e4e9851464278d4b3e25562bc0eea8a41c
Reviewed-on: https://chromium-review.googlesource.com/1141947
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582585}
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/background/background_mode_manager_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/arc/arc_util_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/attestation/enrollment_policy_observer_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/extensions/device_local_account_external_policy_loader_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/extensions/external_cache_impl_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/extensions/info_private_apitest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/extensions/wallpaper_private_api_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/file_manager/file_tasks_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/lock_screen_apps/app_manager_impl_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/lock_screen_apps/lock_screen_profile_creator_impl_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/auto_launched_kiosk_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/demo_mode/demo_session_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/demo_mode/demo_setup_controller_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/easy_unlock/easy_unlock_app_manager_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/enrollment/enrollment_screen_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/users/chrome_user_manager_util.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/users/wallpaper_policy_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/login/wizard_controller_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/component_active_directory_policy_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/policy/site_isolation_flag_handling_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/power/extension_event_observer_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/power/renderer_freezer_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/cros_settings_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_settings_provider.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_settings_provider.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_settings_service_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_settings_test_helper.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/device_settings_test_helper.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/install_attributes.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/install_attributes.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/stub_install_attributes.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/settings/stub_install_attributes.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/system/device_disabling_manager_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/chromeos/tpm_firmware_update_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/activity_log/counting_policy_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/activity_log/fullstream_ui_policy_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_api_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/api/identity/identity_apitest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/extension_action_icon_factory_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/extension_service_test_base.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/extension_web_ui_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/test_extension_environment.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/updater/extension_updater_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/extensions/zipfile_installer_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/media/webrtc/tab_desktop_media_list_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/media_galleries/gallery_watch_manager_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/media_galleries/media_file_system_registry_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/media_galleries/media_galleries_permission_controller_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/media_galleries/media_galleries_preferences_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/policy/profile_policy_connector_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/prefs/proxy_policy_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/profiles/profile_manager_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/safe_browsing/incident_reporting/extension_data_collection_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/themes/theme_syncable_service_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/ui/ash/session_controller_client_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/ui/ash/wallpaper_controller_client_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/ui/sync/profile_signin_confirmation_helper_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/ui/webui/chromeos/login/signin_userlist_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/browser/ui/webui/help/version_updater_chromeos_unittest.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/test/base/browser_with_test_window_test.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/chrome/test/base/testing_profile.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/components/policy/core/browser/browser_policy_connector_base.cc
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/components/policy/core/browser/browser_policy_connector_base.h
[modify] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/components/policy/core/common/BUILD.gn
[add] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/components/policy/core/common/chrome_schema.cc
[add] https://crrev.com/c63621c586998067fb95cf1e83b5608c464fd31f/components/policy/core/common/chrome_schema.h

Project Member

Comment 41 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81bcbe408b0b06d4f534f572a0eaeb4b22f245ca

commit 81bcbe408b0b06d4f534f572a0eaeb4b22f245ca
Author: A Olsen <olsen@chromium.org>
Date: Thu Aug 16 14:11:24 2018

Remove some deps on g_browser_process from chromeos code

go/cros-untangle shows that one reason why Chrome OS code depends on
all kinds of Chrome internals, is because it uses g_browser_process
to get hold of various Chroms OS globals, and g_browser_process
keeps a reference to all globals, Chrome and Chrome OS.

This CL removes a few easy-to-remove references to g_browser_process
from chrome/browser/chromeos.

Bug: 446937
Change-Id: I7ed4d1ccede3d0963ab86c80bba6f9327dd0cf61
Reviewed-on: https://chromium-review.googlesource.com/1174448
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583638}
[modify] https://crrev.com/81bcbe408b0b06d4f534f572a0eaeb4b22f245ca/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
[modify] https://crrev.com/81bcbe408b0b06d4f534f572a0eaeb4b22f245ca/chrome/browser/chromeos/policy/blocking_login_browsertest.cc
[modify] https://crrev.com/81bcbe408b0b06d4f534f572a0eaeb4b22f245ca/chrome/browser/chromeos/policy/device_local_account_policy_service.cc
[modify] https://crrev.com/81bcbe408b0b06d4f534f572a0eaeb4b22f245ca/chrome/browser/chromeos/policy/remote_commands/device_command_start_crd_session_unittest.cc
[modify] https://crrev.com/81bcbe408b0b06d4f534f572a0eaeb4b22f245ca/chrome/browser/chromeos/settings/device_settings_test_helper.cc

Project Member

Comment 42 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13708db41bb8f002cc9f292ed489202eeca03ddb

commit 13708db41bb8f002cc9f292ed489202eeca03ddb
Author: A Olsen <olsen@chromium.org>
Date: Wed Sep 19 10:09:37 2018

DeviceOAuth2TokenServiceFactory ⇏ BrowserProcess

DeviceOAuth2TokenServiceFactory has an undesirable dependency on
g_browser_process. It uses it to obtain LocalState and
SharedURLLoaderFactory. This CL removes the dependency on
g_browser_process by passing in these dependencies explicitly.

Note that the tests still depend on TestingBrowserProcess -
this dependency will also be removed in a follow up CL
(for tests in the settings package).

See go/cros-untangle2

Bug: 446937
Change-Id: I9a046b04175bef6b5af0945d03b03fe1e777fb90
Reviewed-on: https://chromium-review.googlesource.com/1228153
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592358}
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl_unittest.cc
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.cc
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h
[modify] https://crrev.com/13708db41bb8f002cc9f292ed489202eeca03ddb/chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc

Project Member

Comment 43 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b16a5e5369333aba4966cf3be5272af25d66f3dd

commit b16a5e5369333aba4966cf3be5272af25d66f3dd
Author: A Olsen <olsen@chromium.org>
Date: Fri Sep 21 15:48:46 2018

Remove BrowserProcess dep in c/b/chromeos/settings

DeviceSettingsProvider in chrome/browser/chromeos/settings has an
undesirable dependency on g_browser_process, which it uses to access
the LocalState singleton. This CL removes the dependency on
g_browser_process by passing in LocalState explicitly.

Note that the tests and test helpers still depend on BrowserProcess
or TestingBrowserProcess, which they use to initialize and access the
LocalState. For tests in the settings package, I need to find a way to
remove this dependency in a follow up CL, so that the settings package
along with tests & test helpers can be moved out of chrome/browser.

See go/cros-untangle2

Bug: 446937
Change-Id: If9b14940f05ceef1fa8f33e612df22d3d9173d8f
Reviewed-on: https://chromium-review.googlesource.com/1233711
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: A Olsen <olsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593199}
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/arc/arc_util_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/login/existing_user_controller_auto_login_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/ownership/owner_settings_service_chromeos_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/policy/device_status_collector_browsertest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/cros_settings.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/cros_settings.h
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/cros_settings_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/device_settings_provider.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/device_settings_provider.h
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/owner_flags_storage.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.cc
[modify] https://crrev.com/b16a5e5369333aba4966cf3be5272af25d66f3dd/chrome/browser/ui/ash/multi_user/multi_user_window_manager_chromeos_unittest.cc

Project Member

Comment 44 by bugdroid1@chromium.org, Sep 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28dfa07120d26f09332034528a0a2cca237774f4

commit 28dfa07120d26f09332034528a0a2cca237774f4
Author: A Olsen <olsen@chromium.org>
Date: Wed Sep 26 15:54:22 2018

Remove more chrome/browser deps

Removing chrome/browser deps from c/b/chromeos/settings code that, so
that the code can eventually be moved to src/chromeos/settings:

- Delete some chrome/browser includes that are now completely unused in
chrome/browser/chromeos/settings.

- Change SystemSettingsProvider to use UserManager, instead of
ProfileManager and LoginState, since UserManager does not depend on
chrome/browser

Bug: 446937
Change-Id: I5d1edb34643d6c48a8b8351af68ec9ae5c0118b4
Reviewed-on: https://chromium-review.googlesource.com/1238450
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594330}
[modify] https://crrev.com/28dfa07120d26f09332034528a0a2cca237774f4/chrome/browser/chromeos/settings/device_oauth2_token_service_factory.cc
[modify] https://crrev.com/28dfa07120d26f09332034528a0a2cca237774f4/chrome/browser/chromeos/settings/device_settings_provider.cc
[modify] https://crrev.com/28dfa07120d26f09332034528a0a2cca237774f4/chrome/browser/chromeos/settings/session_manager_operation.cc
[modify] https://crrev.com/28dfa07120d26f09332034528a0a2cca237774f4/chrome/browser/chromeos/settings/system_settings_provider.cc

Project Member

Comment 45 by bugdroid1@chromium.org, Sep 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec3aded00c036c3f142a194c42d5df8108cc1070

commit ec3aded00c036c3f142a194c42d5df8108cc1070
Author: A Olsen <olsen@chromium.org>
Date: Thu Sep 27 11:10:03 2018

Remove most of chrome_user_manager_util

chrome_user_manager_util is a static helper class that mostly helps
implement ChromeUserManagerImpl and FakeChromeUserManager. A few other
classes also call it directly. The different functions in it make use
of four different global singletons -

1. chromeos::LoginState::Get()
2. user_manager::UserManager::Get()
3. g_browser_process->platform_part()
        ->browser_policy_connector_chromeos()
        ->GetMinimumVersionPolicyHandler()
4. g_browser_process->platform_part()
          ->browser_policy_connector_chromeos()
          ->GetDeviceLocalAccountPolicyService()

This changes moves most of the code out of chrome_user_manager_util,
and makes sure that the remaining code doesn't depend on these
singletons. This is done as follows:

1. Code common to ChromeUserManagerImpl and FakeChromeUserManager now
goes in ChromeUserManager.
2. Code only used by ChromeUserManagerImpl is now there.
3. Direct calls to chrome_user_manager_util by GaiaScreenHandler or
SigninScreenHandler are now replaced by the equivalent calls to the
ChromeUserManager singleton
4. The only code which remains is the completely static function
IsUserAllowed - only contains logic, not state, since all policy data
is passed in - and a variant which takes the policy data as a
ChromeDeviceSettingsProto. (This variant is still called directly
from DeviceOffHoursController.)

The previous design - static functions with dependencies on global
singletons - had some disadvantages, which this CL tries to address:

1. Unwanted dependencies on g_browser_process in chromeos code -
removing these is part of http://crbug.com/446937
2. Static cling - go/staticcling - it is harder to setup tests for
static utilities if they depend on global singletons.
3. Strange dependency cycles. When calling IsUserAllowed on a
ChromeUserManager, it delegated to chrome_user_manager_util, which
calls user_manager::UserManager::Get(), which could be a different
ChromeUserManager, or not be initialized. This also makes tests harder
to set up. Now, all the code is part of the one ChromeUserManager
instance.

Bug: 446937
Change-Id: Ifb6c7aa5bf7081b4c6dd9f5f4043a07d51c79a96
Reviewed-on: https://chromium-review.googlesource.com/1243074
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594666}
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/chrome_user_manager.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/chrome_user_manager.h
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/chrome_user_manager_impl.h
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/chrome_user_manager_util.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/chrome_user_manager_util.h
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/login/users/fake_chrome_user_manager.h
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/policy/minimum_version_policy_handler.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/policy/minimum_version_policy_handler.h
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc
[modify] https://crrev.com/ec3aded00c036c3f142a194c42d5df8108cc1070/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

Blockedon: 433840
Project Member

Comment 47 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a35a7fe2bea84fe913ea585ba642357279e0a5cf

commit a35a7fe2bea84fe913ea585ba642357279e0a5cf
Author: A Olsen <olsen@chromium.org>
Date: Tue Oct 02 09:41:10 2018

SystemSettingsProvider dep back to LoginState

SystemSettingProvider had a dep on both LoginState and ProfileManager,
in order to check if the user had permission to change the timezone.
ProfileManager is in chrome/browser and I am trying to remove
chrome/browser deps, so I changed the code to use only UserManager.
See https://chromium-review.googlesource.com/1238450

I realized afterwards that UserManager deps are also currently not
allowed in src/chromeos, where this code will soon live. Rather than
expand the list of deps (which should be kept small), I have updated
LoginState so that only it is needed here (instead of ProfileManager or
UserManager). So, the last change is no longer needed, and is reverted.

I also tidied up conversion code in ChromeUserManager slightly -
Moving if-else-else chain into a helper function with switch + return
means that the compiler checks that all cases are handled.

Bug: 446937
Change-Id: I3aa44cacdef752f997d12b647d6e073f806371d3
Reviewed-on: https://chromium-review.googlesource.com/1249209
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: A Olsen <olsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595777}
[modify] https://crrev.com/a35a7fe2bea84fe913ea585ba642357279e0a5cf/chrome/browser/chromeos/login/users/chrome_user_manager.cc
[modify] https://crrev.com/a35a7fe2bea84fe913ea585ba642357279e0a5cf/chrome/browser/chromeos/settings/system_settings_provider.cc
[modify] https://crrev.com/a35a7fe2bea84fe913ea585ba642357279e0a5cf/chrome/browser/ui/ash/network/enrollment_dialog_view.cc
[modify] https://crrev.com/a35a7fe2bea84fe913ea585ba642357279e0a5cf/chromeos/login/login_state.cc
[modify] https://crrev.com/a35a7fe2bea84fe913ea585ba642357279e0a5cf/chromeos/login/login_state.h

Project Member

Comment 48 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d994a4c1638021a38c7f580911efc82b70d995a4

commit d994a4c1638021a38c7f580911efc82b70d995a4
Author: A Olsen <olsen@chromium.org>
Date: Tue Oct 02 11:35:30 2018

c/b/cros/policy/weeklytime -> cros/policy/weeklytime

Weeklytime package has no dependencies of its own, and few things
depend on it, so it is relatively easy to move out of
src/chrome/browser and into src/chromeos.

This is useful as part of bug 446937 - moving CrosSettings out of
chrome/browser - since CrosSettings depends on this class for the
DeviceOffHoursPolicyController.

Bug: 446937
Change-Id: I9866f341a15f67b452b57cc219b5ca88cd18cfdb
Reviewed-on: https://chromium-review.googlesource.com/1254661
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595797}
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_decoder.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_decoder_unittest.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/device_auto_update_time_restrictions_utils_unittest.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.h
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/off_hours/off_hours_policy_applier_unittest.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser_unittest.cc
[modify] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/BUILD.gn
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/time_utils.cc
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/time_utils.h
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/time_utils_unittest.cc
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/weekly_time.cc
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/weekly_time.h
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/weekly_time_interval.cc
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/weekly_time_interval.h
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/weekly_time_interval_unittest.cc
[rename] https://crrev.com/d994a4c1638021a38c7f580911efc82b70d995a4/chromeos/policy/weekly_time/weekly_time_unittest.cc

Project Member

Comment 49 by bugdroid1@chromium.org, Oct 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99667348706d3f3e98e3c857e45afe0f5d9f1aca

commit 99667348706d3f3e98e3c857e45afe0f5d9f1aca
Author: A Olsen <olsen@chromium.org>
Date: Thu Oct 04 09:45:13 2018

Move system_settings_provider into chromeos/system

Moves system_settings_provider from chrome/browser/chromeos/system to
chromeos/system - part of an effort to move cros_settings and related
settings code out of chrome/browser and into chromeos/system, so that
they can be used in processes other than the chrome process.

Duplicates the timezone_util functions that system_settings_provider
depends on - ideally, all of timezone_util would move too but since
it depends on cros_settings, it might have to be moved last.

Bug: 446937
Change-Id: I3863cd3dd274b0f94dc4d87938b7ccda2fe89269
Reviewed-on: https://chromium-review.googlesource.com/c/1128861
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Commit-Queue: A Olsen <olsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596559}
[modify] https://crrev.com/99667348706d3f3e98e3c857e45afe0f5d9f1aca/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/99667348706d3f3e98e3c857e45afe0f5d9f1aca/chrome/browser/chromeos/settings/cros_settings.cc
[modify] https://crrev.com/99667348706d3f3e98e3c857e45afe0f5d9f1aca/chromeos/BUILD.gn
[rename] https://crrev.com/99667348706d3f3e98e3c857e45afe0f5d9f1aca/chromeos/settings/system_settings_provider.cc
[rename] https://crrev.com/99667348706d3f3e98e3c857e45afe0f5d9f1aca/chromeos/settings/system_settings_provider.h

Project Member

Comment 50 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3a2e065e870cc3097b224e33040a162f3edaab4

commit c3a2e065e870cc3097b224e33040a162f3edaab4
Author: A Olsen <olsen@chromium.org>
Date: Fri Oct 05 14:51:40 2018

DeviceSettingsProvider →🚫 DeviceLocalAccount

Removes DeviceSettingsProvider dep on DeviceLocalAccount, since
the settings package is to be moved into src/chromeos/settings,
so the less deps it can have, the easier it will be to move.

This change relies on the fact that there are two identical
enums that are kept in sync - a proto one, and a native C++ one.
The code already has a dependency on the proto, so we can easily
switch to using that enum to avoid depending on DeviceLocalAccount.

Bug: 446937
Change-Id: I1bddb3d7f35b208540f6f02fa027bc11c46d0903
Reviewed-on: https://chromium-review.googlesource.com/c/1264582
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: A Olsen <olsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597116}
[modify] https://crrev.com/c3a2e065e870cc3097b224e33040a162f3edaab4/chrome/browser/chromeos/settings/device_settings_provider.cc

Project Member

Comment 51 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b18174595a0b32d19f7c541915b510dc7116026

commit 0b18174595a0b32d19f7c541915b510dc7116026
Author: A Olsen <olsen@chromium.org>
Date: Fri Oct 05 20:05:18 2018

Move OWNERS to src/chromeos/settings

src/chrome/browser/chromeos/settings OWNERS should also own
src/chromeos/settings, so now they do.

Moved the OWNERS file to chromeos/settings, and
c/b/chromeos/settings OWNERS inherits from there.

Bug: 446937
Change-Id: I72202017fa2c0c6fd3f4a322b8ea38dfb3bc6c04
Reviewed-on: https://chromium-review.googlesource.com/c/1264584
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597262}
[modify] https://crrev.com/0b18174595a0b32d19f7c541915b510dc7116026/chrome/browser/chromeos/settings/OWNERS
[add] https://crrev.com/0b18174595a0b32d19f7c541915b510dc7116026/chromeos/settings/OWNERS

Project Member

Comment 52 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23537344c6cf3f376fb9e8d87cccd085aff80493

commit 23537344c6cf3f376fb9e8d87cccd085aff80493
Author: A Olsen <olsen@chromium.org>
Date: Wed Oct 10 13:23:21 2018

Separate out source sets for src/chromeos subdirs

Its good to break huge packages into smaller packages,
especially if they are already separate directories.

This breaks a number of packages out of the chromeos monolith package,
starting with chromeos/settings, and then any other packages that are
necessary for the chromeos/settings package to be separated.

Bug: 446937
Change-Id: I71909b7e7afa79f4f26d1dbac5217a3f4c769b5b
Reviewed-on: https://chromium-review.googlesource.com/c/1268315
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598302}
[modify] https://crrev.com/23537344c6cf3f376fb9e8d87cccd085aff80493/chromeos/BUILD.gn
[add] https://crrev.com/23537344c6cf3f376fb9e8d87cccd085aff80493/chromeos/cryptohome/BUILD.gn
[add] https://crrev.com/23537344c6cf3f376fb9e8d87cccd085aff80493/chromeos/dbus/BUILD.gn
[add] https://crrev.com/23537344c6cf3f376fb9e8d87cccd085aff80493/chromeos/login/BUILD.gn
[add] https://crrev.com/23537344c6cf3f376fb9e8d87cccd085aff80493/chromeos/settings/BUILD.gn

Project Member

Comment 53 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c6aeb96da8745b949f1a246a19e41f6d6772f4b

commit 7c6aeb96da8745b949f1a246a19e41f6d6772f4b
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Wed Oct 10 14:22:16 2018

Revert "Separate out source sets for src/chromeos subdirs"

This reverts commit 23537344c6cf3f376fb9e8d87cccd085aff80493.

Reason for revert: broke compilation on CrOS
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8933046391246969968/+/steps/compile/0/stdout

../../services/device/fingerprint/fingerprint_chromeos.cc:12308: error: undefined reference to 'chromeos::DBusThreadManager::Get()'
  ../../services/device/fingerprint/fingerprint_chromeos.cc:12308: error: undefined reference to 'chromeos::DBusThreadManager::GetBiodClient()'
  ../../services/device/fingerprint/fingerprint_chromeos.cc:18906: error: undefined reference to 'chromeos::EmptyVoidDBusMethodCallback()'
  ../../services/device/fingerprint/fingerprint_chromeos.cc:19785: error: undefined reference to 'chromeos::EmptyVoidDBusMethodCallback()'
  ../../services/device/fingerprint/fingerprint_chromeos.cc:34261: error: undefined reference to 'chromeos::DBusThreadManager::Get()'
  ../../services/device/fingerprint/fingerprint_chromeos.cc:34261: error: undefined reference to 'chromeos::DBusThreadManager::GetBiodClient()'

Original change's description:
> Separate out source sets for src/chromeos subdirs
> 
> Its good to break huge packages into smaller packages,
> especially if they are already separate directories.
> 
> This breaks a number of packages out of the chromeos monolith package,
> starting with chromeos/settings, and then any other packages that are
> necessary for the chromeos/settings package to be separated.
> 
> Bug: 446937
> Change-Id: I71909b7e7afa79f4f26d1dbac5217a3f4c769b5b
> Reviewed-on: https://chromium-review.googlesource.com/c/1268315
> Commit-Queue: A Olsen <olsen@chromium.org>
> Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Dan Erat <derat@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598302}

TBR=derat@chromium.org,hashimoto@chromium.org,stevenjb@chromium.org,olsen@chromium.org

Change-Id: I816d9e1df68f9d5776724854d19a0ba26f00d732
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 446937
Reviewed-on: https://chromium-review.googlesource.com/c/1273520
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598310}
[modify] https://crrev.com/7c6aeb96da8745b949f1a246a19e41f6d6772f4b/chromeos/BUILD.gn
[delete] https://crrev.com/602835ba62b1491ea9a3282c2146220c0f4c1775/chromeos/cryptohome/BUILD.gn
[delete] https://crrev.com/602835ba62b1491ea9a3282c2146220c0f4c1775/chromeos/dbus/BUILD.gn
[delete] https://crrev.com/602835ba62b1491ea9a3282c2146220c0f4c1775/chromeos/login/BUILD.gn
[delete] https://crrev.com/602835ba62b1491ea9a3282c2146220c0f4c1775/chromeos/settings/BUILD.gn

Project Member

Comment 55 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa6c62dce936e966e2db445045dd04414553642b

commit aa6c62dce936e966e2db445045dd04414553642b
Author: A Olsen <olsen@chromium.org>
Date: Fri Oct 12 08:19:44 2018

Move InstallAttributes to chromeos/settings

Necessary for moving CrosSettings to chromeos/settings, so it
can eventually be used by Mustash.

The following changes were necessary:
1. Move install_attributes{.h, .cc} and unit_test.cc
2. Update c/b/chromeos/BUILD.gn (source) and chromeos/settings/BUILD.gn
(destination)
3. Update chromeos/DEPS to allow a dependency on cloud_policy_constants.h,
which is included by install_attributes.h

4. Unfortunately, that same dependency lead to a circular dependency.
I was able to avoid that by separating out a "common_constants" rule in
components/policy/core/common, so that chromeos/settings only depends on
that, and not on all of components/policy. So I had to change
components/policy/core/common/BUILD.gn
and I removed an unnecessary include from
components/policy/core/common/cloud/cloud_policy_constants.cc

5. Update #include directives for install_attributes to point to new
location. This is the other 22 files, all with diffs of +1 -1.

Bug: 446937
Change-Id: Icc8ba19c5fec91b13004b19680ead9c9f8b0a288
Reviewed-on: https://chromium-review.googlesource.com/c/1264757
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: A Olsen <olsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599140}
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/app_mode/kiosk_app_manager.h
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/arc/auth/arc_active_directory_enrollment_token_fetcher.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/dbus/dbus_helper.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/login/demo_mode/demo_session.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/blocking_login_browsertest.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos_unittest.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/device_policy_cros_browser_test.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/enrollment_handler_chromeos.h
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/enrollment_status_chromeos.h
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/settings/device_settings_provider.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/settings/stub_install_attributes.h
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/chromeos/system/device_disabling_manager.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chrome/browser/ui/webui/policy_ui_handler.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chromeos/DEPS
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chromeos/settings/BUILD.gn
[rename] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chromeos/settings/install_attributes.cc
[rename] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chromeos/settings/install_attributes.h
[rename] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/chromeos/settings/install_attributes_unittest.cc
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/components/policy/core/common/BUILD.gn
[modify] https://crrev.com/aa6c62dce936e966e2db445045dd04414553642b/components/policy/core/common/cloud/cloud_policy_constants.cc

Project Member

Comment 56 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9

commit f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9
Author: Florent Castelli <orphis@chromium.org>
Date: Fri Oct 12 08:33:39 2018

Revert "Move InstallAttributes to chromeos/settings"

This reverts commit aa6c62dce936e966e2db445045dd04414553642b.

Reason for revert: Breaks build https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/chromeos-daisy-rel/33402

Original change's description:
> Move InstallAttributes to chromeos/settings
> 
> Necessary for moving CrosSettings to chromeos/settings, so it
> can eventually be used by Mustash.
> 
> The following changes were necessary:
> 1. Move install_attributes{.h, .cc} and unit_test.cc
> 2. Update c/b/chromeos/BUILD.gn (source) and chromeos/settings/BUILD.gn
> (destination)
> 3. Update chromeos/DEPS to allow a dependency on cloud_policy_constants.h,
> which is included by install_attributes.h
> 
> 4. Unfortunately, that same dependency lead to a circular dependency.
> I was able to avoid that by separating out a "common_constants" rule in
> components/policy/core/common, so that chromeos/settings only depends on
> that, and not on all of components/policy. So I had to change
> components/policy/core/common/BUILD.gn
> and I removed an unnecessary include from
> components/policy/core/common/cloud/cloud_policy_constants.cc
> 
> 5. Update #include directives for install_attributes to point to new
> location. This is the other 22 files, all with diffs of +1 -1.
> 
> Bug: 446937
> Change-Id: Icc8ba19c5fec91b13004b19680ead9c9f8b0a288
> Reviewed-on: https://chromium-review.googlesource.com/c/1264757
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Commit-Queue: A Olsen <olsen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599140}

TBR=stevenjb@chromium.org,emaxx@chromium.org,olsen@chromium.org

Change-Id: I50c58b55830961d97834d86a76088dc50eff672d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 446937
Reviewed-on: https://chromium-review.googlesource.com/c/1278748
Reviewed-by: Florent Castelli <orphis@chromium.org>
Commit-Queue: Florent Castelli <orphis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599143}
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/app_mode/kiosk_app_manager.h
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/arc/auth/arc_active_directory_enrollment_token_fetcher.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/dbus/dbus_helper.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/login/demo_mode/demo_session.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/blocking_login_browsertest.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos_unittest.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/device_policy_cros_browser_test.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/enrollment_handler_chromeos.h
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/enrollment_status_chromeos.h
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/settings/device_settings_provider.cc
[rename] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/settings/install_attributes.cc
[rename] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/settings/install_attributes.h
[rename] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/settings/install_attributes_unittest.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/settings/stub_install_attributes.h
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/chromeos/system/device_disabling_manager.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chrome/browser/ui/webui/policy_ui_handler.cc
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chromeos/DEPS
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/chromeos/settings/BUILD.gn
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/components/policy/core/common/BUILD.gn
[modify] https://crrev.com/f32368f166f8a2d51078a6c6c2f6ffc8c3f2f7a9/components/policy/core/common/cloud/cloud_policy_constants.cc

Project Member

Comment 57 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/37962d2eeca8b5cd6d6130cfa8dff3433974202b

commit 37962d2eeca8b5cd6d6130cfa8dff3433974202b
Author: A Olsen <olsen@chromium.org>
Date: Fri Oct 12 15:05:18 2018

Split chromeos/network out of chromeos

Network is by far the largest subdirectory of chromeos.
This gives it its own BUILD.gn file.

Also added a ":chromeos_implementation" config.
Better to use a single config than a define each time, less chance for
undetected spelling errors.

Bug: 446937
Change-Id: I2f26cf3f9f15c4b4db364d7d2bc6cf5cc7e57887
Reviewed-on: https://chromium-review.googlesource.com/c/1276568
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599216}
[modify] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/chromeos/BUILD.gn
[modify] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/chromeos/cryptohome/BUILD.gn
[modify] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/chromeos/dbus/BUILD.gn
[modify] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/chromeos/login/BUILD.gn
[add] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/chromeos/network/BUILD.gn
[modify] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/chromeos/settings/BUILD.gn
[modify] https://crrev.com/37962d2eeca8b5cd6d6130cfa8dff3433974202b/net/dns/BUILD.gn

Project Member

Comment 58 by bugdroid1@chromium.org, Oct 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b346dc5435f1f61bf2a791e3467433f34b2e66da

commit b346dc5435f1f61bf2a791e3467433f34b2e66da
Author: A Olsen <olsen@chromium.org>
Date: Tue Oct 16 09:46:32 2018

Reland "Move InstallAttributes to chromeos/settings"

This is a reland of aa6c62dce936e966e2db445045dd04414553642b

Original change's description:
> Move InstallAttributes to chromeos/settings
>
> Necessary for moving CrosSettings to chromeos/settings, so it
> can eventually be used by Mustash.
>
> The following changes were necessary:
> 1. Move install_attributes{.h, .cc} and unit_test.cc
> 2. Update c/b/chromeos/BUILD.gn (source) and chromeos/settings/BUILD.gn
> (destination)
> 3. Update chromeos/DEPS to allow a dependency on cloud_policy_constants.h,
> which is included by install_attributes.h
>
> 4. Unfortunately, that same dependency lead to a circular dependency.
> I was able to avoid that by separating out a "common_constants" rule in
> components/policy/core/common, so that chromeos/settings only depends on
> that, and not on all of components/policy. So I had to change
> components/policy/core/common/BUILD.gn
> and I removed an unnecessary include from
> components/policy/core/common/cloud/cloud_policy_constants.cc
>
> 5. Update #include directives for install_attributes to point to new
> location. This is the other 22 files, all with diffs of +1 -1.
>
> Bug: 446937
> Change-Id: Icc8ba19c5fec91b13004b19680ead9c9f8b0a288
> Reviewed-on: https://chromium-review.googlesource.com/c/1264757
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Commit-Queue: A Olsen <olsen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599140}

Bug: 446937
Change-Id: If20ad953b2cb9c3808415c7c112eec8d0e97915b
Reviewed-on: https://chromium-review.googlesource.com/c/1278750
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: A Olsen <olsen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599933}
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/app_mode/kiosk_app_manager.h
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/arc/auth/arc_active_directory_enrollment_token_fetcher.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/dbus/dbus_helper.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/login/demo_mode/demo_session.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/blocking_login_browsertest.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos_unittest.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/device_policy_cros_browser_test.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/enrollment_handler_chromeos.h
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/enrollment_status_chromeos.h
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/settings/device_settings_provider.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/settings/stub_install_attributes.h
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/chromeos/system/device_disabling_manager.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chrome/browser/ui/webui/policy_ui_handler.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chromeos/DEPS
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chromeos/settings/BUILD.gn
[rename] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chromeos/settings/install_attributes.cc
[rename] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chromeos/settings/install_attributes.h
[rename] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/chromeos/settings/install_attributes_unittest.cc
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/components/policy/core/common/BUILD.gn
[modify] https://crrev.com/b346dc5435f1f61bf2a791e3467433f34b2e66da/components/policy/core/common/cloud/cloud_policy_constants.cc

Sign in to add a comment