Appropriate Directory for Common origin_trials Code

[Alex Vallée]

Hi all,

I'm working on my task for platform feature mojoification for origin trials.

The main implementation lives in content/common and is called from both content/browser and content/renderer. It validates origin trial tokens which allows some experimental features to be enabled.

As it is, the content/renderer code is just glue to allow blink to call out to the implementation.

It seems a bit strange to wrap this up into a service which would satisfy the layering requirements. This code is non-trivial and so we do not want to duplicate it, but need to find a spot that can both be included from content/browser and WebKit/core.

Any input on where I should be putting this.

[Jeremy Roman]

It seems like a shame to do IPC for this, so it'd be nice to have a good answer for cases like this. We allow some libraries (like base/, ui/gfx/, cc/, device/) to be called from Blink, at least in Source/platform/ and Source/modules/, so something similar would seem appropriate here. Most of those presently seem to have their own top-level directories in src/, but I'm not sure if there's a better home for small-ish things like this origin trial token code.

Also, haraken@: is this still something that we'd request a wrapper library in Source/platform/ for (since this library would presumably take std::string or base::StringPiece arguments, rather than WTF::String)? I know we've been looking at allowing more direct access, but I haven't heard that we've allowed std::string in core for uses like this so far.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAJwg7svjFvDcER%3D46iDb6r3rsqFb%2BWEw3TW2BYtOf8Yvs8sNkw%40mail.gmail.com.

[Kentaro Hara]

On Wed, Jul 5, 2017 at 4:43 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Tue, Jul 4, 2017 at 3:36 PM, Alex Vallée <ava...@chromium.org> wrote:

Hi all,

I'm working on my task for platform feature mojoification for origin trials.

The main implementation lives in content/common and is called from both content/browser and content/renderer. It validates origin trial tokens which allows some experimental features to be enabled.

As it is, the content/renderer code is just glue to allow blink to call out to the implementation.

It seems a bit strange to wrap this up into a service which would satisfy the layering requirements. This code is non-trivial and so we do not want to duplicate it, but need to find a spot that can both be included from content/browser and WebKit/core.

Any input on where I should be putting this.

It seems like a shame to do IPC for this, so it'd be nice to have a good answer for cases like this.

Totally agreed.

jam@: Do you have any idea on where we should put the common code between browser and renderer?

 

We allow some libraries (like base/, ui/gfx/, cc/, device/) to be called from Blink, at least in Source/platform/ and Source/modules/, so something similar would seem appropriate here. Most of those presently seem to have their own top-level directories in src/, but I'm not sure if there's a better home for small-ish things like this origin trial token code.

Also, haraken@: is this still something that we'd request a wrapper library in Source/platform/ for (since this library would presumably take std::string or base::StringPiece arguments, rather than WTF::String)? I know we've been looking at allowing more direct access, but I haven't heard that we've allowed std::string in core for uses like this so far.

I'm assuming that Blink (including core/) can directly use those libraries but need to convert std::strings with WTF::Strings before using the strings in Blink.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAJwg7svjFvDcER%3D46iDb6r3rsqFb%2BWEw3TW2BYtOf8Yvs8sNkw%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

[John Abd-El-Malek]

On Tue, Jul 4, 2017 at 5:31 PM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 4:43 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Tue, Jul 4, 2017 at 3:36 PM, Alex Vallée <ava...@chromium.org> wrote:

Hi all,

I'm working on my task for platform feature mojoification for origin trials.

The main implementation lives in content/common and is called from both content/browser and content/renderer. It validates origin trial tokens which allows some experimental features to be enabled.

As it is, the content/renderer code is just glue to allow blink to call out to the implementation.

It seems a bit strange to wrap this up into a service which would satisfy the layering requirements. This code is non-trivial and so we do not want to duplicate it, but need to find a spot that can both be included from content/browser and WebKit/core.

Any input on where I should be putting this.

It seems like a shame to do IPC for this, so it'd be nice to have a good answer for cases like this.

Totally agreed.

jam@: Do you have any idea on where we should put the common code between browser and renderer?

This is something that has come up in the past and will come up more frequently as part of getting rid of content/renderer. Our previous attempt was to put this in components. This feels unsatisfying, because it makes the layering confusing for most devs (i.e. components can generally depend on content & blink, except a few that blink depends on). This is what led Kentaro & I to push to remove components/ dependencies on blink/. https://bugs.chromium.org/p/chromium/issues/detail?id=732668 is the tracking bug. There are currently only two includes left (link_header_util & mime_util), which have similar issues as this one (i.e. small piece of code, different string type).

