components/arc/DEPS - Issue 1966133002: Run RemoveArcData after a user has opted out

Side by Side Diff: components/arc/DEPS

Issue 1966133002: Run RemoveArcData after a user has opted out (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Addressed comments Created 4 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 include_rules = [	1 include_rules = [

	2 "+chrome/common",
	Daniel Erat 2016/05/18 00:31:26 i'm not sure that this is permitted. i don't see a i'm not sure that this is permitted. i don't see any existing dependencies on chrome/ in components//DEPS right now. given the amount of arc code that lives outside of chrome/, there's probably some other way to check if it's enabled without looking at a chrome pref, right? dspaid* 2016/05/18 00:41:43 I was worried about that, and it was one of the re Show quoted text On 2016/05/18 00:31:26, Daniel Erat wrote: > i'm not sure that this is permitted. i don't see any existing dependencies on > chrome/ in components//DEPS right now. > > given the amount of arc code that lives outside of chrome/, there's probably > some other way to check if it's enabled without looking at a chrome pref, right? I was worried about that, and it was one of the reasons that I originally had this in chrome/browser/chromeos/arc... As far as I can tell there's no way to get the arc enabled state outside of looking at the user preferences. Presumably I could also pass the preference key along with the PrefService pointer (or pass a callback to specifically get the kArcEnabled preference), but both seem pretty hacky. Daniel Erat* 2016/05/18 00:58:27 how will the pref be updated, and is it used by an Show quoted text On 2016/05/18 00:41:43, dspaid wrote: > On 2016/05/18 00:31:26, Daniel Erat wrote: > > i'm not sure that this is permitted. i don't see any existing dependencies on > > chrome/ in components//DEPS right now. > > > > given the amount of arc code that lives outside of chrome/, there's probably > > some other way to check if it's enabled without looking at a chrome pref, > right? > > I was worried about that, and it was one of the reasons that I originally had > this in chrome/browser/chromeos/arc... As far as I can tell there's no way to > get the arc enabled state outside of looking at the user preferences. > Presumably I could also pass the preference key along with the PrefService > pointer (or pass a callback to specifically get the kArcEnabled preference), but > both seem pretty hacky. how will the pref be updated, and is it used by anything else? i only see mentions to it in ArcAuthService in my checkout. do you expect the opt-out state to be needed by other code in components/arc in the future? if so, it seems like it'd be cleaner to pass the opt-out state into some more-central place in the component where it can be distributed to everything that needs it. that'll probably make testing easier as well. Luis Héctor Chávez* 2016/05/19 16:10:27 chrome/ is definitely not permitted from component Show quoted text On 2016/05/18 00:58:27, Daniel Erat wrote: > On 2016/05/18 00:41:43, dspaid wrote: > > On 2016/05/18 00:31:26, Daniel Erat wrote: > > > i'm not sure that this is permitted. i don't see any existing dependencies > on > > > chrome/ in components//DEPS right now. > > > > > > given the amount of arc code that lives outside of chrome/, there's probably > > > some other way to check if it's enabled without looking at a chrome pref, > > right? > > > > I was worried about that, and it was one of the reasons that I originally had > > this in chrome/browser/chromeos/arc... As far as I can tell there's no way to > > get the arc enabled state outside of looking at the user preferences. > > Presumably I could also pass the preference key along with the PrefService > > pointer (or pass a callback to specifically get the kArcEnabled preference), > but > > both seem pretty hacky. > > how will the pref be updated, and is it used by anything else? i only see > mentions to it in ArcAuthService in my checkout. > > do you expect the opt-out state to be needed by other code in components/arc in > the future? if so, it seems like it'd be cleaner to pass the opt-out state into > some more-central place in the component where it can be distributed to > everything that needs it. that'll probably make testing easier as well. chrome/ is definitely not permitted from components :( If you can't inject the dependency, maybe you need to move the code to chrome/browser/chromeos/arc. dspaid* 2016/05/20 07:27:07 Done. Show quoted text On 2016/05/19 16:10:27, Luis Héctor Chávez (slow 5-23) wrote: > On 2016/05/18 00:58:27, Daniel Erat wrote: > > On 2016/05/18 00:41:43, dspaid wrote: > > > On 2016/05/18 00:31:26, Daniel Erat wrote: > > > > i'm not sure that this is permitted. i don't see any existing dependencies > > on > > > > chrome/ in components/*/DEPS right now. > > > > > > > > given the amount of arc code that lives outside of chrome/, there's > probably > > > > some other way to check if it's enabled without looking at a chrome pref, > > > right? > > > > > > I was worried about that, and it was one of the reasons that I originally > had > > > this in chrome/browser/chromeos/arc... As far as I can tell there's no way > to > > > get the arc enabled state outside of looking at the user preferences. > > > Presumably I could also pass the preference key along with the PrefService > > > pointer (or pass a callback to specifically get the kArcEnabled preference), > > but > > > both seem pretty hacky. > > > > how will the pref be updated, and is it used by anything else? i only see > > mentions to it in ArcAuthService in my checkout. > > > > do you expect the opt-out state to be needed by other code in components/arc > in > > the future? if so, it seems like it'd be cleaner to pass the opt-out state > into > > some more-central place in the component where it can be distributed to > > everything that needs it. that'll probably make testing easier as well. > > chrome/ is definitely not permitted from components :( If you can't inject the > dependency, maybe you need to move the code to chrome/browser/chromeos/arc. Done.
2 "+chromeos/arc",	3 "+chromeos/arc",

3 "+chromeos/chromeos_switches.h",	4 "+chromeos/chromeos_switches.h",

4 "+chromeos/dbus",	5 "+chromeos/dbus",

5 "+components/exo",	6 "+components/exo",

6 "+components/prefs",	7 "+components/prefs",

7 "+components/signin/core/account_id",	8 "+components/signin/core/account_id",

	9 "+components/user_manager",

8 "+ipc",	10 "+ipc",

9 "+mojo",	11 "+mojo",

10 "+third_party/re2",	12 "+third_party/re2",

11 "+third_party/skia",	13 "+third_party/skia",

12 "+ui/arc",	14 "+ui/arc",

13 ]	15 ]

OLD	NEW

« components/arc.gypi ('K') | « components/arc/BUILD.gn ('k') | components/arc/arc_bridge_service.h » ('j') | components/arc/arc_bridge_service.cc » ('J')