Move values.mojom from common to base [chromium/src : master]

Oksana Zhuravlova (Gerrit)

unread,

Mar 16, 2018, 10:03:20 PM3/16/18

to alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Ken Rockot, Scott Violet, Daniel Cheng, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Adding Daniel as primary reviewer and Scott for owner approval.

View Change

1 comment:

File mojo/public/mojom/base/values.mojom:
- Patch Set #4, Line 30: // To avoid versioning problems for arc. TODO(sammc): Remove ASAP.
  Removed this since it was not referenced anywhere.

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Daniel Cheng (Gerrit)

unread,

Mar 17, 2018, 2:06:01 AM3/17/18

to Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Ken Rockot, Scott Violet, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.

I have a CL which already implements the new semantics, but I never got back to updating it after rockot's proposal to just keep typemapped //base constructs under mojo. I'll update that CL and send it out for review, if that works for you?

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Ken Rockot

unread,

Mar 19, 2018, 12:31:17 PM3/19/18

to change...@chromium-review.googlesource.com, Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, site-isolation-reviews_chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

On Fri, Mar 16, 2018 at 11:05 PM, Daniel Cheng (Gerrit) <noreply-gerritcoderevie...@chromium.org> wrote:

I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.

I'm confused about what you're suggesting here. Are you saying you want to leave mojo.common.mojom.Value around and have it typemap to std::unique_ptr<base::Value>? I think we still want to get rid of that, but maybe it can be done incrementally once your CL lands.

Or put another way, this CL could be broken into multiple smaller CLs, since it would also now have to change C++ code from std::unique_ptr<base::Value> to just base::Value.

Ken Rockot (Gerrit)

unread,

Mar 19, 2018, 12:31:58 PM3/19/18

to Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Daniel Cheng, Ken Rockot, Scott Violet, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Patch Set 4:

I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.

I'm confused about what you're suggesting here. Are you saying you want to leave mojo.common.mojom.Value around and have it typemap to std::unique_ptr<base::Value>? I think we still want to get rid of that, but maybe it can be done incrementally once your CL lands.

Or put another way, this CL could be broken into multiple smaller CLs, since it will now aksi have to change C++ code from std::unique_ptr<base::Value> to just base::Value.

I have a CL which already implements the new semantics, but I never got back to updating it after rockot's proposal to just keep typemapped //base constructs under mojo. I'll update that CL and send it out for review, if that works for you?

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Daniel Cheng (Gerrit)

unread,

Mar 19, 2018, 12:45:04 PM3/19/18

to Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Ken Rockot, Scott Violet, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Patch Set 4:

Patch Set 4:
I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.
I'm confused about what you're suggesting here. Are you saying you want to leave mojo.common.mojom.Value around and have it typemap to std::unique_ptr<base::Value>? I think we still want to get rid of that, but maybe it can be done incrementally once your CL lands.
Or put another way, this CL could be broken into multiple smaller CLs, since it will now aksi have to change C++ code from std::unique_ptr<base::Value> to just base::Value.

I have a CL which already implements the new semantics, but I never got back to updating it after rockot's proposal to just keep typemapped //base constructs under mojo. I'll update that CL and send it out for review, if that works for you?

Yes, sorry if that was unclear: the request is to leave the legacy one as-is and incrementally migrate over to the new one.

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Scott Violet (Gerrit)

unread,

Mar 20, 2018, 12:07:53 PM3/20/18

to Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Daniel Cheng, Ken Rockot, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Patch Set 4:
Patch Set 4:
Patch Set 4:
I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.
I'm confused about what you're suggesting here. Are you saying you want to leave mojo.common.mojom.Value around and have it typemap to std::unique_ptr<base::Value>? I think we still want to get rid of that, but maybe it can be done incrementally once your CL lands.
Or put another way, this CL could be broken into multiple smaller CLs, since it will now aksi have to change C++ code from std::unique_ptr<base::Value> to just base::Value.

I have a CL which already implements the new semantics, but I never got back to updating it after rockot's proposal to just keep typemapped //base constructs under mojo. I'll update that CL and send it out for review, if that works for you?
Yes, sorry if that was unclear: the request is to leave the legacy one as-is and incrementally migrate over to the new one.

