New issue
Advanced search Search tips

Issue 758767 link

Starred by 1 user
Status: Fixed
Owner:
rsorokin@chromium.org
Closed: Oct 2017
Cc:
tnagel@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

//chromeos depends on //chrome

Project Member Reported by brettw@chromium.org, Aug 24 2017 
In this patch:
https://codereview.chromium.org/2531063002/diff/180001/chromeos/BUILD.gn
a dependency was added from //chromeos to //chrome.

This violates the layering of the system. //chromeos is at a lower layer and should not depend on chrome. If protos need to be shared, they should be put into a shared location.

We had a significant regression in //components accidentally depending on //chrome. I am trying to add annotations in GN to prevent this, but can not do so completely due to this issue.

The dependency is:

  //components:components_browsertests ->
  //components/autofill/content/browser:browser ->
  //components/autofill/core/browser:browser ->
  //components/infobars/core:core ->
  //ui/base:base ->
  //chromeos:chromeos ->
  //chrome/browser/chromeos:device_policy_proto

Comment 1 by tnagel@chromium.org, Aug 25 2017

Cc: tnagel@chromium.org
Components: Enterprise
Labels: Chromad
Owner: ----
Status: Available (was: Assigned)
Thanks Brett for pointing that out. I've changed teams, thus kicking this over to the fine Chromad folks.

Comment 2 by tnagel@chromium.org, Aug 25 2017

Cc: rsorokin@chromium.org
Roman, do you think you could take a look?

Comment 3 by pbond@chromium.org, Aug 28 2017

Cc: -rsorokin@chromium.org
Labels: Enterprise-Triaged
Owner: rsorokin@chromium.org

Comment 4 by rsorokin@chromium.org, Aug 29 2017

Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 29 2017 

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

commit 820f161f0df1a95b1269ebbd08e5e05e4cb895c9
Author: Brett Wilson <brettw@chromium.org>
Date: Tue Aug 29 23:49:27 2017

Assert no components depend on Chrome.

Not all components flow into components_unittests, but most do.
Adding an assert_no_deps at this level will help prevent accidental
dependencides into Chrome.

ChromeOS currently has violations that are currently allowed as noted.

Bug:  758767 
Change-Id: I0cd7e641a0c9a0151c22446f1cbeeffc160feeb7
Reviewed-on: https://chromium-review.googlesource.com/633754
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498289}
[modify] https://crrev.com/820f161f0df1a95b1269ebbd08e5e05e4cb895c9/components/BUILD.gn
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 16 2017 

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

commit 1b1677d079519049efec7ce7a54bee7232888b96
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Mon Oct 16 12:27:48 2017

Move chrome_device_policy.proto into components/policy/proto

Removes //chromeos on //chrome dependency

TBR=atwilson@chromium.org,achuith@chromium.org

Bug:  758767 
Change-Id: I377f2cedfcd5dea8a3b8cff656780c177d552bcc
Reviewed-on: https://chromium-review.googlesource.com/649548
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509025}
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/attestation/attestation_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/login/login_screen_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/login/saml/saml_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/affiliated_cloud_policy_invalidator_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_cloud_policy_validator.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_local_account_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_local_account_policy_service_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_policy_builder.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_policy_builder.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_policy_remover.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_quirks_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/device_system_use_24hour_clock_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/display_rotation_default_handler_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/login_screen_default_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/off_hours/device_off_hours_controller_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/off_hours/off_hours_policy_applier.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/off_hours/off_hours_proto_parser.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/power_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/signin_profile_apps_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/policy/variations_service_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/cros_settings_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_cache_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_provider.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_service.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_service.h
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_service_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/device_settings_test_helper.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/scoped_cros_settings_test_helper.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/session_manager_operation.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/settings/session_manager_operation_unittest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/shutdown_policy_browsertest.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/system/timezone_resolver_manager.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/system/timezone_util.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/chromeos/tpm_firmware_update.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chrome/browser/ui/webui/settings/chromeos/date_time_handler.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chromeos/BUILD.gn
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chromeos/DEPS
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/chromeos/dbus/fake_auth_policy_client.cc
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/components/BUILD.gn
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/components/policy/proto/BUILD.gn
[rename] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/components/policy/proto/chrome_device_policy.proto
[modify] https://crrev.com/1b1677d079519049efec7ce7a54bee7232888b96/ui/display/display.h

