Fix linker warning in broadcast_channel.mojom-blink.obj.

Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin
which the typemap translated to webkit's SecurityOrigin. This means that
mojo_bindings needs to set BLINK_PLATFORM_IMPLEMENTATION and have that
propagate to the generated target. Otherwise SecurityOrigin is imported
by mojo_bindings to blink_platform.dll, which defines it.

The fix is to split weborigin into a separate build target. Since it
depends on runtime enabled features, these also have to be split out
into a separate target. The WebPublicSuffixList interface is removed
since platform can now call out to net/base directly.

BUG=632082

[jam, 2016-07-29 18:31:16]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 18:32:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 19:45:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 19:45:26]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[jam, 2016-07-30 01:35:31]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-30 01:35:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-07-30 01:49:59]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-30 01:50:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-30 02:30:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-30 02:30:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2016-07-30 17:23:51]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-30 17:23:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-30 18:09:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-30 18:09:30]

Dry run: Try jobs failed on following builders:
  mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gyp_...)

[jam, 2016-08-01 15:39:02]

Description was changed from

==========
Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin which
the typemap translated to webkit's SecurityOrigin. This means that mojo_bindings
needs to set BLINK_PLATFORM_IMPLEMENTATION and have that propagate to the
generated target. Otherwise SecurityOrigin is imported by mojo_bindings to
blink_platform.dll, which defines it.

BUG=632082
==========

to

==========
Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin which
the typemap translated to webkit's SecurityOrigin. This means that mojo_bindings
needs to set BLINK_PLATFORM_IMPLEMENTATION and have that propagate to the
generated target. Otherwise SecurityOrigin is imported by mojo_bindings to
blink_platform.dll, which defines it.

The fix is to split weborigin into a separate build target. Since it depends on
runtime enabled features, these also have to be split out into a separate
target. The WebPublicSuffixList interface is removed since platform can now call
out to net/base directly.

BUG=632082
==========

[jam, 2016-08-01 15:40:42]

jam@chromium.org changed reviewers:
+ jochen@chromium.org

[jochen (gone - plz use gerrit), 2016-08-01 15:47:18]

jochen@chromium.org changed reviewers:
+ esprehn@chromium.org

[jochen (gone - plz use gerrit), 2016-08-01 15:47:19]

+esprehn so you get faster feedback.

Is it possible to restrict the DEPS file for weborigin further? It seems like it
should not include other platform headers, right?

[jam, 2016-08-01 16:32:32]

The CQ bit was checked by jam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 16:32:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2016-08-01 16:34:46]

On 2016/08/01 15:47:19, jochen wrote:
> +esprehn so you get faster feedback.
> 
> Is it possible to restrict the DEPS file for weborigin further? It seems like
it
> should not include other platform headers, right?

sure, documented this restriction through DEPS rules.

[esprehn, 2016-08-01 18:12:52]

Description was changed from

==========
Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin which
the typemap translated to webkit's SecurityOrigin. This means that mojo_bindings
needs to set BLINK_PLATFORM_IMPLEMENTATION and have that propagate to the
generated target. Otherwise SecurityOrigin is imported by mojo_bindings to
blink_platform.dll, which defines it.

The fix is to split weborigin into a separate build target. Since it depends on
runtime enabled features, these also have to be split out into a separate
target. The WebPublicSuffixList interface is removed since platform can now call
out to net/base directly.

BUG=632082
==========

to

==========
Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin
which the typemap translated to webkit's SecurityOrigin. This means that
mojo_bindings needs to set BLINK_PLATFORM_IMPLEMENTATION and have that
propagate to the generated target. Otherwise SecurityOrigin is imported
by mojo_bindings to blink_platform.dll, which defines it.

The fix is to split weborigin into a separate build target. Since it
depends on runtime enabled features, these also have to be split out
into a separate target. The WebPublicSuffixList interface is removed
since platform can now call out to net/base directly.

BUG=632082
==========

[esprehn, 2016-08-01 18:13:07]

Description was changed from

==========
Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin
which the typemap translated to webkit's SecurityOrigin. This means that
mojo_bindings needs to set BLINK_PLATFORM_IMPLEMENTATION and have that
propagate to the generated target. Otherwise SecurityOrigin is imported
by mojo_bindings to blink_platform.dll, which defines it.

The fix is to split weborigin into a separate build target. Since it
depends on runtime enabled features, these also have to be split out
into a separate target. The WebPublicSuffixList interface is removed
since platform can now call out to net/base directly.

BUG=632082
==========

to

==========
Fix linker warning in broadcast_channel.mojom-blink.obj.

The problem is that r406852 introduced a dependency from
//third_party/WebKit/public:mojo_bindings to //url/mojo:url_mojom_origin
which the typemap translated to webkit's SecurityOrigin. This means that
mojo_bindings needs to set BLINK_PLATFORM_IMPLEMENTATION and have that
propagate to the generated target. Otherwise SecurityOrigin is imported
by mojo_bindings to blink_platform.dll, which defines it.

The fix is to split weborigin into a separate build target. Since it
depends on runtime enabled features, these also have to be split out
into a separate target. The WebPublicSuffixList interface is removed
since platform can now call out to net/base directly.

BUG=632082
==========

[esprehn, 2016-08-01 18:17:12]

I'm not sure this is true that it should be a component below platform. Why
can't it use Oilpan? Having RuntimeEnabledFeatures be a separate component is
also strange. Is this a mojo bindings bug since the bindings are in the wrong
library?

[commit-bot: I haz the power, 2016-08-01 18:25:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 18:25:31]

Dry run: This issue passed the CQ dry run.

[jam, 2016-08-01 19:15:45]

On 2016/08/01 18:17:12, esprehn wrote:
> I'm not sure this is true that it should be a component below platform. Why
> can't it use Oilpan?

If in the future it needed to use it, platform/heap's build target can change
from source_set to component.

> Having RuntimeEnabledFeatures be a separate component is
> also strange. Is this a mojo bindings bug since the bindings are in the wrong
> library?

Full background is in the bug and a chromium-mojo thread about these circular
dependencies:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/hnPIZZ5vYWg.
The summary is that there's a circular dependency involving
//third_party/WebKit/public:mojo_bindings and
//third_party/WebKit/Source/platform. mojo_bindings needs to depend on
SecurityOrigin which is defined in platform. However platform also consumes
these bindings.

Other solutions could be to make platform not depend on mojo interfaces. It
looks like https://codereview.chromium.org/2036663003 introduced this, and so
that code in platform/graphics would need to move elsewhere. Or maybe split up
the mojo interfaces in blink into different targets, and the ones that use
mojo::Url wouldn't be used in platform (but I don't know if this constraint is
something that'll always want).

Of these options and the other ones discussed in the bug, this seemed like the
least maintenance burden to me but I'm open to other ideas.

[jochen (gone - plz use gerrit), 2016-08-02 15:26:55]

historically, web origin was a separate component that we intended to be able to
link into the browser process, so that part makes sense IMO.

I'm deferring to Elliott for the overall code structure

[jam, 2016-08-02 19:02:37]

ping

[esprehn, 2016-08-02 19:56:54]

I'm not really a fan of having a separate component for RuntimeEnabledFeatures.
Making something go from source_set to component means we compile it separately
and have to export symbols and such?

Do we need to move the mojo bindings for platform to a separate target like
platform_bindings?

[jam, 2016-08-02 20:10:44]

On 2016/08/02 19:56:54, esprehn wrote:
> I'm not really a fan of having a separate component for
RuntimeEnabledFeatures.
> Making something go from source_set to component means we compile it
separately
> and have to export symbols and such?

Correct.

Since component build is just for developer productivity, what's your concern
with separating runtimeenabledfeatures into a separate target?

> 
> Do we need to move the mojo bindings for platform to a separate target like
> platform_bindings?

This is one of the options mentioned above. The issue is that if platform wants
to use any of the types defined in it in a mojo interface, which seems likely,
then the same circular dependency would occur.

[esprehn, 2016-08-02 20:42:11]

On 2016/08/02 at 20:10:44, jam wrote:
> On 2016/08/02 19:56:54, esprehn wrote:
> > I'm not really a fan of having a separate component for
RuntimeEnabledFeatures.
> > Making something go from source_set to component means we compile it
separately
> > and have to export symbols and such?
> 
> Correct.
> 
> Since component build is just for developer productivity, what's your concern
with separating runtimeenabledfeatures into a separate target?

Maintaining separate components is a burden, the export dance and having to
plumb interfaces around is time consuming and confusing.

> 
> > 
> > Do we need to move the mojo bindings for platform to a separate target like
> > platform_bindings?
> 
> This is one of the options mentioned above. The issue is that if platform
wants to use any of the types defined in it in a mojo interface, which seems
likely, then the same circular dependency would occur.

Right, so can mojo generate the code into the platform target instead? Why does
mojo need to depend on platform?

[jam, 2016-08-02 20:58:40]

On 2016/08/02 20:42:11, esprehn wrote:
> On 2016/08/02 at 20:10:44, jam wrote:
> > On 2016/08/02 19:56:54, esprehn wrote:
> > > I'm not really a fan of having a separate component for
> RuntimeEnabledFeatures.
> > > Making something go from source_set to component means we compile it
> separately
> > > and have to export symbols and such?
> > 
> > Correct.
> > 
> > Since component build is just for developer productivity, what's your
concern
> with separating runtimeenabledfeatures into a separate target?
> 
> Maintaining separate components is a burden, the export dance and 

There shouldn't be a maintenance burden here. The new runtime_enabled_features
target is only depended on by platform and weborigin, which is enforced through
gn's visibility feature. It's a public dependency of the former, so from the POV
of other targets, things are the same.

> having to plumb interfaces around is time consuming and confusing.

I don't know what you mean by plumbing interfaces, can you elaborate?

> 
> > 
> > > 
> > > Do we need to move the mojo bindings for platform to a separate target
like
> > > platform_bindings?
> > 
> > This is one of the options mentioned above. The issue is that if platform
> wants to use any of the types defined in it in a mojo interface, which seems
> likely, then the same circular dependency would occur.
> 
> Right, so can mojo generate the code into the platform target instead?

mojom files need to be specified using the mojom GN template. GN doesn't support
making a template output to an existing target; it has to be a new one. Even if
GN did support this, we couldn't just make
//third_party/WebKit/public:mojo_bindings depend on platform always, since it's
linked to by code in the browser process which doesn't link in blink. We'd have
to make only the blink version of the bindings be merged with platform.. but
this is moot since gn doesn't support this.

> Why does mojo need to depend on platform?

By mojo do you mean //third_party/WebKit/public:mojo_bindings? If so, the
dependency on platform comes from one of the mojoms there using a mojo::Origin
type which is typemapped to blink::SecurityOrigin.

[jochen (gone - plz use gerrit), 2016-08-03 07:22:52]

Sorry that this takes so long. After thinking a bit about this, I wonder what
you think about the following two solutions:

a) if mojo bindings have to access internal types of some other component,
shouldn't they conceptually be part of that component instead of having the
component export some of its internal types?

or b) if mojo bindings are supposed to be separate from the component, shouldn't
the typemaps be in terms of public types of that component, e.g., here,
WebSecurityOrigin instead of SecurityOrigin?

[esprehn, 2016-08-03 09:36:21]

Indeed jam@ and I had that discussion and I came to the same conclusion today. I
think we want to make the Mojo bindings be generated into the component that
defines the types it's mapping. Having it be separate means the component can't
use it's own bindings which seems kind of strange. John didn't think gn had a
way to do this yet, but don't we generate a bunch of code in blink using python
that we compile directly in (ex. The style builder)? Can mojo do whatever that
does?

Using the public types also seems reasonable for this case, but does that fix
the build cycle?

(Sorry this thread didn't get updated from our private discussion)

[jam, 2016-08-03 17:27:32]

Regarding public types:
-I don't think this would work for two reasons. The primary is that
WebSecurityOrigin depends on SecurityOrigin in its implementation. The secondary
is that afaik with content/renderer code moving into blink, there's no more need
for these web wrapper types.


Regarding compiling the mojo bindings as part of platform:
-GN has a way to get the list of output files for a target (get_target_outputs).
However that only works for targets in the same build file. The mojom_bindings
target is  in src/third_party/WebKit/public/BUILD.gn, which makes sense since
the mojoms specified there are in that directory and in subdirectories. I don't
think we want to move that build target to be in
src/third_party/WebKit/platform/BUILD.gn as that would break the GN convention
of build files being located beside the code, right? And also that browser code
includes the non-blink variants of these mojoms; we only want them to include
build targets from WebKit/public right?


On 2016/08/03 07:22:52, jochen wrote:
> Sorry that this takes so long. After thinking a bit about this, I wonder what
> you think about the following two solutions:
> 
> a) if mojo bindings have to access internal types of some other component,
> shouldn't they conceptually be part of that component instead of having the
> component export some of its internal types?
> 
> or b) if mojo bindings are supposed to be separate from the component,
shouldn't
> the typemaps be in terms of public types of that component, e.g., here,
> WebSecurityOrigin instead of SecurityOrigin?

On 2016/08/03 09:36:21, esprehn wrote:
> Indeed jam@ and I had that discussion and I came to the same conclusion today.
I
> think we want to make the Mojo bindings be generated into the component that
> defines the types it's mapping. Having it be separate means the component
can't
> use it's own bindings which seems kind of strange. John didn't think gn had a
> way to do this yet, but don't we generate a bunch of code in blink using
python
> that we compile directly in (ex. The style builder)? Can mojo do whatever that
> does?
> 
> Using the public types also seems reasonable for this case, but does that fix
> the build cycle?
> 
> (Sorry this thread didn't get updated from our private discussion)

[jam, 2016-08-03 23:13:48]

On 2016/08/03 17:27:32, jam wrote:
> Regarding public types:
> -I don't think this would work for two reasons. The primary is that
> WebSecurityOrigin depends on SecurityOrigin in its implementation. The
secondary
> is that afaik with content/renderer code moving into blink, there's no more
need
> for these web wrapper types.
> 
> 
> Regarding compiling the mojo bindings as part of platform:
> -GN has a way to get the list of output files for a target
(get_target_outputs).
> However that only works for targets in the same build file. The mojom_bindings
> target is  in src/third_party/WebKit/public/BUILD.gn, which makes sense since
> the mojoms specified there are in that directory and in subdirectories. I
don't
> think we want to move that build target to be in
> src/third_party/WebKit/platform/BUILD.gn as that would break the GN convention
> of build files being located beside the code, right? And also that browser
code
> includes the non-blink variants of these mojoms; we only want them to include
> build targets from WebKit/public right?

btw here's a version of this: https://codereview.chromium.org/2209883002/ to
compare.

> 
> 
> On 2016/08/03 07:22:52, jochen wrote:
> > Sorry that this takes so long. After thinking a bit about this, I wonder
what
> > you think about the following two solutions:
> > 
> > a) if mojo bindings have to access internal types of some other component,
> > shouldn't they conceptually be part of that component instead of having the
> > component export some of its internal types?
> > 
> > or b) if mojo bindings are supposed to be separate from the component,
> shouldn't
> > the typemaps be in terms of public types of that component, e.g., here,
> > WebSecurityOrigin instead of SecurityOrigin?
> 
> On 2016/08/03 09:36:21, esprehn wrote:
> > Indeed jam@ and I had that discussion and I came to the same conclusion
today.
> I
> > think we want to make the Mojo bindings be generated into the component that
> > defines the types it's mapping. Having it be separate means the component
> can't
> > use it's own bindings which seems kind of strange. John didn't think gn had
a
> > way to do this yet, but don't we generate a bunch of code in blink using
> python
> > that we compile directly in (ex. The style builder)? Can mojo do whatever
that
> > does?
> > 
> > Using the public types also seems reasonable for this case, but does that
fix
> > the build cycle?
> > 
> > (Sorry this thread didn't get updated from our private discussion)