New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 740288 link

Starred by 1 user
Status: Fixed
Owner:
█ l...@chromium.org
Last visit > 30 days ago
Closed: Jul 2017
Cc:
█ roc...@chromium.org
█ yzshen@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Use base::Value through mojo in blink platform/

Project Member Reported by l...@chromium.org, Jul 7 2017 
We recently notice that if we have the following mojo interface

interface A {
  F(mojo.common.mojom.Value value);
}

We can't call F by passing std::unique_ptr<base::Value> because the generated code for blink accepts blink::ValuePtr.

We tried 2 solutions but both didn't work for us.

1. We copied values.typemap to blink platform/mojo as Values.typemap, and adds it into blink_typemaps.gni, but this results in undefined reference in debug build;

2. We made mojom("common_custom_types") to mojom_component("common_custom_types"), and created a new values_typemap.gni for values.typemap only, and add this gni file to blink_bindings_configuration.gni. But this solution will cause multiple defintions.

Any idea?

Comment 1 by roc...@chromium.org, Jul 8 2017 

For a start, you could always just use blink::ValuePtr from within Blink
code.

The real issue here though is that you do want to share the same typemap
between chromium and blink, and we don't currently have any way to support
that since typemap traits are linked into each variant target separately.

I think we may need some support for a shared typemap configuration which
links traits into the shared c++ target instead of each variant. Then the
values typemap could be added to that config. WDYT yuzhu?

Comment 2 by yzshen@chromium.org, Jul 10 2017 

IIUC, in this case, we could solve the problem by:

- build values_struct_straits.{cc,h} into a component, say, values_component,
- have two typemaps registered for the chromium/webkit variants.
- make values_component a public_deps in these two typemaps.

I would like to avoid having one more kind of typemap config if possible.

Existing example in our code base:
https://cs.chromium.org/chromium/src/ui/gfx/geometry/mojo/geometry.typemap?q=file:typemap+geometry&sq=package:chromium&dr=C&l=5
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/mojo/Geometry.typemap?q=file:typemap+geometry&sq=package:chromium&dr=C&l=5

Comment 3 by l...@chromium.org, Jul 10 2017

Owner: l...@chromium.org
Status: Started (was: Untriaged)
Thanks, I uploaded a patch to fix this. https://chromium-review.googlesource.com/c/565337/
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 12 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/623921bf99007b51cd9652a9adff459da54f9519

commit 623921bf99007b51cd9652a9adff459da54f9519
Author: Peiyong Lin <lpy@chromium.org>
Date: Wed Jul 12 04:18:55 2017

[mojo] Fixed mojo.common.mojom.Value builds inside Blink platform.

This patch moves values_struct_traits into a separate target to be shared by the
chromium and blink variant, and adds values.typemap to blink's typemap config.

BUG= 740288 

Change-Id: Iae1bf24dd90ca16af6f7b3e0d3e83d255ae41b16
Reviewed-on: https://chromium-review.googlesource.com/565337
Commit-Queue: lpy <lpy@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485852}
[modify] https://crrev.com/623921bf99007b51cd9652a9adff459da54f9519/mojo/common/BUILD.gn
[modify] https://crrev.com/623921bf99007b51cd9652a9adff459da54f9519/mojo/common/values.typemap
[modify] https://crrev.com/623921bf99007b51cd9652a9adff459da54f9519/mojo/common/values_struct_traits.h
[modify] https://crrev.com/623921bf99007b51cd9652a9adff459da54f9519/third_party/WebKit/Source/platform/mojo/blink_typemaps.gni

Comment 5 by l...@chromium.org, Jul 12 2017

Status: Fixed (was: Started)
This should be fixed.
►

Sign in to add a comment