Can //content depend on //components?

[François Doray]

Question:

Is //components a good place to share code that is currently duplicated between //content and //ios/web?

If not, should we introduce a new top-level directory on which both //content and //ios/web can depend? This top-level directory would be similar to //base, except that it would allow Web feature code. It would prevent code duplication between //content and //ios/web and make the dependency rules clearer for //content and //components.

Background:

1. //components is currently allowed to depend on //content.
2. //content is currently allowed to depend on //components under some conditions. 
3. (1) and (2) imply that there is a dependency cycle between //components and //content. This cycle could be broken by introducing a new top-level directory on which //content can depend, but which cannot itself depend on //content.
4. There is duplicated code between //content and //ios/web. There is a cost to maintaining this code.
• FaviconURL iOS content
• BrowserThread iOS content
• IsOriginSecure iOS content
• WebContentsUserData iOS content
5. //content and //ios/web currently depend on //components/url_formatter.
6. //content currently depends on:

• components/discardable_memory

• components/download

• components/filename_generation

• components/services/font

• components/services/filesystem

• components/services/font

• components/services/leveldb

• components/session_manager

• components/link_header_util

• components/metrics

• components/network_session_configurator

• components/offline_pages

• components/payments

• components/profile_service

• components/rappor

• components/tracing

• components/ukm

• components/url_formatter

• components/viz

• components/cbor

[David Turner]

The document you link to states:

By default, components can depend only on the lower layers of the Chromium
codebase (e.g. base/, net/, etc.). Individual components may additionally allow
dependencies on the content API and IPC; however, if such a component is used
by Chrome for iOS (which does not use the content API or IPC), the component
will have to be in the form of a layered component
(http://www.chromium.org/developers/design-documents/layered-components-design).
This sounds like you should be able to place such code under components/. And it's clear from the list you give
that content/ *definitely* depends on content/ today, despite what the documentation states for it.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAGD3t5Fb4%2BMB5GC_9Fp8Jj55sS3f7YgDh3YY%2Bv9_w4G_qgMd4A%40mail.gmail.com.

[Sylvain Defresne]

My personal opinion is that allowing having //content and //ios/web to depends on some //components make it somewhat confusing given that most of the components should be layered on top of //contents and //ios/web.

I would second the addition of another layer between //base and //components that can be used from //ios/web and //content, and moving the components that are currently used by either //ios/web or //content in that new top-level directory.

-- Sylvain

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACnJMqr2%2B105zybhjxbducPvN1K3Pa3sPJAmz1ft%2BKkDcxZ6eg%40mail.gmail.com.

[Colin Blundell]

Hello,

Thanks for starting this conversation!

I would also be in favor of reinstating the rule that //content is layered strictly below //components. I think that the elimination of that rule has contributed significantly to Chromium engineers' increased confusion about what //components is (which I've heard expressed anecdotally many times).

IIUC, the reason that //content depends on individual components today is to share code with Blink that doesn't belong in Blink itself (e.g., //components/tracing). I would note that components that are depended on by //content don't really fit the conceptual idea of //components as optional features that can be used by multiple embedders. Anything that //content relies on I would think of as "core" rather than "optional".

Given that the expressed desire in the OP is also to share code between //content and //ios/web, I would suggest that the relevant components go in a new //platform or //web_platform directory: these are conceptually features that are underpinnings of Chromium's web platform implementations (I'm not at all tied to these specific names, as I'm sure that there would be a ton of bikeshedding on them and I'm aware that there's potential conflict with naming inside Blink). There is then an easy logical map:

//platform: features that are used by multiple implementations (or multiple pieces) of the web platform. Cannot depend on implementations of the web platform or on //components.

//content, //third_party/blink, //ios/web: Chromium's implementations of the web platform. Can depend on //platform, cannot depend on //components.

//components: optional features for usage by embedders of the web platform. Can depend on specific implementations of the web platform in layered structure (if used by embedders of multiple web platform implementations) or non-layered structure (if used by embedders of only one web platform implementation).

//android_webview, //chrome, //ios/chrome, etc: Embedders. Can depend on all of the above.

The concern that I've heard expressed about splitting the structure is that having to move code if it grows a new dependency is sub-optimal. However, if a component that is used by //content grows a dependency on //content, that would already be a significant design change, as it would imply an inversion of the existing dependency structure. As such, moving that code from //platform to //components as part of the change seems like a logical consequence of the dependency inversion.

Perhaps more frequent would be a feature in //components that has no //content or //ios/web dependencies starting to get used by //content or //ios/web. However, this also seems to me like it should be a decision that isn't taken lightly, and so introducing some friction there to make that a very explicit decision seems not bad either.

Best,

Colin

[John Abd-El-Malek]

The questions I'd have are:

-how much is duplicated between content and ios?

-what is the cost of that duplication (clearly it's non-zero)?

-what is the cost of adding new concepts to users?

I agree that having content/ depending on some components is a bit confusing. At the same time, we have avoided having "component directory" at different levels because one can also end up with a large number of combinations of dependent directories, and that adds its own complexity.

[Colin Blundell]

I'm not close enough at this point to answer the questions about duplication between //content and //ios/web, but wrt growing multiple components directories: I think that //content is fundamental enough to merit a clear separation in the codebase between "features that //content depends on" and "features that depend on //content". I wouldn't view this separation as precedent for creating new top-level directories for "features that can sit below //<foo>" as opposed to "features that can sit above //<foo>".

Best,

Colin

[Ben Goodger]

I recall having this discussion at various points. My sense was that at large within Chromium developers think of //components as //lib, and so reasoning about precise dependencies is quite hard to intuit at scale beyond just what DEPS and .gn files articulate for individual components...

-Ben

[Colin Blundell]

Hi Ben,

Thanks! I think that anything that we can do to mitigate //components becoming //lib would be beneficial, and that relayering //content strictly below //components would be a big step in that direction. I'm definitely aware that we've had discussions like this at several times in the past years and always concluded that the best thing was essentially to leave the status quo in place, but in my experience engineers' confusion about what //components is only continues to grow over time.

Best,

Colin

[Scott Violet]

By my count there are ~190 non grd directories in components. Of those, it looks like 20 already depend upon content.

  -Scott

[John Abd-El-Malek]

FWIW I think introducing a new type of components would add to the confusion. I'm still wondering what the answer is to my questions below.

[Peter Kasting]

On Fri, Mar 29, 2019 at 10:12 AM Colin Blundell <blun...@chromium.org> wrote:

Thanks! I think that anything that we can do to mitigate //components becoming //lib would be beneficial, and that relayering //content strictly below //components would be a big step in that direction. I'm definitely aware that we've had discussions like this at several times in the past years and always concluded that the best thing was essentially to leave the status quo in place, but in my experience engineers' confusion about what //components is only continues to grow over time.

Is there a reason it's problematic for developers to reason only about layering of individual components, and not formally have layering rules that are globally true of all components?

To me it is a conceptual win to imagine //components as simply meaning "this contains subdirectories which are isolated pieces that can express a fine-grained dependency graph".  Declaring things about the layering of all components turns it into "having something in //components means something very specific about what kind of code lives here and how it relates to the rest of the tree; simply isolating a piece and knowing its dependencies is not good enough".  This acts as a hurdle to refactoring efforts to break code into distinct, well-defined, reusable pieces, increases pressure to have things like //base/util, and tends to result in "your code could live in a lot of places", making it harder to understand the tree.

I am quite willing to be wrong here, but I'd like to understand why our tree is simpler and easier to understand when //components has a global layering meaning, and not simply take that as given.

PK

[Ben Goodger]

I agree with Peter. My understanding is we crossed the bridge toward having //components effectively be //lib some time ago.

-Ben

[Colin Blundell]

Hi Scott,

//components depending on //content is and always has been fine conceptually (and was part of the original design for //components). For individual components that are shared with iOS, they either express a //content dependency in a layered fashion or just don't have a //content dependency. It's the introduction of //content depending on //components that I was proposing to undo.

Thanks,

Colin

[Jochen Eisinger]

Just to add a bit to the confusion, //content/shell can also (and does..) depend on components that again depend on //content, as content shell lives on top of the content api

[Colin Blundell]

Well, it's not how //components was originally envisioned, which was optional browser features for embedders to use that sit on top of the web platform.

The concrete thing that we lose by giving up (in the limit) on any global layering rules is the ability to envision where //components is in the Chromium layercake. For a given subdirectory in //components, it would be necessary to understand exactly what that subdirectory is to know what it can depend on and what can depend on it. I think that to the extent that this has been happening over time, it has been a large contributor to Chromium engineers feeling (and expressing) that they don't know what //components is.

In my mind, the tree is simpler and easier to understand when the following (high-to-low) layering holds:

Embedder that glues platform and reusable features (e.g., //chrome, //ios/chrome)

Reusable features (//components)

Platform (//content, //ios/web)

That was the envisioned role and layering of //components from its inception. We could formally define //components as simply meaning "this contains subdirectories that are isolated pieces that can express a fine-grained dependency graph and are used in multiple places throughout the source tree", in which case it just doesn't fit into a drawing of the layering of Chromium as a concept in of itself.  Maybe it would be a net positive to make that change for the reasons that you express; it's not something that I had thought about prior to your email. It's not how we've historically defined //components, though.

Best,

Colin

[Peter Kasting]

On Mon, Apr 1, 2019 at 6:58 AM Colin Blundell <blun...@chromium.org> wrote:

The concrete thing that we lose by giving up (in the limit) on any global layering rules is the ability to envision where //components is in the Chromium layercake. For a given subdirectory in //components, it would be necessary to understand exactly what that subdirectory is to know what it can depend on and what can depend on it.

FWIW I have never understood where //components is, and have always sought to understand individual directories as a one-off, so this is status quo for me.

Unless one works with a particular layer constantly, in which case I'm sure it's clear, I'm skeptical on the degree to which individuals understand the global layering.

The more interesting question is whether they need to.  If information is always available locally about what a layer depends on, is that good enough?  If we had a graph browsing tool that autogenerated its display based on all the DEPS files, would it be sufficient when people needed a larger-scope view?  Or is understanding the large-scale layering of the tree a thing that is useful to the majority of engineers in getting work done?

PK

[François Doray]

On Fri, Mar 29, 2019 at 1:58 PM John Abd-El-Malek <j...@chromium.org> wrote:

FWIW I think introducing a new type of components would add to the confusion. I'm still wondering what the answer is to my questions below.

On Fri, Mar 29, 2019 at 10:12 AM Colin Blundell <blun...@chromium.org> wrote:

Hi Ben,

Thanks! I think that anything that we can do to mitigate //components becoming //lib would be beneficial, and that relayering //content strictly below //components would be a big step in that direction. I'm definitely aware that we've had discussions like this at several times in the past years and always concluded that the best thing was essentially to leave the status quo in place, but in my experience engineers' confusion about what //components is only continues to grow over time.

Example confusion: discussion.

 

Best,

Colin

On Thu, Mar 28, 2019 at 6:32 PM Ben Goodger <b...@chromium.org> wrote:

I recall having this discussion at various points. My sense was that at large within Chromium developers think of //components as //lib, and so reasoning about precise dependencies is quite hard to intuit at scale beyond just what DEPS and .gn files articulate for individual components...

-Ben

On Thu, Mar 28, 2019 at 10:30 AM Colin Blundell <blun...@chromium.org> wrote:

I'm not close enough at this point to answer the questions about duplication between //content and //ios/web, but wrt growing multiple components directories: I think that //content is fundamental enough to merit a clear separation in the codebase between "features that //content depends on" and "features that depend on //content". I wouldn't view this separation as precedent for creating new top-level directories for "features that can sit below //<foo>" as opposed to "features that can sit above //<foo>".

Best,

Colin

On Thu, Mar 28, 2019 at 6:24 PM John Abd-El-Malek <j...@chromium.org> wrote:

The questions I'd have are:

-how much is duplicated between content and ios?

Not that much. ~1000 lines of code.

 

-what is the cost of that duplication (clearly it's non-zero)?

63 commits from gab@, fdoray@, altimin@ and eseckler@ wouldn't have needed to edit iOS files if code had been shared.

We're adding scheduling features on the UI thread in //content. These features won't be available on iOS unless we share code.

 

-what is the cost of adding new concepts to users?

The main cost is that Chrome devs will have to understand the dependency rules of an extra top-level directories. If we think that having to know about the dependency rules of too many top-level directories is an issue, we could move //url, //mojo, //crypto, //net into the new top-level directory (I don't necessary want to make these moves, just highlighting the fact that we can tweak my solution to reduce instead of increase the number of top-level layers that Chrome devs have to know about).

[François Doray]

I would like one of the 2 following outcomes on this thread:

Possible outcome #1: Agreement that individual components can either be above or below //content. Any future confusion can be resolved by linking to this thread.

• No refactoring required.
• No need to teach a new concept to Chrome devs.

Possible outcome #2: A new top-level //web_platform directory is created and //content is no longer allowed to depend on //components.

• It is trivial to determine whether a "component" is "used by //content" or "can use //content".
• There are no exceptions in DEPS files.
• Code can be added to //web_platform instead of //base/util if it is too browser-specific.

In both cases, it's easy to share code between //ios/web and //content.

Re. "Is there a reason it's problematic for developers to reason only about layering of individual components, and not formally have layering rules that are globally true of all components?"

Re. "If information is always available locally about what a layer depends on, is that good enough?"

Today, Chrome devs have different understanding of what can go in //components. We can solve this by explicitly saying that "Individual //components can decide whether they are above or below //content in the dependency graph.", or by having two top-level directories with strict dependency rules, that allow us to draw the dependency rules between Chrome's top-level directories.

Re. "I am quite willing to be wrong here, but I'd like to understand why our tree is simpler and easier to understand when //components has a global layering meaning, and not simply take that as given."

I prefer clear dependency rules for top-level directories, but any solution that leads to a clear/shared understanding of what code goes where is acceptable to me.

[Colin Blundell]

Thank you, François!

Re: your two possible outcomes: 

- Today, individual components can sit above or below //content, as you've illustrated.

- This thread has indicated that at a high level we prefer maintaining this status quo to other possible organizations (e.g., outcome #2, which is my preferred outcome for the reasons I've outlined above and your reason of "preferr[ing] clear dependency rules for top-level directories").

Given those facts, it certainly looks like we're going toward outcome #1. I see two remaining questions to resolve:

1. Should //ios/web and //content be allowed to share code via //components?

2. What should the definition of //components be if we go in this direction, given how far it has drifted from its original purpose of "high-level browser features used by multiple embedders"?

My answers:

1. I don't see any reason why not, given that both //content and //ios/web now depend on //components.

2. I would simply use my augmented variant of PK's definition: "//components contains subdirectories that are isolated pieces that can express a fine-grained dependency graph and are used in multiple places throughout the source tree." This explicitly eliminates any global meaning of //components in the layering of the tree.

Thanks,

Colin

[Scott Violet]

On Tue, Apr 2, 2019 at 5:52 AM Colin Blundell <blun...@chromium.org> wrote:

Thank you, François!

Re: your two possible outcomes: 

- Today, individual components can sit above or below //content, as you've illustrated.

- This thread has indicated that at a high level we prefer maintaining this status quo to other possible organizations (e.g., outcome #2, which is my preferred outcome for the reasons I've outlined above and your reason of "preferr[ing] clear dependency rules for top-level directories").

Given those facts, it certainly looks like we're going toward outcome #1. I see two remaining questions to resolve:

1. Should //ios/web and //content be allowed to share code via //components?

2. What should the definition of //components be if we go in this direction, given how far it has drifted from its original purpose of "high-level browser features used by multiple embedders"?

My answers:

1. I don't see any reason why not, given that both //content and //ios/web now depend on //components.

2. I would simply use my augmented variant of PK's definition: "//components contains subdirectories that are isolated pieces that can express a fine-grained dependency graph and are used in multiple places throughout the source tree." This explicitly eliminates any global meaning of //components in the layering of the tree.

+1

[François Doray]

I attempted to improve the README file of //components so that its existing purpose can be easily understood by everybody:  https://chromium-review.googlesource.com/c/chromium/src/+/1549603

[John Abd-El-Malek]

FWIW I've looked at the example given for sharing browser_thread, and it doesn't seem to make things that much better than the status quo. Sometimes sharing complicated logic is worth it, but sharing has a tradeoff so we should examine it carefully on a component-by-component basis.

If we do identify a piece of logic that's complicated and the benefits are bigger than the cost, that we can add an entry to components/README giving an example of how content/ and ios/ can share logic.

[Mike Dougherty]

Another benefit of sharing logic between //content and //ios is to prevent //ios from falling behind on particular features. If there are separate implementations, it tends to be very easy for improvements to be made in //content while //ios is ignored. This can then cause bigger issues later on when something which is shared depends on that functionality. IMO, sharing core logic can help to show developers that a feature exists on a platform they may not be looking at.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CALhVsw2U5Hf-YCSq-8MZRJiRdyD%2BEp%3DQ%3D35eN_qyFkZErn3WDg%40mail.gmail.com.

[François Doray]

re. "sharing browser_thread [..] doesn't seem to make things that much better than the status quo"

In the status quo:

• https://crbug.com/926785 is a crash that happened because iOS fell behind //content.
• WebThreads on iOS still uses a lock that was identified as a source of jank on other platforms. The issue was fixed 1 year ago on other platforms.  https://crbug.com/821034#c15
• BrowserUIThreadScheduler exists on all platforms except iOS.
• //content: base::PostTaskWithTraits({BrowserThread::UI, TaskPriority::BEST_EFFORT}, ...) posts a task that is scheduled with low priority.
• //ios: base::PostTaskWithTraits({WebThread::UI, TaskPriority::BEST_EFFORT}, ...) posts a task ignoring the BEST_EFFORT priority.

Are we in a better state if we share code between //content and iOS or if we let the iOS team try to keep up with //content?

[Eugene But]

Chrome for iOS team is quite small, so it will be hard for us to keep up with //content. Chrome for iOS product will be in much better state if we share the code between //content and iOS

[John Abd-El-Malek]

To be clear, I'm not saying I'm opposed to code sharing between non-iOS. This is why we have components/ in the first place :)

I'm just saying to share something between src/content and src/ios, we should examine everything on a case-by-case basis. This is a new need for sharing with iOS, as before it was src/chrome that needed to be split. Most of the features that were shared there were each many times bigger than the 1000 lines quoted below. src/content is already overly complex, and it's used by a lot more than src/chrome, so I want to be very mindful about slowing development there (death by a thousand cuts). For browser threads for example, most have already been eliminated and I'm aiming that for pretty much most code knowledge of BrowserThread::IO should go away. If there's a few hundred lines that's shared there, then I don't think it's automatic that sharing is better. We should have this conversation on a per-proposed-component level, and please loop me in for anything being shared out of content/.

[Alex Clarke]

On Tue, 2 Apr 2019 at 22:18, John Abd-El-Malek <j...@chromium.org> wrote:

To be clear, I'm not saying I'm opposed to code sharing between non-iOS. This is why we have components/ in the first place :)

I'm just saying to share something between src/content and src/ios, we should examine everything on a case-by-case basis. This is a new need for sharing with iOS, as before it was src/chrome that needed to be split. Most of the features that were shared there were each many times bigger than the 1000 lines quoted below. src/content is already overly complex, and it's used by a lot more than src/chrome, so I want to be very mindful about slowing development there (death by a thousand cuts). For browser threads for example, most have already been eliminated and I'm aiming that for pretty much most code knowledge of BrowserThread::IO should go away.

That's interesting.  I've not heard of this effort before, why do we want to do that? :)

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CALhVsw1PMcXCtUexSdw%3DmWFxkyO2w61WEtEVyqJMU3L3gy61LA%40mail.gmail.com.

[John Abd-El-Malek]

On Wed, Apr 3, 2019 at 12:55 AM Alex Clarke <alexc...@google.com> wrote:

On Tue, 2 Apr 2019 at 22:18, John Abd-El-Malek <j...@chromium.org> wrote:

To be clear, I'm not saying I'm opposed to code sharing between non-iOS. This is why we have components/ in the first place :)

I'm just saying to share something between src/content and src/ios, we should examine everything on a case-by-case basis. This is a new need for sharing with iOS, as before it was src/chrome that needed to be split. Most of the features that were shared there were each many times bigger than the 1000 lines quoted below. src/content is already overly complex, and it's used by a lot more than src/chrome, so I want to be very mindful about slowing development there (death by a thousand cuts). For browser threads for example, most have already been eliminated and I'm aiming that for pretty much most code knowledge of BrowserThread::IO should go away.

That's interesting.  I've not heard of this effort before, why do we want to do that? :)

Once the network service launches to the rest of the platforms (Android, ChromeOS, ChromeCast), then most of the logic that lives on the IO thread can move to the UI thread. This is code that lived on the IO thread because it was needed to respond to synchronous callbacks based from networking. This resulted in duplication of state between UI and IO thread, and extra hops in various codepaths. There will be some stuff that remains on the IO thread (e.g. if anything needs to be filtered there for performance), but the vast majority can move out. This doc has more background about what would change.

[Alex Clarke]

Thanks! Interesting stuff.

[Albert J. Wong (王重傑)]

FYI, Nico and Colin pointed out that there is a bit of a duplication now between the //base/util directory that had

been discussed in January and the new structure here in components.  We had a mini discussion on the

the CL for //base/util/README.md and wanted to loop back here. Here is a snippet of what we threw
in the README.md.

## How does this differ from //components
Both //components and //base/util contain subdirectories that are (a) intended
for reuse. In addition, //components imposes no global layering in Chromium, so
a subdirectory placed in //components can be used from most-to-all layers in the
codebase, subject to the dependencies that that subdirectory itself holds.

 

In spite of these similarities, there are *conceptual* differences: //components
contains things are closer to full features or subsystems (eg autofill, heap
profiler, cloud devices, visited link tracker) that are not really intended for
large scale reuse.

 

There is some overlap and at some point it will become a judgment call, but
in general, //components are a better fit if the code in question is a feature
module, or subsystem.  //base/util is better if it is a more narrow construct
such as a data structure, coding primitive, etc.

Our current thought is that there is still a useful distinction. In particular, the flat

structure of //components makes it hard to browse and discover things

like flat_map if they're next to autofill.

Let us know what you think!

-Albert

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAPG_qM6sM7W3ch%2BDzL7W%3DHiGJ7r1WVuecXpG89R82Jh7u7NTpw%40mail.gmail.com.

[Peter Kasting]

On Thu, Apr 4, 2019 at 1:11 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

In spite of these similarities, there are *conceptual* differences: //components

contains things are closer to full features or subsystems (eg autofill, heap
profiler, cloud devices, visited link tracker) that are not really intended for
large scale reuse.

I'm not sure whether this restriction -- that things in components are closer to full subsystems -- is always true in practice or generally desirable in principle.

(To be clear, I'm not sure it's _not_ desirable.  I'm being literal here: I'm uncertain.)

My distinction would have been: //base is for things so broadly useful that they are truly "general purpose" and would conceivably fit at any point in the stack, or even in non-Chromium projects.  //components is for all other types of reuse, e.g. anything that's "Chromium-specific".  I dunno if that makes sense though.

(I am biased; I think the main thing //base/util is good for is as a staging ground to get to fine-grained OWNERship, and I don't want it to live long-term.)

PK

[Colin Blundell]

Hi PK,

I think that this viewpoint is reasonable as well. I think that there's also not actually that much distinction, as in practice things that are Chromium-specific are more likely to look like full features or subsystems IMO. I suggest that we wait and see over some amount of time what kinds of things end up in //base/util and where //base/util itself goes.

[François Doray]

The CL that updates the components/README.md file was LGTMed by 3 owners  https://chromium-review.googlesource.com/c/1549603. I'll land it later today if no more comments are added.

The new //components/README.md file links to //base/util/README.md for an explanation of the difference between //components and //base/util. Therefore, any change to what goes in //components vs //base/util can be made there, if necessary.