I propose that that we put this code in webkit/public. mime_util is clearly part of blink, it's a description of mime types that blink knows about. link_header_util was in blink, but was moved out to share with content/browser.

Given that this code is in components/ (or content/common), it already doesn't use blink string types so having it use std::string while in webkit/public isn't a regression. Maybe some of the functions could be written to use char* instead (but that seems orthogonal)?

The other issue is what kind of target to use. Historically we've done a lot of work to avoid content/browser depending on blink target; right now it only uses enums/structs. We could create another target, although I'm slightly in favor of putting these few methods in a header if it doesn't cause large binary size increases.

[John Abd-El-Malek]

On Wed, Jul 5, 2017 at 9:36 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Jul 4, 2017 at 5:31 PM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 4:43 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Tue, Jul 4, 2017 at 3:36 PM, Alex Vallée <ava...@chromium.org> wrote:

Hi all,

I'm working on my task for platform feature mojoification for origin trials.

The main implementation lives in content/common and is called from both content/browser and content/renderer. It validates origin trial tokens which allows some experimental features to be enabled.

As it is, the content/renderer code is just glue to allow blink to call out to the implementation.

It seems a bit strange to wrap this up into a service which would satisfy the layering requirements. This code is non-trivial and so we do not want to duplicate it, but need to find a spot that can both be included from content/browser and WebKit/core.

Any input on where I should be putting this.

It seems like a shame to do IPC for this, so it'd be nice to have a good answer for cases like this.

Totally agreed.

jam@: Do you have any idea on where we should put the common code between browser and renderer?

This is something that has come up in the past and will come up more frequently as part of getting rid of content/renderer. Our previous attempt was to put this in components. This feels unsatisfying, because it makes the layering confusing for most devs (i.e. components can generally depend on content & blink, except a few that blink depends on). This is what led Kentaro & I to push to remove components/ dependencies on blink/. https://bugs.chromium.org/p/chromium/issues/detail?id=732668 is the tracking bug. There are currently only two includes left (link_header_util & mime_util), which have similar issues as this one (i.e. small piece of code, different string type).

I propose that that we put this code in webkit/public. mime_util is clearly part of blink, it's a description of mime types that blink knows about. link_header_util was in blink, but was moved out to share with content/browser.

Given that this code is in components/ (or content/common), it already doesn't use blink string types so having it use std::string while in webkit/public isn't a regression. Maybe some of the functions could be written to use char* instead (but that seems orthogonal)?

The other issue is what kind of target to use. Historically we've done a lot of work to avoid content/browser depending on blink target; right now it only uses enums/structs. We could create another target, although I'm slightly in favor of putting these few methods in a header if it doesn't cause large binary size increases.

Thinking about this more, I take this back. We should decide where the files go, and if we all agree on blink/public, then having a GN target (blink_shared/blink_common?)to match that is consistent with the rest of the code.

[Marijn Kruisselbrink]

Ultimately, once we have a blink/browser or whatever the directory for browser-process-side web-platform code would be, hopefully we'll have less of these cases as well I imagine. But (as the author of link_header_util) moving things to blink/public sounds sensible enough, even if it might be a bit weird to have code in blinks public api that doesn't really follow the form/shape of most blink APIs.

Also potential downside is that it might make it harder to restrict which parts of blink can end up using these non-blink-style shared code bits. For example while as you say currently link_header_util indeed uses std::string, within blink this is wrapped in blink::LinkHeader (in platform/loader), which provides a more blink style API for the same functionality, using the right string types, and DEPS files enforce that no code outside of platform/loader uses the lower-level std::string based API. Of course increasing how much of base/std can be used in blink directly will make this less of an issue in most cases, but for things like std::string it'll still be a bit weird.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALhVsw0B6PJNk95sVMupV%2B16HV3GT3%3DfbA3TfWZFLxAU%3De9ooQ%40mail.gmail.com.

[Daniel Cheng]

Can we just provide a simple string adapter for shared code like this that understands std::string and WTF::String?