Waiting for this to be resolved before continuing.

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Scott Violet (Gerrit)

unread,

Mar 20, 2018, 12:08:05 PM3/20/18

to Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Daniel Cheng, Ken Rockot, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.
I'm confused about what you're suggesting here. Are you saying you want to leave mojo.common.mojom.Value around and have it typemap to std::unique_ptr<base::Value>? I think we still want to get rid of that, but maybe it can be done incrementally once your CL lands.
Or put another way, this CL could be broken into multiple smaller CLs, since it will now aksi have to change C++ code from std::unique_ptr<base::Value> to just base::Value.

I have a CL which already implements the new semantics, but I never got back to updating it after rockot's proposal to just keep typemapped //base constructs under mojo. I'll update that CL and send it out for review, if that works for you?
Yes, sorry if that was unclear: the request is to leave the legacy one as-is and incrementally migrate over to the new one.
Waiting for this to be resolved before continuing.

By continuing I mean reviewing.

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Oksana Zhuravlova (Gerrit)

unread,

Mar 21, 2018, 4:34:34 PM3/21/18

to alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Daniel Cheng, Ken Rockot, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Patch Set 4:
Patch Set 4:
Patch Set 4:
I'd actually prefer to leave the legacy one. The current base::Value typemap uses the legacy semantics using std::unique_ptr, and will make it harder to migrate Mojo things to use the new interface.
I'm confused about what you're suggesting here. Are you saying you want to leave mojo.common.mojom.Value around and have it typemap to std::unique_ptr<base::Value>? I think we still want to get rid of that, but maybe it can be done incrementally once your CL lands.
Or put another way, this CL could be broken into multiple smaller CLs, since it will now aksi have to change C++ code from std::unique_ptr<base::Value> to just base::Value.

I have a CL which already implements the new semantics, but I never got back to updating it after rockot's proposal to just keep typemapped //base constructs under mojo. I'll update that CL and send it out for review, if that works for you?
Yes, sorry if that was unclear: the request is to leave the legacy one as-is and incrementally migrate over to the new one.

Would the migration to base::Value be outside the scope of this bug (https://bugs.chromium.org/p/chromium/issues/detail?id=799482) and in scope of https://bugs.chromium.org/p/chromium/issues/detail?id=646113?
In this case I will abandon this CL.

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Ken Rockot

unread,

Mar 21, 2018, 4:43:45 PM3/21/18

to change...@chromium-review.googlesource.com, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, site-isolation-reviews_chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Daniel Cheng, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

I think I would count it as part of this bug, but I can certainly help with conversions. We could save base::Value usage conversion for last.

Ken Rockot (Gerrit)

unread,

Mar 21, 2018, 4:43:53 PM3/21/18

to Oksana Zhuravlova, alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Daniel Cheng, Ken Rockot, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

I think I would count it as part of this bug, but I can certainly help with conversions. We could save base::Value usage conversion for last.

View Change

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Oksana Zhuravlova (Gerrit)

unread,

Mar 21, 2018, 5:33:46 PM3/21/18

to alokp...@chromium.org, amp+...@chromium.org, blink-...@chromium.org, blundell+serv...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, creis...@chromium.org, droger+w...@chromium.org, elijahtayl...@chromium.org, feature-me...@chromium.org, hidehik...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, lhchave...@chromium.org, miu+...@chromium.org, msrame...@chromium.org, nasko+c...@chromium.org, poscia...@chromium.org, qsr+...@chromium.org, raymes...@chromium.org, sdefresne...@chromium.org, tracing...@chromium.org, victorhs...@chromium.org, viettrung...@chromium.org, wfh+...@chromium.org, xjz+...@chromium.org, yusuke...@chromium.org, Scott Violet, Daniel Cheng, Ken Rockot, Commit Bot, Aaron Boodman, chromium...@chromium.org, Darin Fisher, John Abd-El-Malek, Kalyan Kondapally, Markus Heintz, Sadrul Chowdhury, Thiago Farina

Oksana Zhuravlova abandoned this change.

View Change

Abandoned

https://chromium-review.googlesource.com/569468 adds a new typemap using values, and existing code will have to be migrated

To view, visit change 967416. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward