Using url.mojom.Url in services that consumed by Blink's platform

[Victor Costan]

Dear Mojo experts,

I'm struggling with https://crrev.com/c/683635, which introduces a Mojo interface in //services/network that contains a url.mojom.Url. The Blink variant's type mappings map url.mojom.Url to blink::KURL, which is defined in Blink's platform.

The problem I'm running into is that blink_platform depends on //services/network/public/interfaces but, due to the type mapping mentioned above, //services/network/public/interfaces ends up depending on blink_platform, which has the KURL definition. The symptom of the problem is the build error below.

I think it's reasonable to expect that the network service would use a url.mojom.Url quite a bit, and imagine that other service would want to use this too. Can you please give some advice for navigating this situation?

Relevant build error snippet:

29716/33629] LINK(DLL) blink_platform.dll blink_platform.dll.lib blink_platform.dll.pdb FAILED: blink_platform.dll blink_platform.dll.lib blink_platform.dll.pdb E:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False link.exe /nologo /IMPLIB:./blink_platform.dll.lib /DLL /OUT:./blink_platform.dll /PDB:./blink_platform.dll.pdb @./blink_platform.dll.rsp cookie_manager.mojom-blink.obj : warning LNK4217: locally defined symbol ??0KURL@blink@@QAE@XZ (public: __thiscall blink::KURL::KURL(void)) imported in function "public: static bool __cdecl network::mojom::blink::CookieManagerStubDispatch::Accept(class network::mojom::blink::CookieManager *,class mojo::Message *)" (?Accept@CookieManagerStubDispatch@blink@mojom@network@@SA_NPAVCookieManager@234@PAVMessage@mojo@@@Z) LINK : error LNK1218: warning treated as error; no output file generated [29717/33629] LINK remoting_unittests.exe remoting_unittests.exe.pdb ninja: build stopped: subcommand failed. step returned non-zero exit code: 1

Full build error log: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin_chromium_compile_dbg_ng%2F504928%2F%2B%2Frecipes%2Fsteps%2Fcompile__with_patch_%2F0%2Fstdout

Thank you very much!

    Victor

[Colin Blundell]

This seems like the same high-level issue being discussed here, unless I'm missing something. Maybe join on to that thread?

--
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 post to this group, send email to chromi...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAP_mGKrcJjrhXTnTkN6_OF82%2Bu5N09p1hStnO%3D8K1Ov%3Do9iGsA%40mail.gmail.com.

[Daniel Cheng]

For now, I think you'll need to play games with overridden_deps_blink [1]: it sounds like what's happening is the header is being included as-if it were dllimported when using the generated mojom from within Blink--but since the Blink variant of the mojom and KURL are built in the same DLL, the compiler complains.

I did manage to split apart KURL into a separate platform library, but the split leaves something to be desired: it ends up grouping together a rather arbitrary collection of things and requires adding yet another *_EXPORT macro (we could use source sets to work around this but I'm a bit wary of ending up with accidentally duplicated globals).

Daniel

[1] https://cs.chromium.org/chromium/src/components/pdf/common/BUILD.gn?rcl=57a3a0cae00854761d9d838a07c2ba71d8a29928&l=21

[Colin Blundell]

adding in Renjie.

I suspect that the solution here was incomplete, as it seems like we're tickling a bug related to this. Renjie's CL is adding a dependency from //services/device/public/interfaces to //services/network/public/interfaces. //services/device/public/interfaces is depended on by Blink. His CL is hitting a linker error on Windows, sample:

 error LNK2005: "public: class WTF::String const & __thiscall blink::KURL::GetString(void)const " (?GetString@KURL@blink@@QBEABVString@WTF@@XZ) already defined in cookie_manager.mojom-blink.obj

Daniel, am I correct that this symptom is related to the same (or a very similar) root cause? Do you have a suggested solution?

Thanks,

Colin

[cos...@google.com]

IMHO the "right" solution is to figure out the layering and build for services and components.

Relevant thread: https://groups.google.com/a/chromium.org/d/topic/platform-architecture-dev/96-wtfa9ckA/discussion

Until then, you're stuck trying various hacks until things build. I spent a few hours with a Windows Z840 to figure out my hacks.

I hope this helps,

    Victor

[Renjie Liu]

Thanks for the help! I will try if there're any other options. :)

[Colin Blundell]

Let's wait for Daniel to weigh in...He's thought about these challenges the most AFAIK.

[Colin Blundell]

+Sam McNally 

On a different thread, Sam proposed the below fix, which seems to work (thanks, Sam!).

Sam (or anyone else), is there some underlying logic behind this fix that we can document somewhere and point people to when they hit these kinds of errors? It seems like the fundamental fix of splitting Platform up into a bunch of more granular targets isn't going to be immediate.

Thanks,

Colin

From Sam:

The right way to fix this is to split KURL and SecurityOrigin from the platform target so dependencies can be correctly specified. In the meantime, applying the following seems to avoid the problem for me:

diff --git a/services/device/public/interfaces/BUILD.gn b/services/device/public/interfaces/BUILD.gn

index 4904b9db9d9d..bc08e024c4fa 100644

--- a/services/device/public/interfaces/BUILD.gn

+++ b/services/device/public/interfaces/BUILD.gn

@@ -31,6 +31,9 @@ mojom("interfaces") {

   public_deps = [

     ":constants",

   ]

+

+  overridden_deps_blink = [ "//services/network/public/interfaces:interfaces" ]

+  component_deps_blink = [ "//third_party/WebKit/Source/platform" ]

 }

 mojom("generic_sensor") {

diff --git a/third_party/WebKit/Source/platform/BUILD.gn b/third_party/WebKit/Source/platform/BUILD.gn

index 4ed2ad45be4a..837a121f2ca9 100644

--- a/third_party/WebKit/Source/platform/BUILD.gn

+++ b/third_party/WebKit/Source/platform/BUILD.gn

@@ -223,6 +223,7 @@ jumbo_component("platform") {

   visibility = []  # Allow re-assignment of list.

   visibility = [

     "//components/pdf/common:interfaces_blink",

+    "//services/device/public/interfaces:interfaces_blink",

     "//third_party/WebKit/*",

     "//url/mojo:url_mojom_origin_blink",

     "//url/mojo:url_mojom_gurl_blink",

[Renjie Liu]

Thanks to Sam for the help! :D

[Daniel Cheng]

On Mon, Dec 4, 2017 at 12:19 AM Colin Blundell <blun...@chromium.org> wrote:

+Sam McNally 

On a different thread, Sam proposed the below fix, which seems to work (thanks, Sam!).

Sam (or anyone else), is there some underlying logic behind this fix that we can document somewhere and point people to when they hit these kinds of errors? It seems like the fundamental fix of splitting Platform up into a bunch of more granular targets isn't going to be immediate.

Yes, I've tried to extract a platform_base target from platform but it's non-trivial: https://chromium-review.googlesource.com/c/chromium/src/+/681616. I think that's probably the simplest fix that's somewhat comprehensive for now. But there's still quite a few loose ends to wrap up in that CL... if someone's interested in picking this patch up, please feel free to adopt it.

Daniel

[Sam McNally]

The strategy to follow for dealing with components and mojom targets is similar to the one for dealing with components and source_sets or static_libraries (when the dependency is in a different component, depend on that component instead). For mojom targets, this means using overridden_deps(_blink) to remove the source_set/static_library dependency and component_deps(_blink) to add the component dependency. The export_* parameters for mojoms can provide information about which component a mojom lives in.

In this example, the export_define_blink = "BLINK_PLATFORM_IMPLEMENTATION=1"

 in //services/network/public/interfaces tells us that the blink variant lives in the component that sets that define for its sources. Codesearch finds the same define being set in //third_party/WebKit/Source/platform:blink_platform_implementation, used by the component //third_party/WebKit/Source/platform. Thus, we can use overridden_deps_blink to remove the cross-component dependency and component_deps_blink to add the dependency on the containing component.

[Renjie Liu]

Thank you for the clarification. 

[Colin Blundell]

Thanks Sam, that's useful!