[Status Report] Migration to new Mojo types - 11/30/2019

154 views
Skip to first unread message

Mario Sanchez Prada

unread,
Dec 2, 2019, 5:12:54 AM12/2/19
to chromium-mojo, platform-architecture-dev

Hi all,

First of all, apologies for having missed the previous bi-weekly report. An unfortunate combination of me being on holidays 1 week after BlinkOn with the fact that my laptop's motherboard died right on Nov 15th made it hard for me to send that report, so I'm updating now with the progress from the entire month, sorry about that.

As usual, we kept focused on converting types outside of Blink + migrating usages of InterfaceProvider API to the new BrowserInterfaceBroker, which naturally helped getting rid of the few old mojo types remaining inside Blink, getting us to the point where there's only one instance left! We'll remove that one now and then will propose a CL to enable presubmit errors inside Blink, so that no old types get now introduced. Note that these numbers now take into account the false positives that the grep query would find, which have been now subtracted from the numbers throughout the whole spreadsheet, see more details at [1].

As for //content and other directories, steady progress in there as well, with old mojo types in now reduced to less than 25% and 60% in those categories compared to what numbers used to look like one month ago, bringing the current progress of the project to 91% if we compared with the date we started, as you can see in the "Project Completion (%)" table I've added to the bottom of this email, after the graph reports.


10/23/2019 10/31/2019 11/7/2019 11/15/2019 11/23/2019 11/30/2019
Blink 6 3 3 3 3 1
Content 339 201 176 161 110 42
Other 957 721 647 607 538 408
TOTAL 1302 925 826 771 651 451



10/23/2019 10/31/2019 11/7/2019 11/15/2019 11/23/2019 11/30/2019
common/associated_interfaces 0 0 0 0 0 0
common/loader 0 0 0 0 0 0
public/common 0 0 0 0 0 0
public/platform 0 0 0 0 0 0
renderer/controller 0 0 0 0 0 0
renderer/core 2 2 2 2 2 0
renderer/modules 0 0 0 0 0 0
renderer/platform 4 1 1 1 1 1
tools/blinkpy 0 0 0 0 0 0
TOTAL 6 3 3 3 3 1


10/23/2019 10/31/2019 11/7/2019 11/15/2019 11/23/2019 11/30/2019
app_shim_remote_cocoa 2 0 0 0 0 0
browser 256 155 134 122 76 27
child 0 0 0 0 0 0
common 9 2 1 1 1 1
gpu 0 0 0 0 0 0
ppapi_plugin 0 0 0 0 0 0
public 21 14 14 13 9 8
renderer 43 24 21 19 18 5
shell 0 0 0 0 0 0
test 7 5 5 5 5 0
utility 1 1 1 1 1 1
TOTAL 339 201 176 161 110 42



10/23/2019 10/31/2019 11/7/2019 11/15/2019 11/23/2019 11/30/2019
android_webview 7 6 5 4 1 0
ash 9 4 4 4 4 4
cc 1 1 1 1 1 1
chrome 144 130 125 108 101 82
chromecast 21 21 19 18 17 16
chromeos 35 10 10 10 10 11
components 71 57 44 40 38 37
device 2 2 2 2 2 2
docs 2 2 2 2 2 2
extensions 22 20 19 15 14 1
fuchsia 14 13 13 13 13 13
google_apis 8 4 1 1 1 1
gpu 0 0 0 0 0 0
headless 1 1 1 1 1 0
ios 5 2 2 2 2 2
ipc 31 31 31 31 30 30
jingle 3 1 0 0 0 0
media 117 47 26 16 5 4
mojo 70 70 71 70 70 70
remoting 1 1 1 1 1 1
sandbox 0 0 0 0 0 0
services 367 289 269 268 225 131
skia 0 0 0 0 0 0
storage 1 1 0 0 0 0
ui 25 8 1 0 0 0
url 0 0 0 0 0 0
TOTAL 957 721 647 607 538 408


Project Completion (%)
Blink 99.77
Content 97.38
Other 86.71
Global 91.16

Find more detail in the base spreadsheet at:
https://docs.google.com/spreadsheets/d/1khYVrfnN74RSDqqXgCFlG9fI89IKWA9J32Ln_2ZcTaU

Thanks,
Mario

[1] https://groups.google.com/a/chromium.org/d/msg/platform-architecture-dev/9vuGL48Lxik/6YscqLI5AwAJ

Mario Sanchez Prada

unread,
Dec 3, 2019, 4:55:48 AM12/3/19
to chromium-mojo, platform-architecture-dev, Jeremy Roman, hide...@chromium.org

Hi again,

After sending this mail yesterday, we had an internal discussion on what to do with //components/arc that we think might be worth bringing it up here as well, since a recent comment by Jeremy Roman in [1] made us realize not everyone might be aware of the issue at hand...

