🏣 Stop including .mojom.js files in GN runtime_deps

Stop including .mojom.js files in GN runtime_deps on Android

They are included in the build via .grd files, so don't need to be
uploaded to swarming individually.

BUG=593416

[agrieve, 2016-04-11 20:29:00]

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

[commit-bot: I haz the power, 2016-04-11 20:29:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1875213002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1875213002/1

[agrieve, 2016-04-11 20:31:30]

agrieve@chromium.org changed reviewers:
+ yzshen@chromium.org

[agrieve, 2016-04-11 20:31:31]

I've done very little testing with this change. Just looked through codesearch
to try and find what uses them at runtime and came up empty. If there are any
places, I think they should probably manually list them as data, since the
(seemingly) more normal way is to include these in .pak files.

[yzshen1, 2016-04-11 20:51:57]

On 2016/04/11 20:31:31, agrieve wrote:
> I've done very little testing with this change. Just looked through codesearch
> to try and find what uses them at runtime and came up empty. If there are any
> places, I think they should probably manually list them as data, since the
> (seemingly) more normal way is to include these in .pak files.

I think we don't always include .mojom.js files via .grd.

Having to explicitly specify mojom.js as data deps is tedious and easy to forget
(besides, the problem may not show up immediately).
Would you please tell me more why you would like to do this?

Thanks!

[commit-bot: I haz the power, 2016-04-11 21:14:08]

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

[commit-bot: I haz the power, 2016-04-11 21:14:09]

Dry run: This issue passed the CQ dry run.

[agrieve, 2016-04-12 02:44:51]

On 2016/04/11 20:51:57, yzshen1 wrote:
> On 2016/04/11 20:31:31, agrieve wrote:
> > I've done very little testing with this change. Just looked through
codesearch
> > to try and find what uses them at runtime and came up empty. If there are
any
> > places, I think they should probably manually list them as data, since the
> > (seemingly) more normal way is to include these in .pak files.
> 
> I think we don't always include .mojom.js files via .grd.
Are you able to point me to an example? any bots I can try?

> 
> Having to explicitly specify mojom.js as data deps is tedious and easy to
forget
> (besides, the problem may not show up immediately).
> Would you please tell me more why you would like to do this?
> 
> Thanks!

https://bugs.chromium.org/p/chromium/issues/detail?id=593416

I'm working on trying to generate .isolate files for Android rather than using
checked in copies. For Android .isolate files are used for both pushing files to
swarming as well as for pushing files to device, so it's important to keep the
list of files to just the set that are used.

If you run: gn desc out/Debug //android_webview/test:android_webview_unittests
runtime_deps | grep mojom

You can see there are a lot of .mojom.js files, which don't show up in any
checked-in .isolate file, which is why I think they are unused (also did a
codesearch for them and couldn't find non-grd references)

[yzshen1, 2016-04-12 17:32:01]

On 2016/04/12 02:44:51, agrieve wrote:
> On 2016/04/11 20:51:57, yzshen1 wrote:
> > On 2016/04/11 20:31:31, agrieve wrote:
> > > I've done very little testing with this change. Just looked through
> codesearch
> > > to try and find what uses them at runtime and came up empty. If there are
> any
> > > places, I think they should probably manually list them as data, since the
> > > (seemingly) more normal way is to include these in .pak files.
> > 
> > I think we don't always include .mojom.js files via .grd.
> Are you able to point me to an example? any bots I can try?
> 
> > 
> > Having to explicitly specify mojom.js as data deps is tedious and easy to
> forget
> > (besides, the problem may not show up immediately).
> > Would you please tell me more why you would like to do this?
> > 
> > Thanks!
> 
> https://bugs.chromium.org/p/chromium/issues/detail?id=593416
> 
> I'm working on trying to generate .isolate files for Android rather than using
> checked in copies. For Android .isolate files are used for both pushing files
to
> swarming as well as for pushing files to device, so it's important to keep the
> list of files to just the set that are used.
> 
> If you run: gn desc out/Debug //android_webview/test:android_webview_unittests
> runtime_deps | grep mojom
> 
> You can see there are a lot of .mojom.js files, which don't show up in any
> checked-in .isolate file, which is why I think they are unused (also did a
> codesearch for them and couldn't find non-grd references)

Thanks for explaining!

I think some tests that need to access standalone mojom.js files are not run
with swarming, such as mojo_js_unittests and mojo_js_integration_tests, which is
probably why you didn't see mojom.js files in .isolate files. 

It seems one way to solve the problem you described is to only generate mojom js
bindings when necessary. Currently, when depending on:
mojom("foo") {
  sources=["foo.mojom"]
}

Even if you only need the C++ bindings, the target generates js bindings and
makes that a data dependency for you. Maybe we should have something like:

mojom("foo") {
  sources=["foo.mojom"]
  language = ["cpp"]
}


WDYT?

[yzshen1, 2016-04-12 17:32:46]

Description was changed from

==========
Stop including .mojom.js files in GN runtime_deps

They are included in the build via .grd files, so don't need to be
uploaded to swarming individually.

BUG=593416
==========

to

==========
Stop including .mojom.js files in GN runtime_deps

They are included in the build via .grd files, so don't need to be
uploaded to swarming individually.

BUG=593416
==========

[yzshen1, 2016-04-12 17:32:46]

yzshen@chromium.org changed reviewers:
+ rockot@chromium.org

[agrieve, 2016-04-12 18:15:30]

On 2016/04/12 17:32:01, yzshen1 wrote:
> On 2016/04/12 02:44:51, agrieve wrote:
> > On 2016/04/11 20:51:57, yzshen1 wrote:
> > > On 2016/04/11 20:31:31, agrieve wrote:
> > > > I've done very little testing with this change. Just looked through
> > codesearch
> > > > to try and find what uses them at runtime and came up empty. If there
are
> > any
> > > > places, I think they should probably manually list them as data, since
the
> > > > (seemingly) more normal way is to include these in .pak files.
> > > 
> > > I think we don't always include .mojom.js files via .grd.
> > Are you able to point me to an example? any bots I can try?
> > 
> > > 
> > > Having to explicitly specify mojom.js as data deps is tedious and easy to
> > forget
> > > (besides, the problem may not show up immediately).
> > > Would you please tell me more why you would like to do this?
> > > 
> > > Thanks!
> > 
> > https://bugs.chromium.org/p/chromium/issues/detail?id=593416
> > 
> > I'm working on trying to generate .isolate files for Android rather than
using
> > checked in copies. For Android .isolate files are used for both pushing
files
> to
> > swarming as well as for pushing files to device, so it's important to keep
the
> > list of files to just the set that are used.
> > 
> > If you run: gn desc out/Debug
//android_webview/test:android_webview_unittests
> > runtime_deps | grep mojom
> > 
> > You can see there are a lot of .mojom.js files, which don't show up in any
> > checked-in .isolate file, which is why I think they are unused (also did a
> > codesearch for them and couldn't find non-grd references)
> 
> Thanks for explaining!
> 
> I think some tests that need to access standalone mojom.js files are not run
> with swarming, such as mojo_js_unittests and mojo_js_integration_tests, which
is
> probably why you didn't see mojom.js files in .isolate files. 

Android tests use .isolate files even without swarming to know what files need
to be pushed to the device before running tests.

> 
> It seems one way to solve the problem you described is to only generate mojom
js
> bindings when necessary. Currently, when depending on:
> mojom("foo") {
>   sources=["foo.mojom"]
> }
> 
> Even if you only need the C++ bindings, the target generates js bindings and
> makes that a data dependency for you. Maybe we should have something like:
> 
> mojom("foo") {
>   sources=["foo.mojom"]
>   language = ["cpp"]
> }
> 
> 
> WDYT?

Sounds like an improvement to me! There are ~50 usages of mojom() atm though
(https://code.google.com/p/chromium/codesearch#search/&q=mojom%5C(%20file:%5C....,
which is more work than I can afford to take on atm. Perhaps for now I can just
put the data= behind an if (!is_android)?

[yzshen1, 2016-04-12 19:25:11]

On 2016/04/12 18:15:30, agrieve wrote:
> On 2016/04/12 17:32:01, yzshen1 wrote:
> > On 2016/04/12 02:44:51, agrieve wrote:
> > > On 2016/04/11 20:51:57, yzshen1 wrote:
> > > > On 2016/04/11 20:31:31, agrieve wrote:
> > > > > I've done very little testing with this change. Just looked through
> > > codesearch
> > > > > to try and find what uses them at runtime and came up empty. If there
> are
> > > any
> > > > > places, I think they should probably manually list them as data, since
> the
> > > > > (seemingly) more normal way is to include these in .pak files.
> > > > 
> > > > I think we don't always include .mojom.js files via .grd.
> > > Are you able to point me to an example? any bots I can try?
> > > 
> > > > 
> > > > Having to explicitly specify mojom.js as data deps is tedious and easy
to
> > > forget
> > > > (besides, the problem may not show up immediately).
> > > > Would you please tell me more why you would like to do this?
> > > > 
> > > > Thanks!
> > > 
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=593416
> > > 
> > > I'm working on trying to generate .isolate files for Android rather than
> using
> > > checked in copies. For Android .isolate files are used for both pushing
> files
> > to
> > > swarming as well as for pushing files to device, so it's important to keep
> the
> > > list of files to just the set that are used.
> > > 
> > > If you run: gn desc out/Debug
> //android_webview/test:android_webview_unittests
> > > runtime_deps | grep mojom
> > > 
> > > You can see there are a lot of .mojom.js files, which don't show up in any
> > > checked-in .isolate file, which is why I think they are unused (also did a
> > > codesearch for them and couldn't find non-grd references)
> > 
> > Thanks for explaining!
> > 
> > I think some tests that need to access standalone mojom.js files are not run
> > with swarming, such as mojo_js_unittests and mojo_js_integration_tests,
which
> is
> > probably why you didn't see mojom.js files in .isolate files. 
> 
> Android tests use .isolate files even without swarming to know what files need
> to be pushed to the device before running tests.
> 
> > 
> > It seems one way to solve the problem you described is to only generate
mojom
> js
> > bindings when necessary. Currently, when depending on:
> > mojom("foo") {
> >   sources=["foo.mojom"]
> > }
> > 
> > Even if you only need the C++ bindings, the target generates js bindings and
> > makes that a data dependency for you. Maybe we should have something like:
> > 
> > mojom("foo") {
> >   sources=["foo.mojom"]
> >   language = ["cpp"]
> > }
> > 
> > 
> > WDYT?
> 
> Sounds like an improvement to me! There are ~50 usages of mojom() atm though
>
(https://code.google.com/p/chromium/codesearch#search/&q=mojom%5C(%20file:%5C....,
> which is more work than I can afford to take on atm. Perhaps for now I can
just
> put the data= behind an if (!is_android)?

I am fine with that. Please add a comment about why it is disabled on android
and link to some tracking bug.

[agrieve, 2016-04-13 18:40:17]

Description was changed from

==========
Stop including .mojom.js files in GN runtime_deps

They are included in the build via .grd files, so don't need to be
uploaded to swarming individually.

BUG=593416
==========

to

==========
Stop including .mojom.js files in GN runtime_deps on Android

They are included in the build via .grd files, so don't need to be
uploaded to swarming individually.

BUG=593416
==========

[agrieve, 2016-04-13 18:41:56]

On 2016/04/12 19:25:11, yzshen1 wrote:
> On 2016/04/12 18:15:30, agrieve wrote:
> > On 2016/04/12 17:32:01, yzshen1 wrote:
> > > On 2016/04/12 02:44:51, agrieve wrote:
> > > > On 2016/04/11 20:51:57, yzshen1 wrote:
> > > > > On 2016/04/11 20:31:31, agrieve wrote:
> > > > > > I've done very little testing with this change. Just looked through
> > > > codesearch
> > > > > > to try and find what uses them at runtime and came up empty. If
there
> > are
> > > > any
> > > > > > places, I think they should probably manually list them as data,
since
> > the
> > > > > > (seemingly) more normal way is to include these in .pak files.
> > > > > 
> > > > > I think we don't always include .mojom.js files via .grd.
> > > > Are you able to point me to an example? any bots I can try?
> > > > 
> > > > > 
> > > > > Having to explicitly specify mojom.js as data deps is tedious and easy
> to
> > > > forget
> > > > > (besides, the problem may not show up immediately).
> > > > > Would you please tell me more why you would like to do this?
> > > > > 
> > > > > Thanks!
> > > > 
> > > > https://bugs.chromium.org/p/chromium/issues/detail?id=593416
> > > > 
> > > > I'm working on trying to generate .isolate files for Android rather than
> > using
> > > > checked in copies. For Android .isolate files are used for both pushing
> > files
> > > to
> > > > swarming as well as for pushing files to device, so it's important to
keep
> > the
> > > > list of files to just the set that are used.
> > > > 
> > > > If you run: gn desc out/Debug
> > //android_webview/test:android_webview_unittests
> > > > runtime_deps | grep mojom
> > > > 
> > > > You can see there are a lot of .mojom.js files, which don't show up in
any
> > > > checked-in .isolate file, which is why I think they are unused (also did
a
> > > > codesearch for them and couldn't find non-grd references)
> > > 
> > > Thanks for explaining!
> > > 
> > > I think some tests that need to access standalone mojom.js files are not
run
> > > with swarming, such as mojo_js_unittests and mojo_js_integration_tests,
> which
> > is
> > > probably why you didn't see mojom.js files in .isolate files. 
> > 
> > Android tests use .isolate files even without swarming to know what files
need
> > to be pushed to the device before running tests.
> > 
> > > 
> > > It seems one way to solve the problem you described is to only generate
> mojom
> > js
> > > bindings when necessary. Currently, when depending on:
> > > mojom("foo") {
> > >   sources=["foo.mojom"]
> > > }
> > > 
> > > Even if you only need the C++ bindings, the target generates js bindings
and
> > > makes that a data dependency for you. Maybe we should have something like:
> > > 
> > > mojom("foo") {
> > >   sources=["foo.mojom"]
> > >   language = ["cpp"]
> > > }
> > > 
> > > 
> > > WDYT?
> > 
> > Sounds like an improvement to me! There are ~50 usages of mojom() atm though
> >
>
(https://code.google.com/p/chromium/codesearch#search/&q=mojom%5C(%20file:%5C....,
> > which is more work than I can afford to take on atm. Perhaps for now I can
> just
> > put the data= behind an if (!is_android)?
> 
> I am fine with that. Please add a comment about why it is disabled on android
> and link to some tracking bug.

Done. Although, I'm back to thinking it shouldn't be the default no matter the
language. data should be opt-in, not opt-out. And I don't think it's the
generator's position to choose how its outputs are used.

[jam, 2016-04-13 19:44:53]

just got pointed at this by Yuzhu.

These mojom.js files aren't listed in isolates now, but will be in isolates
generated from GN.

Moving forward, more and more will be used in tests. Given that they're quite
small, and when we use swarming they'll be cached locally on the bots, I think
we shouldn't do this change.

[yzshen1, 2016-04-13 20:01:36]

On 2016/04/13 19:44:53, jam wrote:
> just got pointed at this by Yuzhu.
> 
> These mojom.js files aren't listed in isolates now, but will be in isolates
> generated from GN.
> 
> Moving forward, more and more will be used in tests. Given that they're quite
> small, and when we use swarming they'll be cached locally on the bots, I think
> we shouldn't do this change.

From the chat with John and Ken, I also learnt that some layout tests have
run-time deps on generated JS files which are not built into resources.
This change passed trybots likely because layout tests are not tried on some
platforms, or stale JS files remain on on the trybots.
It is likely that the change as is will break our build sometime later (when
builds get clobbered or stale JS files go out of sync).

:/

[agrieve, 2016-04-13 20:20:47]

On 2016/04/13 20:01:36, yzshen1 wrote:
> On 2016/04/13 19:44:53, jam wrote:
> > just got pointed at this by Yuzhu.
> > 
> > These mojom.js files aren't listed in isolates now, but will be in isolates
> > generated from GN.
> > 
> > Moving forward, more and more will be used in tests. Given that they're
quite
> > small, and when we use swarming they'll be cached locally on the bots, I
think
> > we shouldn't do this change.
> 
> From the chat with John and Ken, I also learnt that some layout tests have
> run-time deps on generated JS files which are not built into resources.
> This change passed trybots likely because layout tests are not tried on some
> platforms, or stale JS files remain on on the trybots.
> It is likely that the change as is will break our build sometime later (when
> builds get clobbered or stale JS files go out of sync).
> 
> :/

I agree it's not a big deal for swarming, but if these will be used for any
tests that run on Android, then we really can't be pushing the files to devices
unless we're sure they are needed. Android tests are slow enough already :(.

What's wrong with having clients of the generated files declare them as data?
It's the clients that require them on-device, and that dictate how they are to
be packaged.

Right now I'm working on generating .isolate files from runtime_deps for Android
tests, and the current logic is to just strip all .mojom.js files.

[jam, 2016-04-13 20:40:58]

On 2016/04/13 20:20:47, agrieve wrote:
> On 2016/04/13 20:01:36, yzshen1 wrote:
> > On 2016/04/13 19:44:53, jam wrote:
> > > just got pointed at this by Yuzhu.
> > > 
> > > These mojom.js files aren't listed in isolates now, but will be in
isolates
> > > generated from GN.
> > > 
> > > Moving forward, more and more will be used in tests. Given that they're
> quite
> > > small, and when we use swarming they'll be cached locally on the bots, I
> think
> > > we shouldn't do this change.
> > 
> > From the chat with John and Ken, I also learnt that some layout tests have
> > run-time deps on generated JS files which are not built into resources.
> > This change passed trybots likely because layout tests are not tried on some
> > platforms, or stale JS files remain on on the trybots.
> > It is likely that the change as is will break our build sometime later (when
> > builds get clobbered or stale JS files go out of sync).
> > 
> > :/
> 
> I agree it's not a big deal for swarming, but if these will be used for any
> tests that run on Android, then we really can't be pushing the files to
devices
> unless we're sure they are needed. Android tests are slow enough already :(.

Data here would be great to show that this is true. The JS bindings are very
small in comparison to all the other files that are pushed. Additionally, this
would be cached once we switch to swarming.

> 
> What's wrong with having clients of the generated files declare them as data?
> It's the clients that require them on-device, and that dictate how they are to
> be packaged.
> 
> Right now I'm working on generating .isolate files from runtime_deps for
Android
> tests, and the current logic is to just strip all .mojom.js files.

[agrieve, 2016-04-14 14:28:31]

On 2016/04/13 20:40:58, jam wrote:
> On 2016/04/13 20:20:47, agrieve wrote:
> > On 2016/04/13 20:01:36, yzshen1 wrote:
> > > On 2016/04/13 19:44:53, jam wrote:
> > > > just got pointed at this by Yuzhu.
> > > > 
> > > > These mojom.js files aren't listed in isolates now, but will be in
> isolates
> > > > generated from GN.
> > > > 
> > > > Moving forward, more and more will be used in tests. Given that they're
> > quite
> > > > small, and when we use swarming they'll be cached locally on the bots, I
> > think
> > > > we shouldn't do this change.
> > > 
> > > From the chat with John and Ken, I also learnt that some layout tests have
> > > run-time deps on generated JS files which are not built into resources.
> > > This change passed trybots likely because layout tests are not tried on
some
> > > platforms, or stale JS files remain on on the trybots.
> > > It is likely that the change as is will break our build sometime later
(when
> > > builds get clobbered or stale JS files go out of sync).
> > > 
> > > :/
> > 
> > I agree it's not a big deal for swarming, but if these will be used for any
> > tests that run on Android, then we really can't be pushing the files to
> devices
> > unless we're sure they are needed. Android tests are slow enough already :(.
> 
> Data here would be great to show that this is true. The JS bindings are very
> small in comparison to all the other files that are pushed. Additionally, this
> would be cached once we switch to swarming.

Swarming isn't used when devs are testing locally. We also sometimes test on
non-rooted devices, where files are pushed one-at-a-time, and each file takes
about 150 to push even when it's 100 bytes.

> 
> > 
> > What's wrong with having clients of the generated files declare them as
data?
> > It's the clients that require them on-device, and that dictate how they are
to
> > be packaged.
> > 
> > Right now I'm working on generating .isolate files from runtime_deps for
> Android
> > tests, and the current logic is to just strip all .mojom.js files.

[jam, 2016-04-15 16:12:24]

On 2016/04/14 14:28:31, agrieve wrote:
> On 2016/04/13 20:40:58, jam wrote:
> > On 2016/04/13 20:20:47, agrieve wrote:
> > > On 2016/04/13 20:01:36, yzshen1 wrote:
> > > > On 2016/04/13 19:44:53, jam wrote:
> > > > > just got pointed at this by Yuzhu.
> > > > > 
> > > > > These mojom.js files aren't listed in isolates now, but will be in
> > isolates
> > > > > generated from GN.
> > > > > 
> > > > > Moving forward, more and more will be used in tests. Given that
they're
> > > quite
> > > > > small, and when we use swarming they'll be cached locally on the bots,
I
> > > think
> > > > > we shouldn't do this change.
> > > > 
> > > > From the chat with John and Ken, I also learnt that some layout tests
have
> > > > run-time deps on generated JS files which are not built into resources.
> > > > This change passed trybots likely because layout tests are not tried on
> some
> > > > platforms, or stale JS files remain on on the trybots.
> > > > It is likely that the change as is will break our build sometime later
> (when
> > > > builds get clobbered or stale JS files go out of sync).
> > > > 
> > > > :/
> > > 
> > > I agree it's not a big deal for swarming, but if these will be used for
any
> > > tests that run on Android, then we really can't be pushing the files to
> > devices
> > > unless we're sure they are needed. Android tests are slow enough already
:(.
> > 
> > Data here would be great to show that this is true. The JS bindings are very
> > small in comparison to all the other files that are pushed. Additionally,
this
> > would be cached once we switch to swarming.
> 
> Swarming isn't used when devs are testing locally.

Do you have data on how long these extra files take compared to without them?

> We also sometimes test on
> non-rooted devices, where files are pushed one-at-a-time, and each file takes
> about 150 to push even when it's 100 bytes.

When you run these tests, how many files are you pushing in total?

> 
> > 
> > > 
> > > What's wrong with having clients of the generated files declare them as
> data?
> > > It's the clients that require them on-device, and that dictate how they
are
> to
> > > be packaged.
> > > 
> > > Right now I'm working on generating .isolate files from runtime_deps for
> > Android
> > > tests, and the current logic is to just strip all .mojom.js files.

[agrieve, 2016-04-15 18:56:39]

On 2016/04/15 16:12:24, jam wrote:
> On 2016/04/14 14:28:31, agrieve wrote:
> > On 2016/04/13 20:40:58, jam wrote:
> > > On 2016/04/13 20:20:47, agrieve wrote:
> > > > On 2016/04/13 20:01:36, yzshen1 wrote:
> > > > > On 2016/04/13 19:44:53, jam wrote:
> > > > > > just got pointed at this by Yuzhu.
> > > > > > 
> > > > > > These mojom.js files aren't listed in isolates now, but will be in
> > > isolates
> > > > > > generated from GN.
> > > > > > 
> > > > > > Moving forward, more and more will be used in tests. Given that
> they're
> > > > quite
> > > > > > small, and when we use swarming they'll be cached locally on the
bots,
> I
> > > > think
> > > > > > we shouldn't do this change.
> > > > > 
> > > > > From the chat with John and Ken, I also learnt that some layout tests
> have
> > > > > run-time deps on generated JS files which are not built into
resources.
> > > > > This change passed trybots likely because layout tests are not tried
on
> > some
> > > > > platforms, or stale JS files remain on on the trybots.
> > > > > It is likely that the change as is will break our build sometime later
> > (when
> > > > > builds get clobbered or stale JS files go out of sync).
> > > > > 
> > > > > :/
> > > > 
> > > > I agree it's not a big deal for swarming, but if these will be used for
> any
> > > > tests that run on Android, then we really can't be pushing the files to
> > > devices
> > > > unless we're sure they are needed. Android tests are slow enough already
> :(.
> > > 
> > > Data here would be great to show that this is true. The JS bindings are
very
> > > small in comparison to all the other files that are pushed. Additionally,
> this
> > > would be cached once we switch to swarming.
> > 
> > Swarming isn't used when devs are testing locally.
> 
> Do you have data on how long these extra files take compared to without them?
> 
> > We also sometimes test on
> > non-rooted devices, where files are pushed one-at-a-time, and each file
takes
> > about 150 to push even when it's 100 bytes.
> 
> When you run these tests, how many files are you pushing in total?
> 
> > 
> > > 
> > > > 
> > > > What's wrong with having clients of the generated files declare them as
> > data?
> > > > It's the clients that require them on-device, and that dictate how they
> are
> > to
> > > > be packaged.
> > > > 
> > > > Right now I'm working on generating .isolate files from runtime_deps for
> > > Android
> > > > tests, and the current logic is to just strip all .mojom.js files.

$ gn desc . //android_webview/test:_android_webview_unittests__library
runtime_deps | grep -v \.so | grep -v \.py |wc -l
82
agrieve@agrievester ~/ssd/git/clankium1/src/out-gn/Debug (android-amp)$ gn desc
. //android_webview/test:_android_webview_unittests__library runtime_deps | grep
-v \.so | grep -v \.py | grep -v mojom | wc -l
3

This tests requires just 3 files, but with mojom, it's pushing 82.

[jam, 2016-04-21 17:59:00]

On 2016/04/15 18:56:39, agrieve wrote:
> On 2016/04/15 16:12:24, jam wrote:
> > On 2016/04/14 14:28:31, agrieve wrote:
> > > On 2016/04/13 20:40:58, jam wrote:
> > > > On 2016/04/13 20:20:47, agrieve wrote:
> > > > > On 2016/04/13 20:01:36, yzshen1 wrote:
> > > > > > On 2016/04/13 19:44:53, jam wrote:
> > > > > > > just got pointed at this by Yuzhu.
> > > > > > > 
> > > > > > > These mojom.js files aren't listed in isolates now, but will be in
> > > > isolates
> > > > > > > generated from GN.
> > > > > > > 
> > > > > > > Moving forward, more and more will be used in tests. Given that
> > they're
> > > > > quite
> > > > > > > small, and when we use swarming they'll be cached locally on the
> bots,
> > I
> > > > > think
> > > > > > > we shouldn't do this change.
> > > > > > 
> > > > > > From the chat with John and Ken, I also learnt that some layout
tests
> > have
> > > > > > run-time deps on generated JS files which are not built into
> resources.
> > > > > > This change passed trybots likely because layout tests are not tried
> on
> > > some
> > > > > > platforms, or stale JS files remain on on the trybots.
> > > > > > It is likely that the change as is will break our build sometime
later
> > > (when
> > > > > > builds get clobbered or stale JS files go out of sync).
> > > > > > 
> > > > > > :/
> > > > > 
> > > > > I agree it's not a big deal for swarming, but if these will be used
for
> > any
> > > > > tests that run on Android, then we really can't be pushing the files
to
> > > > devices
> > > > > unless we're sure they are needed. Android tests are slow enough
already
> > :(.
> > > > 
> > > > Data here would be great to show that this is true. The JS bindings are
> very
> > > > small in comparison to all the other files that are pushed.
Additionally,
> > this
> > > > would be cached once we switch to swarming.
> > > 
> > > Swarming isn't used when devs are testing locally.
> > 
> > Do you have data on how long these extra files take compared to without
them?
> > 
> > > We also sometimes test on
> > > non-rooted devices, where files are pushed one-at-a-time, and each file
> takes
> > > about 150 to push even when it's 100 bytes.
> > 
> > When you run these tests, how many files are you pushing in total?
> > 
> > > 
> > > > 
> > > > > 
> > > > > What's wrong with having clients of the generated files declare them
as
> > > data?
> > > > > It's the clients that require them on-device, and that dictate how
they
> > are
> > > to
> > > > > be packaged.
> > > > > 
> > > > > Right now I'm working on generating .isolate files from runtime_deps
for
> > > > Android
> > > > > tests, and the current logic is to just strip all .mojom.js files.
> 
> $ gn desc . //android_webview/test:_android_webview_unittests__library
> runtime_deps | grep -v \.so | grep -v \.py |wc -l
> 82
> agrieve@agrievester ~/ssd/git/clankium1/src/out-gn/Debug (android-amp)$ gn
desc
> . //android_webview/test:_android_webview_unittests__library runtime_deps |
grep
> -v \.so | grep -v \.py | grep -v mojom | wc -l
> 3
> 
> This tests requires just 3 files, but with mojom, it's pushing 82.

sorry to be clearer, I meant data on how long it takes to push, not how many
files. i.e. I'm curious what effect this has on developer productivity.