(Incidentally that's kind of what WebString is today...)

Daniel

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAJwg7svjFvDcER%3D46iDb6r3rsqFb%2BWEw3TW2BYtOf8Yvs8sNkw%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALhVsw0B6PJNk95sVMupV%2B16HV3GT3%3DfbA3TfWZFLxAU%3De9ooQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CA%2BOSsVaYRm-ajd82Vf%3Dk8_WetYCV%3DMrYh3z%2BEe09uVNr33ms%3Dg%40mail.gmail.com.

[Marijn Kruisselbrink]

Certainly for the case of link_header_util rewriting its API in terms of base::StringPiece would help (with cheap conversions from std::string and WTF::StringUTF8Adaptor). The reason it's not like that already is that it relies on other net:: utils to do string parsing that are all written in terms of std::string and/or (pairs of) std::string::const_iterator. So updating the relevant net:: APIs to take base::StringPiece instead of pairs of std::const_iterator (where those pairs could trivially be converted to base::StringPiece as well to not require other net:: callers to need updating) and then updating link_header_util to do the same would definitely give things a nicer API and be more performant at the same time.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAJwg7svjFvDcER%3D46iDb6r3rsqFb%2BWEw3TW2BYtOf8Yvs8sNkw%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALhVsw0B6PJNk95sVMupV%2B16HV3GT3%3DfbA3TfWZFLxAU%3De9ooQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CA%2BOSsVaYRm-ajd82Vf%3Dk8_WetYCV%3DMrYh3z%2BEe09uVNr33ms%3Dg%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAF3XrKoiw4%3DTxwZtULssYMsnxv%3DCY9-RGedkt%2BO6iYWwet59jg%40mail.gmail.com.

[John Abd-El-Malek]

Sure if StringPiece could be used, with an WTF adapter somehow that'd be good. mime_util uses a map of a small number of strings, and it's not clear to me at all this really needs to be map or if a static char* array would suffice.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAJwg7svjFvDcER%3D46iDb6r3rsqFb%2BWEw3TW2BYtOf8Yvs8sNkw%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALhVsw0B6PJNk95sVMupV%2B16HV3GT3%3DfbA3TfWZFLxAU%3De9ooQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CA%2BOSsVaYRm-ajd82Vf%3Dk8_WetYCV%3DMrYh3z%2BEe09uVNr33ms%3Dg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

[Jeremy Roman]

On Wed, Jul 5, 2017 at 1:00 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 9:36 AM, John Abd-El-Malek <j...@chromium.org> wrote:

On Tue, Jul 4, 2017 at 5:31 PM, Kentaro Hara <har...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 4:43 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Tue, Jul 4, 2017 at 3:36 PM, Alex Vallée <ava...@chromium.org> wrote:

Hi all,

I'm working on my task for platform feature mojoification for origin trials.

The main implementation lives in content/common and is called from both content/browser and content/renderer. It validates origin trial tokens which allows some experimental features to be enabled.

As it is, the content/renderer code is just glue to allow blink to call out to the implementation.

It seems a bit strange to wrap this up into a service which would satisfy the layering requirements. This code is non-trivial and so we do not want to duplicate it, but need to find a spot that can both be included from content/browser and WebKit/core.

Any input on where I should be putting this.

It seems like a shame to do IPC for this, so it'd be nice to have a good answer for cases like this.

Totally agreed.

jam@: Do you have any idea on where we should put the common code between browser and renderer?

This is something that has come up in the past and will come up more frequently as part of getting rid of content/renderer. Our previous attempt was to put this in components. This feels unsatisfying, because it makes the layering confusing for most devs (i.e. components can generally depend on content & blink, except a few that blink depends on). This is what led Kentaro & I to push to remove components/ dependencies on blink/. https://bugs.chromium.org/p/chromium/issues/detail?id=732668 is the tracking bug. There are currently only two includes left (link_header_util & mime_util), which have similar issues as this one (i.e. small piece of code, different string type).

I propose that that we put this code in webkit/public. mime_util is clearly part of blink, it's a description of mime types that blink knows about. link_header_util was in blink, but was moved out to share with content/browser.

Given that this code is in components/ (or content/common), it already doesn't use blink string types so having it use std::string while in webkit/public isn't a regression. Maybe some of the functions could be written to use char* instead (but that seems orthogonal)?

The other issue is what kind of target to use. Historically we've done a lot of work to avoid content/browser depending on blink target; right now it only uses enums/structs. We could create another target, although I'm slightly in favor of putting these few methods in a header if it doesn't cause large binary size increases.

Thinking about this more, I take this back. We should decide where the files go, and if we all agree on blink/public, then having a GN target (blink_shared/blink_common?)to match that is consistent with the rest of the code.

Such a target would be a little strange, because it would be located in third_party/WebKit/ but essentially follow Chromium rules, so that it could be used in the browser process without WTF etc. existing. That would mean STL/base types rather than WTF types, and so on.

As far as adapters go, StringUTF8Adaptor can produce a base::StringPiece suitable for many uses.

We allow some libraries (like base/, ui/gfx/, cc/, device/) to be called from Blink, at least in Source/platform/ and Source/modules/, so something similar would seem appropriate here. Most of those presently seem to have their own top-level directories in src/, but I'm not sure if there's a better home for small-ish things like this origin trial token code.

Also, haraken@: is this still something that we'd request a wrapper library in Source/platform/ for (since this library would presumably take std::string or base::StringPiece arguments, rather than WTF::String)? I know we've been looking at allowing more direct access, but I haven't heard that we've allowed std::string in core for uses like this so far.

I'm assuming that Blink (including core/) can directly use those libraries but need to convert std::strings with WTF::Strings before using the strings in Blink.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAJwg7svjFvDcER%3D46iDb6r3rsqFb%2BWEw3TW2BYtOf8Yvs8sNkw%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALhVsw0B6PJNk95sVMupV%2B16HV3GT3%3DfbA3TfWZFLxAU%3De9ooQ%40mail.gmail.com.

[Marijn Kruisselbrink]

On Wed, Jul 5, 2017 at 10:35 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 1:00 PM, John Abd-El-Malek <j...@chromium.org> wrote:

Thinking about this more, I take this back. We should decide where the files go, and if we all agree on blink/public, then having a GN target (blink_shared/blink_common?)to match that is consistent with the rest of the code.

Such a target would be a little strange, because it would be located in third_party/WebKit/ but essentially follow Chromium rules, so that it could be used in the browser process without WTF etc. existing. That would mean STL/base types rather than WTF types, and so on.

I thought it was the long term plan to have the browser-process-side code of web platform features be part of blink as well (in some new blink/browser or similar directory)? Wouldn't we have to address such strangenesses anyway in that case? I'm not saying it might not be weird, but it seems like something we'll need to solve one way or another, even if we don't create a blink_common like target now.

[John Abd-El-Malek]

On Wed, Jul 5, 2017 at 10:38 AM, Marijn Kruisselbrink <m...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 10:35 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 1:00 PM, John Abd-El-Malek <j...@chromium.org> wrote:

Thinking about this more, I take this back. We should decide where the files go, and if we all agree on blink/public, then having a GN target (blink_shared/blink_common?)to match that is consistent with the rest of the code.

Such a target would be a little strange, because it would be located in third_party/WebKit/ but essentially follow Chromium rules, so that it could be used in the browser process without WTF etc. existing. That would mean STL/base types rather than WTF types, and so on.

I thought it was the long term plan to have the browser-process-side code of web platform features be part of blink as well (in some new blink/browser or similar directory)?

Correct, the plan is that third_party/webkit now would move to blink/renderer. blink/browser would contain web platform specific browser code (which moves from content/browser).

I don't have a link to email trail for this; I believe dglazkov sent this to some list.

[Jeremy Roman]

Discussion is a little scattered between tkent (et al)'s "great mv" thread and haraken (et al)'s Onion Soup 2.0 doc, but IIUC we'll ultimately have something like:

blink/

  browser/

    ...

  common/ (maybe? seems kinda-implied by dglazkov's decider-hat ruling)

    ...

  renderer/

    bindings/

    core/

    controller/ (things left over from content/renderer/)

    modules/

    platform/

  tools/

  ...

In that world, it does seem reasonable to say "blink/renderer/ principally uses WTF types; everything outside uses STL types" (and have an assert_no_deps from the other code on the renderer code). This does have a number of upsides compared to our current configuration.

If we make this directory today, where would it go? third_party/WebKit/public/ seems a little odd (because it's almost pure API today, and includes stuff only legal to call in the renderer), Source/ is that renderer code, and "common" doesn't exist yet.

If my understanding's correct, making something like "third_party/WebKit/common/" (or maybe even "blink/common/"), in anticipation of a blink/browser/ and blink/common/ in the future, SGTM. If so, we'll definitely need to update READMEs and PRESUBMITs to agree.

[Jeremy Roman]

On Wed, Jul 5, 2017 at 1:46 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 10:38 AM, Marijn Kruisselbrink <m...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 10:35 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 1:00 PM, John Abd-El-Malek <j...@chromium.org> wrote:

Thinking about this more, I take this back. We should decide where the files go, and if we all agree on blink/public, then having a GN target (blink_shared/blink_common?)to match that is consistent with the rest of the code.

Such a target would be a little strange, because it would be located in third_party/WebKit/ but essentially follow Chromium rules, so that it could be used in the browser process without WTF etc. existing. That would mean STL/base types rather than WTF types, and so on.

I thought it was the long term plan to have the browser-process-side code of web platform features be part of blink as well (in some new blink/browser or similar directory)?

Correct, the plan is that third_party/webkit now would move to blink/renderer. blink/browser would contain web platform specific browser code (which moves from content/browser).

I don't have a link to email trail for this; I believe dglazkov sent this to some list.

https://groups.google.com/a/chromium.org/d/msg/blink-dev/mN9XR4YAG2g/ClRyskjfBQAJ

(and https://docs.google.com/document/d/1hBKAYGGM2Zm7qhqXtdLSubklVDKUo9ir-vJtPEOx11Y/edit?usp=sharing is the Onion Soup 2.0 doc)

 

Wouldn't we have to address such strangenesses anyway in that case? I'm not saying it might not be weird, but it seems like something we'll need to solve one way or another, even if we don't create a blink_common like target now.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALhVsw0LmMp6K0RREe9BYm8v7AbnNb%2BMXrwOwrL%3D0fyO0Ai5Rg%40mail.gmail.com.

[Kentaro Hara]

On Thu, Jul 6, 2017 at 3:00 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 1:38 PM, Marijn Kruisselbrink <m...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 10:35 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Jul 5, 2017 at 1:00 PM, John Abd-El-Malek <j...@chromium.org> wrote:

Thinking about this more, I take this back. We should decide where the files go, and if we all agree on blink/public, then having a GN target (blink_shared/blink_common?)to match that is consistent with the rest of the code.

Such a target would be a little strange, because it would be located in third_party/WebKit/ but essentially follow Chromium rules, so that it could be used in the browser process without WTF etc. existing. That would mean STL/base types rather than WTF types, and so on.

I thought it was the long term plan to have the browser-process-side code of web platform features be part of blink as well (in some new blink/browser or similar directory)? Wouldn't we have to address such strangenesses anyway in that case? I'm not saying it might not be weird, but it seems like something we'll need to solve one way or another, even if we don't create a blink_common like target now.

Discussion is a little scattered between tkent (et al)'s "great mv" thread and haraken (et al)'s Onion Soup 2.0 doc, but IIUC we'll ultimately have something like:

blink/

  browser/

    ...

  common/ (maybe? seems kinda-implied by dglazkov's decider-hat ruling)

    ...

  renderer/

    bindings/

    core/

    controller/ (things left over from content/renderer/)

    modules/

    platform/

  tools/

  ...

Great summary! Yes, this is where we're going.

In that world, it does seem reasonable to say "blink/renderer/ principally uses WTF types; everything outside uses STL types" (and have an assert_no_deps from the other code on the renderer code). This does have a number of upsides compared to our current configuration.

If we make this directory today, where would it go? third_party/WebKit/public/ seems a little odd (because it's almost pure API today, and includes stuff only legal to call in the renderer), Source/ is that renderer code, and "common" doesn't exist yet.

Yeah, I agree with Jeremy. I'm not sure if it's a good idea to abuse WebKit/public/ since the public APIs will be gone after Onion Soup 2.0.

If my understanding's correct, making something like "third_party/WebKit/common/" (or maybe even "blink/common/"), in anticipation of a blink/browser/ and blink/common/ in the future, SGTM. If so, we'll definitely need to update READMEs and PRESUBMITs to agree.

+1 to creating third_party/WebKit/common/ or //blink/common/ (if Chromium side people are okay with creating it now).

[John Abd-El-Malek]

From my pov, either is fine with me. I have a slight preference for third_party/WebKit/common/ because it's easier to understand until the src/blink move happens.

[Kinuko Yasuda]

For loading / networking we're really starting to have lots of code that needs to be moved around or shared between processes, so having a clear destination now where common code can live sounds great.  Either WebKit/common or blink/common sounds good, and +1 for having it in WebKit/common for now.

Let me clarify a bit as things are moving fast: so the top-level dependency around Blink and browser-side code would look something like

services/

  ... things that implement services, both Blink and browser-side code can depend on its public interfaces.

WebKit/common (or blink/common, at least eventually)

  ... things that belong to Web platform live, both Blink and browser-side code can depend on this.

components/

  ... used to be the place for various componentized features with mixed deps, we're starting to ban Blink from depending on it.

It's probably implied, but I propose we also move the components stuff that need to be referenced both from Blink and browser-side code into that directory, which will make our story a lot easier.  Not coincidentally all of them (namely MimeUtils and LinkHeader) are loading related, so I'll incorporate that into our WIP loading re-architecturing plan if that sounds right to people / jam / haraken.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

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

[Kentaro Hara]

On Thu, Jul 6, 2017 at 11:26 AM, Kinuko Yasuda <kin...@chromium.org> wrote:

For loading / networking we're really starting to have lots of code that needs to be moved around or shared between processes, so having a clear destination now where common code can live sounds great.  Either WebKit/common or blink/common sounds good, and +1 for having it in WebKit/common for now.

Let me clarify a bit as things are moving fast: so the top-level dependency around Blink and browser-side code would look something like

services/

  ... things that implement services, both Blink and browser-side code can depend on its public interfaces.

WebKit/common (or blink/common, at least eventually)

  ... things that belong to Web platform live, both Blink and browser-side code can depend on this.

components/

  ... used to be the place for various componentized features with mixed deps, we're starting to ban Blink from depending on it.

It's probably implied, but I propose we also move the components stuff that need to be referenced both from Blink and browser-side code into that directory, which will make our story a lot easier.  Not coincidentally all of them (namely MimeUtils and LinkHeader) are loading related, so I'll incorporate that into our WIP loading re-architecturing plan if that sounds right to people / jam / haraken.

Sounds reasonable to me!

Would you mind creating WebKit/common/ (if there's no objection on this thread until tomorrow)?

 

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

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

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

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

[Kinuko Yasuda]

On Thu, Jul 6, 2017 at 11:32 AM, Kentaro Hara <har...@chromium.org> wrote:

On Thu, Jul 6, 2017 at 11:26 AM, Kinuko Yasuda <kin...@chromium.org> wrote:

For loading / networking we're really starting to have lots of code that needs to be moved around or shared between processes, so having a clear destination now where common code can live sounds great.  Either WebKit/common or blink/common sounds good, and +1 for having it in WebKit/common for now.

Let me clarify a bit as things are moving fast: so the top-level dependency around Blink and browser-side code would look something like

services/

  ... things that implement services, both Blink and browser-side code can depend on its public interfaces.

WebKit/common (or blink/common, at least eventually)

  ... things that belong to Web platform live, both Blink and browser-side code can depend on this.

components/

  ... used to be the place for various componentized features with mixed deps, we're starting to ban Blink from depending on it.

It's probably implied, but I propose we also move the components stuff that need to be referenced both from Blink and browser-side code into that directory, which will make our story a lot easier.  Not coincidentally all of them (namely MimeUtils and LinkHeader) are loading related, so I'll incorporate that into our WIP loading re-architecturing plan if that sounds right to people / jam / haraken.

Sounds reasonable to me!

Would you mind creating WebKit/common/ (if there's no objection on this thread until tomorrow)?

Sure I can, maybe after waiting for a day or so.

 

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

[TAMURA, Kent]

For some reason, we should not mix Chromium-origin code and WebKit-origin code.  So //blink/common is preferable at this moment.

--

TAMURA Kent
Software Engineer, Google

[TAMURA, Kent]

Ah, I assumed moving files not in //third_party/WebKit.  If we'd like to move files in //third_party/WebKit, //third_party/WebKit/common is better.

[Alex Vallée]

So, do we have a decision about where this common code should end up?

[Ian Clelland]

It sounds like there's support for third_party/WebKit/common/origin_trials, which will eventually become blink/common/origin_trials.

third_party/WebKit/common just needs DEPS/OWNERS/README.md, as far as I know. Kinuko, were you planning on including those?

(I suppose you could add those as part of a CL that moves the origin trials code there, but if Kinuko is going to create that directory as part of a different CL, then I guess wait for that to land first)

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.

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

[Kinuko Yasuda]

On Tue, Jul 11, 2017 at 4:30 AM, Ian Clelland <icle...@chromium.org> wrote:

It sounds like there's support for third_party/WebKit/common/origin_trials, which will eventually become blink/common/origin_trials.

third_party/WebKit/common just needs DEPS/OWNERS/README.md, as far as I know. Kinuko, were you planning on including those?

Yep, I have a WIP CL that includes them, and can send it for review soon.

 

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