On 2/12/19 11:12, Mario Sanchez Prada wrote:

[...]


10/23/2019 10/31/2019 11/7/2019 11/15/2019 11/23/2019 11/30/2019
[...]
[...] [...] [...] [...] [...] [...]
components 71 57 44 40 38 37
[...]
[...] [...] [...] [...] [...] [...]
TOTAL 957 721 647 607 538 408

As you can see from the numbers above, there are 37 instances of old mojo types to be migrated inside //components according to the grep query we're using, and more than half of those 37 instances (20, to be exact) are inside //components/arc:

      3 components/arc/bluetooth/bluetooth_mojom_traits_unittest.cc
      2 components/arc/keymaster/arc_keymaster_bridge.cc
      1 components/arc/midis/arc_midis_bridge.cc
      2 components/arc/session/arc_bridge_host_impl.cc
      2 components/arc/session/arc_bridge_host_impl.h
      1 components/arc/session/arc_session_impl.cc
      5 components/arc/session/connection_holder.h
      2 components/arc/session/mojo_channel.h
      1 components/arc/timer/arc_timer_bridge.h
      1 components/arc/wake_lock/arc_wake_lock_bridge.h

We did an initial attempt to migrate those bits last month as part of crrev.com/c/1868870, but got a CR-1 because it seems that Android's libchrome repository is not yet ready to handle changes in the .mojom files from Chromium to the new types. The issue seems to be that those .mojom files are copied into the libchrome repo, which does not contain support for the new mojo types yet, so that process would not work anymore like that, see [2] for more details.

This is why we haven't prioritized converting types in //components/arc for now, but now we're approaching the completion of this migration project this question becomes relevant again: what to do with //components/arc?

Note that, as Ken commented in [3], the old and new mojo types are binary compatibles over the wire, so I believe there should be a way to move forward with the new types in the Chromium repo even if libchrome needs to stick to the old ones for a bit longer, but that would require some extra discussion, so adding Hidehiko to Cc here.

Thoughts?

Thanks,
Mario

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1940185/2#message-5cab01a211f6c201ef4e39a13c8a5c62e839f467
[2] https://chromium-review.googlesource.com/c/chromium/src/+/1868870/6#message-5a35c25604873c017644aca5d739f39ca452fe4f
[3] https://chromium-review.googlesource.com/c/chromium/src/+/1868870/6#message-77c2cadc1978b50d739b05cda7f64156b067fe51

Dave Tapuska

unread,
Dec 3, 2019, 9:38:57 AM12/3/19
to Mario Sanchez Prada, chromium-mojo, platform-architecture-dev, Jeremy Roman, hide...@chromium.org
Since they are binary compatible then it is reasonable to make this part of the process of copying them so that we can remove the legacy mojo interfaces entirely. 

Do we know if this is an automated process of copying the mojom files or a manual one? Is there a script that does the copy? Are we able to write a script that turns them (ie. sed) into the appropriate binary compatible version while copying it over?
dave.

--
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 view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/25c57f3a-7ffc-d2e2-71c1-a1c6d091fc54%40igalia.com.

Ken Rockot

unread,
Dec 3, 2019, 9:47:08 AM12/3/19
to Dave Tapuska, Mario Sanchez Prada, chromium-mojo, platform-architecture-dev, Jeremy Roman, Hidehiko Abe
On Tue, Dec 3, 2019 at 6:38 AM Dave Tapuska <dtap...@chromium.org> wrote:
Since they are binary compatible then it is reasonable to make this part of the process of copying them so that we can remove the legacy mojo interfaces entirely. 

Do we know if this is an automated process of copying the mojom files or a manual one? Is there a script that does the copy? Are we able to write a script that turns them (ie. sed) into the appropriate binary compatible version while copying it over?
dave.

I think it's been done manually, but AFAIK typically the roll happens in the other direction -- source of truth in cros repo, rolled into chromium.

Automating translation in that direction is challenging because the old notation for interface remotes is grammatically ambiguous.

I do agree that we want to get rid of these things though, so I would suggest we switch to rolling cross-repo mojoms in the opposite direction, where a correct sed transform is trivial.

It's also possible that none of the cross-repo mojoms change frequently enough for this to require special action at this time.

You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAHgVhZViJgSCqUWwCTwAy4F3PWO9SKGCp-yi9KFH_a-dUHtUkQ%40mail.gmail.com.

Mario Sanchez Prada

unread,
Dec 4, 2019, 4:49:14 AM12/4/19
to Ken Rockot, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, Hidehiko Abe

Hi,

On 3/12/19 15:46, 'Ken Rockot' via chromium-mojo wrote:

On Tue, Dec 3, 2019 at 6:38 AM Dave Tapuska <dtap...@chromium.org> wrote:
Since they are binary compatible then it is reasonable to make this part of the process of copying them so that we can remove the legacy mojo interfaces entirely. 

Do we know if this is an automated process of copying the mojom files or a manual one? Is there a script that does the copy? Are we able to write a script that turns them (ie. sed) into the appropriate binary compatible version while copying it over?
dave.

I think it's been done manually, but AFAIK typically the roll happens in the other direction -- source of truth in cros repo, rolled into chromium.

Do you mean "source of truth in libchrome (Android) repo?" It's the first time I see CrOS mentioned in this topic, that's where my question comes from :-)

Automating translation in that direction is challenging because the old notation for interface remotes is grammatically ambiguous.

I do agree that we want to get rid of these things though, so I would suggest we switch to rolling cross-repo mojoms in the opposite direction, where a correct sed transform is trivial.

I agree it's definitely easier to roll cross-repo mojoms in the opposite direction (i.e. source of truth in Chromium repo) for the reasons you mentioned, but I'm not sure what the actual next steps to make that happen would be... I assume some devs who work on the other repo should obviously get involved, but not sure who to ping on that. Any idea?

It's also possible that none of the cross-repo mojoms change frequently enough for this to require special action at this time.

Just to double-check I'm getting this right... what you mean here is that, we could actually go ahead and change the local copy of the .mojom files in Chromium as long as the original sources in the other repository don't change frequently, right?

We'd still need some agreement from the people working on that other repo on (1) that changes are not frequent and (2) that they would be happy to start off a point where .mojom file would not use the same -old- types in both repos and take it from there, but that would certainly unblock the conversions in Chromium repo.

WDYT?

Thanks again,
Mario


Mario Sanchez Prada

unread,
Dec 9, 2019, 7:33:08 AM12/9/19
to Ken Rockot, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, Hidehiko Abe
Hi again,

On 4/12/19 10:49, Mario Sanchez Prada wrote:
> [...]
>> I do agree that we want to get rid of these things though, so I would suggest we switch to rolling cross-repo mojoms in the opposite direction, where a correct sed transform is trivial.
>
> I agree it's definitely easier to roll cross-repo mojoms in the opposite direction (i.e. source of truth in Chromium repo) for the reasons you mentioned, but I'm not sure what the actual next steps to make that happen would be... I assume some devs who work on the other repo should obviously get involved, but not sure who to ping on that. Any idea?

Pinging here after a few days.

>> It's also possible that none of the cross-repo mojoms change frequently enough for this to require special action at this time.
>
> Just to double-check I'm getting this right... what you mean here is that, we could actually go ahead and change the local /copy/ of the .mojom files in Chromium *as long as* the original sources in the other repository don't change frequently, right?
>
> We'd still need some agreement from the people working on that other repo on (1) that changes are not frequent and (2) that they would be happy to start off a point where .mojom file would not use the same -old- types in both repos and take it from there, but that would certainly unblock the conversions in Chromium repo.
>
> WDYT?

We'd still need to clarify this part as well, in order to determine how to move forward.

Thanks again,
Mario

Hidehiko Abe

unread,
Dec 19, 2019, 10:51:13 AM12/19/19
to Mario Sanchez Prada, Ken Rockot, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, Yusuke Sato, Josh Horwich, Elijah Taylor
Some questions seem based on some misunderstanding of the current situation, so let me give it a try to explain.

> We did an initial attempt to migrate those bits last month as part of crrev.com/c/1868870, but got a CR-1 because it seems that Android's libchrome repository is not yet ready to handle changes in the .mojom files from Chromium to the new types. The issue seems to be that those .mojom files are copied into the libchrome repo, which does not contain support for the new mojo types yet, so that process would not work anymore like that, see [2] for more details.

Two git repos are related, libchrome and ARC, both are a part of Android.
.mojom files are copied from Chrome's component/arc to ARC, not to libchrome. It is built with libchrome.
Current libchrome in Android is not new enough, so new syntax is not available yet.

As for libchrome, there are two libchrome, one in Android (the one explained just above), and the other in Chrome OS.
libchrome is the library extracted from Chrome repository. Now Android's one is upstream, and the it is downstreamed into Chrome OS's one.
I.e., files are copied: Chrome -> Android's libchrome -> Chrome OS libchrome.
There's a plan to swap the libchrome's upstream. Once it happens, the flow will be Chrome -> Chrome OS libchrome -> Android's libchrome. (probably, this is what Ken said).
Next libchrome uprev will be blocked by the upstream swap. The upstream swap is now blocked by Chrome OS libchrome uprev, which is actively on-going.
Once swap happens, Chrome OS libchrome will be uprev'ed more frequently. TBD for Android libchrome. We need a plan, based on Android is released once a year, IMHO.
I think libchrome uprev including new syntax will happen eventually, but it will take time.

> Do we know if this is an automated process of copying the mojom files or a manual one? Is there a script that does the copy?

Copying mojom files are totally manual operation. In most cases, just "cp" command. No script, AFAIK.

> Are we able to write a script that turns them (ie. sed) into the appropriate binary compatible version while copying it over?

It introduces technical complexity to mojom update procedure for ARC.
Also, it makes it difficult to take diff mechanically, thus it makes it difficult to find accidentally stale mojom files. In past, I took just diff, which wouldn't work.
I'm not very sure if ARC would like to accept such a change. CC'ed several ARC core engs.

To me, the straightforward approach is technically, update libchrome in Android, and then use new syntax in ARC's mojom files.

Thanks,
- hidehiko

Dave Tapuska

unread,
Dec 19, 2019, 11:10:07 AM12/19/19
to Hidehiko Abe, Mario Sanchez Prada, Ken Rockot, chromium-mojo, platform-architecture-dev, Jeremy Roman, Yusuke Sato, Josh Horwich, Elijah Taylor
Can we have an estimate when these libchrome changes will be done?  Issue 915258 has been opened for over a year. Also issue 909719 has seen some active progress but it isn't clear how much more work is remaining.  If these libraries take another year to update I don't believe that is an acceptable solution on our end.

dave.


--
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.

Daniel Cheng

unread,
Dec 19, 2019, 5:45:30 PM12/19/19
to Hidehiko Abe, Mario Sanchez Prada, Ken Rockot, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, Yusuke Sato, Josh Horwich, Elijah Taylor
On Fri, Dec 20, 2019 at 12:51 AM 'Hidehiko Abe' via platform-architecture-dev <platform-arc...@chromium.org> wrote:
Some questions seem based on some misunderstanding of the current situation, so let me give it a try to explain.

> We did an initial attempt to migrate those bits last month as part of crrev.com/c/1868870, but got a CR-1 because it seems that Android's libchrome repository is not yet ready to handle changes in the .mojom files from Chromium to the new types. The issue seems to be that those .mojom files are copied into the libchrome repo, which does not contain support for the new mojo types yet, so that process would not work anymore like that, see [2] for more details.

Two git repos are related, libchrome and ARC, both are a part of Android.
.mojom files are copied from Chrome's component/arc to ARC, not to libchrome. It is built with libchrome.
Current libchrome in Android is not new enough, so new syntax is not available yet.

As for libchrome, there are two libchrome, one in Android (the one explained just above), and the other in Chrome OS.
libchrome is the library extracted from Chrome repository. Now Android's one is upstream, and the it is downstreamed into Chrome OS's one.
I.e., files are copied: Chrome -> Android's libchrome -> Chrome OS libchrome.
There's a plan to swap the libchrome's upstream. Once it happens, the flow will be Chrome -> Chrome OS libchrome -> Android's libchrome. (probably, this is what Ken said).
Next libchrome uprev will be blocked by the upstream swap. The upstream swap is now blocked by Chrome OS libchrome uprev, which is actively on-going.
Once swap happens, Chrome OS libchrome will be uprev'ed more frequently. TBD for Android libchrome. We need a plan, based on Android is released once a year, IMHO.
I think libchrome uprev including new syntax will happen eventually, but it will take time.

> Do we know if this is an automated process of copying the mojom files or a manual one? Is there a script that does the copy?

Copying mojom files are totally manual operation. In most cases, just "cp" command. No script, AFAIK.

> Are we able to write a script that turns them (ie. sed) into the appropriate binary compatible version while copying it over?

It introduces technical complexity to mojom update procedure for ARC.
Also, it makes it difficult to take diff mechanically, thus it makes it difficult to find accidentally stale mojom files. In past, I took just diff, which wouldn't work.
I'm not very sure if ARC would like to accept such a change. CC'ed several ARC core engs.


I think it wouldn't be that bad to introduce a rewriter, as long as it were automated. The workflow would basically be:
- copy mojom files from //component/arc to staging directory
- run the script across the staging directory to transform it to the old-style syntax
- cp staging directory into ARC and see if there are diffs with git diff

Daniel
 
To me, the straightforward approach is technically, update libchrome in Android, and then use new syntax in ARC's mojom files.

Thanks,
- hidehiko

On Mon, Dec 9, 2019 at 9:33 PM Mario Sanchez Prada <ma...@igalia.com> wrote:
Hi again,

On 4/12/19 10:49, Mario Sanchez Prada wrote:
> [...]
>> I do agree that we want to get rid of these things though, so I would suggest we switch to rolling cross-repo mojoms in the opposite direction, where a correct sed transform is trivial.
>
> I agree it's definitely easier to roll cross-repo mojoms in the opposite direction (i.e. source of truth in Chromium repo) for the reasons you mentioned, but I'm not sure what the actual next steps to make that happen would be... I assume some devs who work on the other repo should obviously get involved, but not sure who to ping on that. Any idea?

Pinging here after a few days.

>> It's also possible that none of the cross-repo mojoms change frequently enough for this to require special action at this time.
>
> Just to double-check I'm getting this right... what you mean here is that, we could actually go ahead and change the local /copy/ of the .mojom files in Chromium *as long as* the original sources in the other repository don't change frequently, right?
>
> We'd still need some agreement from the people working on that other repo on (1) that changes are not frequent and (2) that they would be happy to start off a point where .mojom file would not use the same -old- types in both repos and take it from there, but that would certainly unblock the conversions in Chromium repo.
>
> WDYT?

We'd still need to clarify this part as well, in order to determine how to move forward.

Thanks again,
Mario

--
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.

Andrew Moylan

unread,
Dec 19, 2019, 6:13:46 PM12/19/19
to Ken Rockot, Dave Tapuska, Mario Sanchez Prada, chromium-mojo, platform-architecture-dev, Jeremy Roman, Hidehiko Abe
On Wed, 4 Dec 2019 at 01:47, 'Ken Rockot' via chromium-mojo <chromi...@chromium.org> wrote:


On Tue, Dec 3, 2019 at 6:38 AM Dave Tapuska <dtap...@chromium.org> wrote:
Since they are binary compatible then it is reasonable to make this part of the process of copying them so that we can remove the legacy mojo interfaces entirely. 

Do we know if this is an automated process of copying the mojom files or a manual one? Is there a script that does the copy? Are we able to write a script that turns them (ie. sed) into the appropriate binary compatible version while copying it over?
dave.

I think it's been done manually, but AFAIK typically the roll happens in the other direction -- source of truth in cros repo, rolled into chromium.

Automating translation in that direction is challenging because the old notation for interface remotes is grammatically ambiguous.

I do agree that we want to get rid of these things though, so I would suggest we switch to rolling cross-repo mojoms in the opposite direction, where a correct sed transform is trivial.

It's also possible that none of the cross-repo mojoms change frequently enough for this to require special action at this time.

Sorry to jump in, but to color this story a bit with a non-ARC example of this:

For ML Service, we usually are rolling mojoms from CrOS -> Chrome direction, because the CrOS side is where the API is usually "expanding".
We do it with a standard cp + sed as shown in the description of this commit:

Now that the Chromium side has been migrated to the new types, next time I roll I plan to simply add another sed command(s) to convert old to new types. Probably an explicit separate sed for each and every ThingPtr -> pending_remote<Thing>.

As Ken suggested, this is infrequent enough not to be a worry.
The gerrit code review interface will reveal if we missed any ThingPtrs in our sed(s).

And if the bash command gets too ugly to paste into Gerrit description with a straight face, a roll_mojoms.sh can be constructed.



 

Hidehiko Abe

unread,
Dec 20, 2019, 4:28:03 AM12/20/19
to Andrew Moylan, Ken Rockot, Dave Tapuska, Mario Sanchez Prada, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii
> Can we have an estimate when these libchrome changes will be done?  Issue 915258 has been opened for over a year. Also issue 909719 has seen some active progress but it isn't clear how much more work is remaining.  If these libraries take another year to update I don't believe that is an acceptable solution on our end.

Current libchrome uprev focus is Chrome OS, and TBD for Android's libchrome (At least there's no plan in Q1). CC'ed libchrome team.

I had quick chat with yusukes@ and he told me that, if mid-term solution is needed,
probably the alternative approach is to run a script on build time, instead of uprev time, since it is fully automated. Then, the mirroring between Chrome and ARC does not need to be worried.
Though, I don't think sed is a way to go, because it looks too weak. It wouldn't work even just with an additional line break, and there's no guarantee that it works for any mojom files in the future.

If it is necessary, could you develop and maintain the script which ensures that "downgrading mojom to mojom" works properly for any valid mojom files? Then, it can be run in the ARC build, I think.

Thanks,
- hidehiko


Ken Rockot

unread,
Dec 20, 2019, 10:17:50 AM12/20/19
to Hidehiko Abe, Andrew Moylan, Dave Tapuska, Mario Sanchez Prada, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii


On Fri, Dec 20, 2019, 1:28 AM Hidehiko Abe <hide...@google.com> wrote:
> Can we have an estimate when these libchrome changes will be done?  Issue 915258 has been opened for over a year. Also issue 909719 has seen some active progress but it isn't clear how much more work is remaining.  If these libraries take another year to update I don't believe that is an acceptable solution on our end.

Current libchrome uprev focus is Chrome OS, and TBD for Android's libchrome (At least there's no plan in Q1). CC'ed libchrome team.

I had quick chat with yusukes@ and he told me that, if mid-term solution is needed,
probably the alternative approach is to run a script on build time, instead of uprev time, since it is fully automated. Then, the mirroring between Chrome and ARC does not need to be worried.
Though, I don't think sed is a way to go, because it looks too weak. It wouldn't work even just with an additional line break, and there's no guarantee that it works for any mojom files in the future.

If it is necessary, could you develop and maintain the script which ensures that "downgrading mojom to mojom" works properly for any valid mojom files? Then, it can be run in the ARC build, I think.

Sounds reasonable to me. The script would be trivial to write and maintain.

Mario Sanchez Prada

unread,
Dec 23, 2019, 7:29:28 AM12/23/19
to Ken Rockot, Hidehiko Abe, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii

Hi,

On 20/12/19 16:17, 'Ken Rockot' via chromium-mojo wrote:

On Fri, Dec 20, 2019, 1:28 AM Hidehiko Abe <hide...@google.com> wrote:
> Can we have an estimate when these libchrome changes will be done?  Issue 915258 has been opened for over a year. Also issue 909719 has seen some active progress but it isn't clear how much more work is remaining.  If these libraries take another year to update I don't believe that is an acceptable solution on our end.

Current libchrome uprev focus is Chrome OS, and TBD for Android's libchrome (At least there's no plan in Q1). CC'ed libchrome team.

I had quick chat with yusukes@ and he told me that, if mid-term solution is needed,
probably the alternative approach is to run a script on build time, instead of uprev time, since it is fully automated. Then, the mirroring between Chrome and ARC does not need to be worried.
Though, I don't think sed is a way to go, because it looks too weak. It wouldn't work even just with an additional line break, and there's no guarantee that it works for any mojom files in the future.

If it is necessary, could you develop and maintain the script which ensures that "downgrading mojom to mojom" works properly for any valid mojom files? Then, it can be run in the ARC build, I think.

Sounds reasonable to me. The script would be trivial to write and maintain.

Looks like we have a plan then, thank you all for your feedback and ideas!

I'm going OoO today until January 7th (back on that day), so I'll take a look into writing that script then if nobody beats me to it. I'll probably start with a bash script and see how it goes, but if it gets too ugly I'll likely switch to python, which should be ok too from a build-time perspective I think.

Anyway, time to go now... happy holidays and happy new year in advance!

Thanks,
Mario

PS: I'm not sure I'll be able to send the status report on the 1st of January due to me being OoO, so I'll just share here that //content has been fully migrated now too, joining Blink in the 100% achievement! See https://docs.google.com/spreadsheets/d/1khYVrfnN74RSDqqXgCFlG9fI89IKWA9J32Ln_2ZcTaU/edit#gid=346422195

Mario Sanchez Prada

unread,
Jan 7, 2020, 7:42:26 AM1/7/20
to Ken Rockot, Hidehiko Abe, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii

Hi all,

On 23/12/19 13:29, Mario Sanchez Prada wrote:
On 20/12/19 16:17, 'Ken Rockot' via chromium-mojo wrote:
On Fri, Dec 20, 2019, 1:28 AM Hidehiko Abe <hide...@google.com> wrote:
[...]

I had quick chat with yusukes@ and he told me that, if mid-term solution is needed,
probably the alternative approach is to run a script on build time, instead of uprev time, since it is fully automated. Then, the mirroring between Chrome and ARC does not need to be worried.
Though, I don't think sed is a way to go, because it looks too weak. It wouldn't work even just with an additional line break, and there's no guarantee that it works for any mojom files in the future.

If it is necessary, could you develop and maintain the script which ensures that "downgrading mojom to mojom" works properly for any valid mojom files? Then, it can be run in the ARC build, I think.

Sounds reasonable to me. The script would be trivial to write and maintain.

Looks like we have a plan then, thank you all for your feedback and ideas!

I'm going OoO today until January 7th (back on that day), so I'll take a look into writing that script then if nobody beats me to it. I'll probably start with a bash script and see how it goes, but if it gets too ugly I'll likely switch to python, which should be ok too from a build-time perspective I think

I've taken a look to having such a script this morning and came up with a pretty simple one that takes the target directory as its only parameter and converts all the .mojom files inside of it from the new to the old types, using sed.

It's attached as mojo_types_converted.sh in this mail, but the gist of it is this:

find "${SRCDIR}" -type f -name "*.mojom" | while read file; do
sed --in-place --regexp-extended \
-e 's/pending_remote<(.*)>/\1/g' \
-e 's/pending_receiver<(.*)>/\1\&/g' \
-e 's/pending_associated_remote<(.*)>/associated \1/g' \
-e 's/pending_associated_receiver<(.*)>/associated \1\&/g' \
"${file}"
done

@hidehiko I considered your comment on avoiding sed here, but I honestly think it's good enough since the bits we'd be translating are never split across line breaks AFAICS, and using sed seemed simple yet powerful enough to me in this particular case, where we're only translating the 4 cases mentioned above. WDYT?

Also, for testing purposes, I ran it over the src/third_party/blink/public/mojom directory and then attached the output of running `git diff` afterwards in mojo_types_converter.diff. See below an excerpt of it:

--- a/third_party/blink/public/mojom/devtools/devtools_agent.mojom
+++ b/third_party/blink/public/mojom/devtools/devtools_agent.mojom
@@ -72,9 +72,9 @@ interface DevToolsAgent {
// |client_expects_binary_response| indicates that responses (and
// notifications) sent from this session should use binary (CBOR)
// encoding as opposed to JSON encoding.
- AttachDevToolsSession(pending_associated_remote<DevToolsSessionHost> host,
- pending_associated_receiver<DevToolsSession> session,
- pending_receiver<DevToolsSession> io_session,
+ AttachDevToolsSession(associated DevToolsSessionHost host,
+ associated DevToolsSession& session,
+ DevToolsSession& io_session,
DevToolsSessionState? reattach_session_state,
bool client_expects_binary_responses);
@@ -102,8 +102,8 @@ interface DevToolsAgentHost {
// |waiting_for_debugger| is true if worker was paused on startup and
// should be resumed by debugger to actually start.
ChildWorkerCreated(
- pending_remote<DevToolsAgent> worker_devtools_agent,
- pending_receiver<DevToolsAgentHost> worker_devtools_agent_host,
+ DevToolsAgent worker_devtools_agent,
+ DevToolsAgentHost& worker_devtools_agent_host,
url.mojom.Url url,
string name,
mojo_base.mojom.UnguessableToken devtools_worker_token,


If this is looking like something that could get accepted, next step would be to get the script (or simply the sed line) integrated with libchrome's build system, both for Android and ChromeOS, so that the conversion is fully automated.

@hidehiko Since I've never contributed directly to libchrome maybe someone else from that team could pick this script and integrate it there? Alternatively, I could try to do that myself, in which case I'd appreciate some guidance.

Thanks,
Mario
mojo_types_converter.diff
mojo_types_converter.sh

Mario Sanchez Prada

unread,
Jan 8, 2020, 4:42:43 AM1/8/20
to Ken Rockot, Hidehiko Abe, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii
On 7/1/20 13:42, Mario Sanchez Prada wrote:

[...]

It's attached as mojo_types_converted.sh in this mail, but the gist of it is this:

find "${SRCDIR}" -type f -name "*.mojom" | while read file; do
sed --in-place --regexp-extended \
-e 's/pending_remote<(.*)>/\1/g' \
-e 's/pending_receiver<(.*)>/\1\&/g' \
-e 's/pending_associated_remote<(.*)>/associated \1/g' \
-e 's/pending_associated_receiver<(.*)>/associated \1\&/g' \
"${file}"
done

Actually, this is not correct, as sed's greedy nature means that it could end up doing the wrong conversion sometimes, as it was the case for native_file_system_file_writer.mojom:

- Write(uint64 offset, pending_remote<Blob> data) => (NativeFileSystemError result,
+ Write(uint64 offset, Blob> data) = (NativeFileSystemError result,
uint64 bytes_written);

Thus, we should avoid that, which we can do with a minor change to the regular expression:

find "${SRCDIR}" -type f -name "*.mojom" | while read file; do
sed --in-place --regexp-extended \
-e 's/pending_remote<([^>]*)>/\1/g' \
-e 's/pending_receiver<([^>]*)>/\1\&/g' \
-e 's/pending_associated_remote<([^>]*)>/associated \1/g' \
-e 's/pending_associated_receiver<([^>]*)>/associated \1\&/g' \
"${file}"
done

See attached the updated script and diff after running this new script over Blink's mojom files.

Apologies for the extra noise. Everything else from my previous email still applies, though

Thanks
Mario


mojo_types_converter.sh
mojo_types_converter.diff

Hidehiko Abe

unread,
Jan 10, 2020, 8:09:56 AM1/10/20
to Mario Sanchez Prada, Ken Rockot, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii, Yusuke Sato
Hi,

I think we have miscommunication.
Actually, running "sed" to conver mojom files looks fragile. E.g., even if just adding a linebreak after "<" due to the length of identifiers, the script wouldn't work,
but it is still valid in mojom_bindings_generator, IIUC.
What I asked was; Could you give it a try to parse the mojom file by the actual parser, and generate a code which is compatible, so that it is guaranteed that the script works for any mojom files supported by mojom_bindings_generator?

Thanks,
- hidehiko


Ken Rockot

unread,
Jan 10, 2020, 11:35:21 AM1/10/20
to Hidehiko Abe, Mario Sanchez Prada, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii, Yusuke Sato


On Fri, Jan 10, 2020, 5:09 AM Hidehiko Abe <hide...@google.com> wrote:
Hi,

I think we have miscommunication.
Actually, running "sed" to conver mojom files looks fragile. E.g., even if just adding a linebreak after "<" due to the length of

I agree that in theory it seems fragile, but in practice the use of such a script is a temporary concern and we have hundreds of mojoms over years of development without a line break in these type names. So very unlikely to be a real issue.

identifiers, the script wouldn't work,
but it is still valid in mojom_bindings_generator, IIUC.
What I asked was; Could you give it a try to parse the mojom file by the actual parser, and generate a code which is compatible, so that it is guaranteed that the script works for any mojom files supported by mojom_bindings_generator?

Doing this would require changes to the parser, which is part of the repo we're trying to uprev.  We would have to develop this change within the libchrome repo and update the dependency in each repo that uses it, wouldn't we?

I worry that this would be significantly more complex than having a simple copyable script and separate build step that can be stuffed into individual builds as needed.

Mario Sanchez Prada

unread,
Jan 14, 2020, 6:24:37 AM1/14/20
to Ken Rockot, Hidehiko Abe, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii, Yusuke Sato

Hi,

On 10/1/20 17:35, 'Ken Rockot' via chromium-mojo wrote:
[...]


On Fri, Jan 10, 2020, 5:09 AM Hidehiko Abe <hide...@google.com> wrote:
Hi,

I think we have miscommunication.
Actually, running "sed" to conver mojom files looks fragile. E.g., even if just adding a linebreak after "<" due to the length of

I agree that in theory it seems fragile, but in practice the use of such a script is a temporary concern and we have hundreds of mojoms over years of development without a line break in these type names. So very unlikely to be a real issue.

This was my thinking exactly: I realize that a sed line might not be perfect and future-proof if we eventually start getting affected by line breaks, but considering that this is a temporary measure to work around this situation while those libchrome repos don't support the new types, I thought it could be a good trade-off.

identifiers, the script wouldn't work,
but it is still valid in mojom_bindings_generator, IIUC.
What I asked was; Could you give it a try to parse the mojom file by the actual parser, and generate a code which is compatible, so that it is guaranteed that the script works for any mojom files supported by mojom_bindings_generator?

Doing this would require changes to the parser, which is part of the repo we're trying to uprev.  We would have to develop this change within the libchrome repo and update the dependency in each repo that uses it, wouldn't we?

I worry that this would be significantly more complex than having a simple copyable script and separate build step that can be stuffed into individual builds as needed.

FWIW, I also share these concerns. Personally, I was hoping we could move on with the simpler approach proposed, since this is the last roadblock preventing us from removing the old types from the chromium repository, but it's not clear to me at the moment whether that would be accepted. So, I think it would be good to try to clarify what the situation is, first of all.

hidehiko@ +others: WDYT? Would you be open in the end to this simpler approach of using a sed script? Or would you rather push for a more complex alternative?

Thanks,
Mario

Hidehiko Abe

unread,
Jan 21, 2020, 11:40:18 AM1/21/20
to Mario Sanchez Prada, Ken Rockot, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii, Yusuke Sato
# Sorry for delayed response.



What I'd like to see is the guarantee that the script produces the mojom files which are collectly parsable by older mojom bindings generator, in order not to break the build.
Another idea I can think of is to write tests inside Chrome tree verifying all mojom files exported to Android repository can be successfully down-coverted (successfully parsed by an old parser) by the sed script.
Because Chrome repository is the upstream of those mojom files, it can help to capture the possible errors in an earlier stage.
Though, this is still more fragile than the down conversion by the actual parser. I'd like to hear the opinion by yusukes@, too.

Thanks,
- hidehiko

Thanks,
Mario

Hidehiko Abe

unread,
Jan 23, 2020, 7:24:19 AM1/23/20
to Mario Sanchez Prada, Ken Rockot, Andrew Moylan, Dave Tapuska, chromium-mojo, platform-architecture-dev, Jeremy Roman, chromeos-libchrome, Laurent Chavey, Qijiang Yūki Ishii, Yusuke Sato, Kentaro Hara