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

Issue 734791 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 1
Type: Bug

Blocked on:
issue 740791
issue 754029

Blocking:
issue 707031



Sign in to add a comment

Port //ipc to Fuchsia

Project Member Reported by scottmg@chromium.org, Jun 19 2017

Issue description

One of the next levels up. Has ipc_tests so can figure out if it's mostly working and get it on the bot.
 
apparently depends on boringssl

[15->246/283 ~23] CC obj/third_party/boringssl/boringssl/fuchsia.o
FAILED: obj/third_party/boringssl/boringssl/fuchsia.o 
/home/sgraham/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF obj/third_party/boringssl/boringssl/fuchsia.o.d -DV8_DEPRECATION_WARNINGS -DDCHECK_ALWAYS_ON=1 -DNO_TCMALLOC -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"305489-1\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DSYSROOT_VERSION=5b6bc0cc84b7962fa04c3b5c215115cb58369177 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBORINGSSL_IMPLEMENTATION -DBORINGSSL_NO_STATIC_INITIALIZER -DOPENSSL_SMALL -D_XOPEN_SOURCE=700 -DOPENSSL_NO_ASM -I../.. -Igen -I../../third_party/boringssl/src/include -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -funwind-tables -fPIC -pipe -fcolor-diagnostics --target=x86_64-fuchsia -m64 -march=x86-64 -O2 -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -g2 --sysroot=../../third_party/fuchsia-sdk/sysroot/x86_64-fuchsia -fvisibility=hidden -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Werror -Wall -Wno-unused-variable -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -std=c99 -c ../../third_party/boringssl/src/crypto/rand_extra/fuchsia.c -o obj/third_party/boringssl/boringssl/fuchsia.o
../../third_party/boringssl/src/crypto/rand_extra/fuchsia.c:35:19: error: use of undeclared identifier 'NO_ERROR'
    if (status != NO_ERROR) {

that's been fixed in doing  crbug.com/731302 
- sizeof(long) == sizeof(int64_t) which ipc seems against
- ipc_tests use a SyncSocket a bunch, which I thought (https://chromium-review.googlesource.com/536239) we wouldn't need, but maybe I was wrong and we'll have to implement/emulate that somehow.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 22 2017

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

commit f4b38b4c51515572bf46923654402e5fba86e1a6
Author: Scott Graham <scottmg@chromium.org>
Date: Thu Jun 22 23:50:08 2017

fuchsia: get //ipc compiling

Links but 25/80 tests crash.

Bug:  734791 
Change-Id: I0e53a60c33ec016bb051b661eaa398d246b6d5eb
Reviewed-on: https://chromium-review.googlesource.com/540288
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481724}
[modify] https://crrev.com/f4b38b4c51515572bf46923654402e5fba86e1a6/ipc/BUILD.gn
[modify] https://crrev.com/f4b38b4c51515572bf46923654402e5fba86e1a6/ipc/ipc_message_utils.h

Cc: jam...@chromium.org
Jamesr is haxing up a socketpair() on the Fuchsia side which will get a bunch of stuff working here (incl. probably ipc/sync_socket_unittest.cc too, which I compiled out before.)
Cc: w...@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 30 2017

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

commit 3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf
Author: Scott Graham <scottmg@chromium.org>
Date: Fri Jun 30 01:07:10 2017

fuchsia: get ipc_tests compiling again

The broke when the message loop file descriptor stuff was removed, and
some mojo things were refactored.

handle_fuchsia.* and handle_attachment_fuchsia.* are just the
_win versions with s/HANDLE/mx_handle_t/.

Adds .filter file for 54/80 tests that pass, so this implementation
undoubtedly needs work, but can be sharded better once this lands.

Bug:  734791 
Change-Id: If9c506972e1d70119f9d20a66108902ee2fbfd16
Reviewed-on: https://chromium-review.googlesource.com/557100
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483568}
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/BUILD.gn
[add] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/handle_attachment_fuchsia.cc
[add] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/handle_attachment_fuchsia.h
[add] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/handle_fuchsia.cc
[add] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/handle_fuchsia.h
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/ipc.mojom
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/ipc_message_attachment_set.cc
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/ipc/ipc_message_utils.cc
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/mojo/edk/embedder/platform_channel_utils_posix.cc
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/mojo/edk/embedder/platform_shared_buffer.cc
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/mojo/public/c/system/platform_handle.h
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/mojo/public/cpp/system/platform_handle.cc
[modify] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/mojo/public/cpp/system/platform_handle.h
[add] https://crrev.com/3eebff0fa6e10cfc7f8b5529bc103c1f8f769ddf/testing/buildbot/filters/fuchsia.ipc_tests.filter

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 30 2017

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

commit 904b494651543370dfac88ca7424c84796bd2dd0
Author: Scott Graham <scottmg@chromium.org>
Date: Fri Jun 30 01:24:54 2017

fuchsia: run ipc_tests (filtered) on bots too

Bug:  734791 
Change-Id: Iea06df3003ace8d41478a9f33fd0ac8f982f140a
Reviewed-on: https://chromium-review.googlesource.com/557461
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483578}
[modify] https://crrev.com/904b494651543370dfac88ca7424c84796bd2dd0/testing/buildbot/chromium.fyi.json