Comment 7 by rsorokin@chromium.org, Oct 16 2017

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 16 2017 

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

commit 5da6600b57b7644f3c7d1269c797874f4396405c
Author: Brett Wilson <brettw@chromium.org>
Date: Mon Oct 16 19:55:12 2017

Assert components don't depend on chrome.

//chromeos has been fixed to not depend on //chrome, so the
components assert_no_deps to chrome can be unconditionally
set.

Bug:  758767 
Change-Id: I5b702b1fb28d041ce00116ffc2e37949fdb24e9a
Reviewed-on: https://chromium-review.googlesource.com/721767
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509143}
[modify] https://crrev.com/5da6600b57b7644f3c7d1269c797874f4396405c/components/BUILD.gn
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 17 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/f9357a5c8cdd8eaadbf80fe96a5546022caca669

commit f9357a5c8cdd8eaadbf80fe96a5546022caca669
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Tue Oct 17 11:44:37 2017

Move chrome_device_policy.proto

In CL:649548 moved it on the Chrome side.
Now adjusting protofiles.ebuild accordingly

Also Uprev protofiles and VERSION to Chromium 64.0.3242

This picks up the following changes:

Changes for components/policy.git
6c661b5 Roman Sorokin   Move chrome_device_policy.proto into
components/policy/proto
4f847a5 Thiemo Nagel    Improve disclaimer for unreleased policies
12fd474 David Reveman   Revert "Change Picture: Policy to disable user
avatar videos"
771b879 Achuith Bhandarkar      Remove logging for 586961
0e59eae Kevin Marshall  Add Fuchsia (target_os=fuchsia) to the list of
Policy platforms.
844e2f0 David Reveman   Change Picture: Policy to disable user avatar
videos
e8bb354 Krishna Govind  Updating XTBs based on .GRDs from branch master
40abcaa Daria Yakovleva [DeviceOffHours] Add "OffHours" mode
notification
6efc575 jdoerrie        Rewrite base::Value::GetType to
base::Value::type on Linux
b595dae Lutz Justen     Fix ArcPolicy and PinnedLauncherApps
documentation
0a650c4 Cait Phillips   Add a pref to disable autofill credit cards so
we can control it by policy.
403570b Owen Min        TemplateWriter: checks supported version on each
platform
bad1ef8 Marc-Andr (MAD) Decoste        Add another flavor of Domain
joined to UMA.
7b4ba07 Krishna Govind  Updating XTBs based on .GRDs from branch master
86137a6 Pavol Marko     Add proto fields for policy invalidation
notifications
2783609 Sean Kau        Fix printing policy documentation.
e5324b6 xdai    Update device wallpaper policy template.
70ede8d Bernhard Bauer  Add a policy to run all Flash content when the
content setting is ALLOW.

Changes for chrome/browser/chromeos/policy/proto.git
cc44113 Roman Sorokin   Move chrome_device_policy.proto into
components/policy/proto
578cd46 David Reveman   Revert "Change Picture: Policy to disable user
avatar videos"
ea7c06f David Reveman   Change Picture: Policy to disable user avatar
videos

BUG= chromium:758767 
TEST=cros_run_unit_tests --board=$BOARD --packages authpolicy

Change-Id: Ic12e9346c165f68730a1cf2ab3398678a066325d
Reviewed-on: https://chromium-review.googlesource.com/721539
Commit-Ready: Roman Sorokin <rsorokin@chromium.org>
Tested-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>

[rename] https://crrev.com/f9357a5c8cdd8eaadbf80fe96a5546022caca669/chromeos-base/protofiles/protofiles-0.0.10.ebuild
[modify] https://crrev.com/f9357a5c8cdd8eaadbf80fe96a5546022caca669/chromeos-base/protofiles/files/VERSION
►

Sign in to add a comment