Was just looking at IPCFuzzingTest.SanityTest as an example disabled one. It's failing to start the child server for multiprocess tests (and so hanging) because it gets mucked up in the handle passing. It's possible that I was a bit aggressive in not passing fds through? or otherwise we need to add a handles_to_inherit like windows to pass those through.

Per dicussion on irc, I think wez is already looking at implementing PlatformChannelPairFuchsia (or making Posix do a thing) so will ignore for now.
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 13 2017

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

commit a98ff6b50930e5fda046942cce1d8d3ff35f6e2e
Author: Sergey Ulanov <sergeyu@google.com>
Date: Thu Jul 13 01:54:20 2017

Define ParamTraits<long>::Log() on Fuchsia

Previously the function was declared, but not defined.

Bug:  734791 
Change-Id: I848bca19b1e79f33c8f4b796f5b340fdade315f9
Reviewed-on: https://chromium-review.googlesource.com/565796
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486216}
[modify] https://crrev.com/a98ff6b50930e5fda046942cce1d8d3ff35f6e2e/ipc/ipc_message_utils.cc

Comment 10 by w...@chromium.org, Jul 15 2017

Components: Internals>PlatformIntegration

Comment 11 by w...@chromium.org, Aug 3 2017

Blockedon: 740791

Comment 12 by w...@chromium.org, Aug 3 2017

Owner: ----
Status: Available (was: Started)

Comment 13 by w...@chromium.org, Aug 9 2017

Owner: w...@chromium.org
Status: Started (was: Available)

Comment 14 by w...@chromium.org, Aug 9 2017

Labels: -Pri-3 M-62 Pri-1

Comment 15 by w...@chromium.org, Aug 9 2017

Blockedon: 754029
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 10 2017

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

commit 8c49ebc982519843ed90a612e1dec68688617057
Author: Wez <wez@chromium.org>
Date: Thu Aug 10 17:11:13 2017

Implement most of Chrome IPC for Fuchsia.

- Clean up the Handle[Attachment]Fuchsia classes, removing unused fields.
- Add a missing OS_FUCHSIA case to ipc_message_utils.cc
- Update the test filters.

Bug:  734791 
Change-Id: I6ce0eb74166fbb40b16d9c77ebb782c514d3a160
Reviewed-on: https://chromium-review.googlesource.com/609384
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493436}
[modify] https://crrev.com/8c49ebc982519843ed90a612e1dec68688617057/ipc/handle_attachment_fuchsia.cc
[modify] https://crrev.com/8c49ebc982519843ed90a612e1dec68688617057/ipc/handle_attachment_fuchsia.h
[modify] https://crrev.com/8c49ebc982519843ed90a612e1dec68688617057/ipc/handle_fuchsia.cc
[modify] https://crrev.com/8c49ebc982519843ed90a612e1dec68688617057/ipc/handle_fuchsia.h
[modify] https://crrev.com/8c49ebc982519843ed90a612e1dec68688617057/ipc/ipc_message_utils.cc
[modify] https://crrev.com/8c49ebc982519843ed90a612e1dec68688617057/testing/buildbot/filters/fuchsia.ipc_tests.filter

Project Member

Comment 17 by bugdroid1@chromium.org, Aug 22 2017

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

commit da4409217a419c39b18bca164e9467960595a9c5
Author: Wez <wez@chromium.org>
Date: Tue Aug 22 22:54:31 2017

Handle multi-MX-handle file descriptors in Mojo messages on Fuchsia.

Previously the mojo::edk::ChannelFuchsia was hard-wired to cope only
with file-descriptors for simple files, which wrap a single underlying
platform handle, e.g. a virtual memory object containing the file data.

This CL extends the implementation to cope with MXIO file-descriptors
which build on multiple platform resources, notably device files such as
"/dev/zero". We make some assumptions about the contents of the |info|
returned with the unwrapped handles, so this may need revisiting in
future.

Bug:  734791 ,  740791 
Change-Id: I6aac82bb5b34995381d08bd5b0f8df44a156bc37
Reviewed-on: https://chromium-review.googlesource.com/624815
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496479}
[modify] https://crrev.com/da4409217a419c39b18bca164e9467960595a9c5/mojo/edk/system/channel.h
[modify] https://crrev.com/da4409217a419c39b18bca164e9467960595a9c5/mojo/edk/system/channel_fuchsia.cc
[modify] https://crrev.com/da4409217a419c39b18bca164e9467960595a9c5/testing/buildbot/filters/fuchsia.ipc_tests.filter

Comment 18 by w...@chromium.org, Aug 22 2017

Status: Fixed (was: Started)
Project Member

Comment 19 by bugdroid, Today (19 minutes ago)

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

commit 5ccc61c8c0517f74bc3dc33b48b958f6ed6bdd5f
Author: Wez <wez@chromium.org>
Date: Wed Jan 23 03:08:49 2019

Remove ChattyServer ipc_tests, which are slow & unnecessary.

These tests were added to guard against chatty synchronous IPC causing
Windows' PostMessage() limit being reached, and causing messages to be
dropped.

Since Chrome IPC now runs over Mojo, and our message pumps do not call
PostMessage() on each task, and these tests are slow and timeout on
Android & Fuchsia ARM64, just remove the tests.

Bug:  734791 , 339980
Change-Id: I26c105f3685995e24011bbc90ed9a391833ec4e2
Reviewed-on: https://chromium-review.googlesource.com/c/1424162
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625053}

Sign in to add a comment