Fold //ipc/mojo into //ipc.

Fold //ipc/mojo into //ipc.

BUG=604282
NOPRESUBMIT=true

Committed: https://crrev.com/d4aa70da301b278a8713c43a0d8bbf9b75ef6981
Cr-Commit-Position: refs/heads/master@{#401559}

[Anand Mistry (off Chromium), 2016-06-15 04:15:01]

amistry@chromium.org changed reviewers:
+ jam@chromium.org, rockot@chromium.org, tsepez@chromium.org

[Anand Mistry (off Chromium), 2016-06-15 04:15:02]

tsepez@chromium.org: For security and IPC owner.
jam@chromium.org: For everything owners, including IPC.
rockot@: Well, duh!

I realise this dups part of https://codereview.chromium.org/2033243003/ and will
rebase once that's submitted.

I think we're at the point where it no longer makes sense to have ChannelMojo be
a separate thing from IPC. The Cl link above will make Mojo a hard dependency of
IPC, and there are changes I'd like to make to IPC (i.e. adding message pipe
handle to ChannelHandle) to help migrate everything to ChannelMojo and
eventually delete the platform-specific IPC channel implementations.

[jam, 2016-06-15 18:57:31]

lgtm

https://codereview.chromium.org/2069803003/diff/20001/ipc/mojo/BUILD.gn
File ipc/mojo/BUILD.gn (right):

https://codereview.chromium.org/2069803003/diff/20001/ipc/mojo/BUILD.gn#newcode9
ipc/mojo/BUILD.gn:9: "scoped_ipc_support.cc",
why are you leaving these two files here instead of moving them to ipc/?

seems like these two should be moved and then the directory could be removed

also to avoid losing owners status, you can move yourself/rockot to ipc/owners
(the other owners in /mojo/owners don't work on the project anymore). ipc/owners
needs some more since cpu doesn't work on the project, and darin isn't going to
have time to review changes there (and he's already top level owner)

[Anand Mistry (off Chromium), 2016-06-16 00:24:29]

https://codereview.chromium.org/2069803003/diff/20001/ipc/mojo/BUILD.gn
File ipc/mojo/BUILD.gn (right):

https://codereview.chromium.org/2069803003/diff/20001/ipc/mojo/BUILD.gn#newcode9
ipc/mojo/BUILD.gn:9: "scoped_ipc_support.cc",
On 2016/06/15 18:57:30, jam wrote:
> why are you leaving these two files here instead of moving them to ipc/?
> 
> seems like these two should be moved and then the directory could be removed

This class has nothing to do with legacy IPC and doesn't belong in //ipc. It
also has nothing to do with ChannelMojo so it really doesn't belong here either.
I'll move it to a more appropriate location in a separate CL (which will
probably land before this).

> 
> also to avoid losing owners status, you can move yourself/rockot to ipc/owners
> (the other owners in /mojo/owners don't work on the project anymore).
ipc/owners
> needs some more since cpu doesn't work on the project, and darin isn't going
to
> have time to review changes there (and he's already top level owner)

Done.

[Ken Rockot(use gerrit already), 2016-06-16 00:30:22]

Agree it'd be nice if ScopedIPCSupport just moved out of //ipc. I think
mojo/edk/embedder makes sense as a home. And could we then remove the one in
testing?

Anyway lgtm

[Anand Mistry (off Chromium), 2016-06-16 00:40:13]

On 2016/06/16 00:30:22, Ken Rockot wrote:
> Agree it'd be nice if ScopedIPCSupport just moved out of //ipc. I think
> mojo/edk/embedder makes sense as a home.

I'm doing that at this very moment.

> And could we then remove the one in
> testing?

I think there's some additional shutdown stuff in the test ipc support, but it
should be movable.


> Anyway lgtm

[dcheng, 2016-06-23 00:51:11]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-06-23 00:51:13]

RS LGTM for mojom and ParamTraits changes.

[Anand Mistry (off Chromium), 2016-06-23 03:24:59]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2016-06-23 03:25:00]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
rockot@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2069803003/#ps220001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-23 03:25:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2069803003/220001

[commit-bot: I haz the power, 2016-06-23 03:33:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-23 03:33:28]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Anand Mistry (off Chromium), 2016-06-23 06:48:13]

Description was changed from

==========
Fold //ipc/mojo into //ipc.

BUG=604282
==========

to

==========
Fold //ipc/mojo into //ipc.

BUG=604282
NOPRESUBMIT=true
==========

[Anand Mistry (off Chromium), 2016-06-23 06:48:40]

Skipping presubmit due to crbug.com/622569

[Anand Mistry (off Chromium), 2016-06-23 06:48:50]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2016-06-23 06:48:51]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
jam@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2069803003/#ps240001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-23 06:49:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2069803003/240001

[commit-bot: I haz the power, 2016-06-23 07:52:59]

Description was changed from

==========
Fold //ipc/mojo into //ipc.

BUG=604282
NOPRESUBMIT=true
==========

to

==========
Fold //ipc/mojo into //ipc.

BUG=604282
NOPRESUBMIT=true
==========

[commit-bot: I haz the power, 2016-06-23 07:53:01]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-06-23 07:54:21]

Description was changed from

==========
Fold //ipc/mojo into //ipc.

BUG=604282
NOPRESUBMIT=true
==========

to

==========
Fold //ipc/mojo into //ipc.

BUG=604282
NOPRESUBMIT=true

Committed: https://crrev.com/d4aa70da301b278a8713c43a0d8bbf9b75ef6981
Cr-Commit-Position: refs/heads/master@{#401559}
==========

[commit-bot: I haz the power, 2016-06-23 07:54:23]

Patchset 13 (id:??) landed as
https://crrev.com/d4aa70da301b278a8713c43a0d8bbf9b75ef6981
Cr-Commit-Position: refs/heads/master@{#401559}

[Nico, 2016-06-23 15:01:04]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-06-23 15:01:06]

This breaks debug component builds with clang/win:
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%28dbg%29/builds...

C:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-with-manifests
environment.x86 True ipc_tests.exe "C:\b\depot_tools\python276_bin\python.exe
gyp-win-tool link-wrapper environment.x86 False link.exe /nologo
/OUT:ipc_tests.exe @ipc_tests.exe.rsp" 1 mt.exe rc.exe
"obj\ipc\ipc_tests.ipc_tests.exe.intermediate.manifest"
obj\ipc\ipc_tests.ipc_tests.exe.generated.manifest
..\..\build\win\compatibility.manifest
LINK : ipc_tests.exe not found or not built by the last incremental link;
performing full link

ipc_tests.ipc_mojo_bootstrap_unittest.obj : error LNK2019: unresolved external
symbol "public: __thiscall
mojo::ScopedInterfaceEndpointHandle::~ScopedInterfaceEndpointHandle(void)"
(??1ScopedInterfaceEndpointHandle@mojo@@QAE@XZ) referenced in function "public:
__thiscall mojo::AssociatedInterfacePtrInfo<class
IPC::mojom::Channel>::~AssociatedInterfacePtrInfo<class
IPC::mojom::Channel>(void)"
(??1?$AssociatedInterfacePtrInfo@VChannel@mojom@IPC@@@mojo@@QAE@XZ)

ipc_tests.exe : fatal error LNK1120: 1 unresolved externals

I guess something's missing an export somewhere. Can you take a look, please?

[Nico, 2016-06-23 15:02:49]

Oh, broken in release component builds too:
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dll%29/buil...

I think it's broken only in gyp builds, not in gn builds. So I guess it's not a
missing _EXPORT but some missed change in a gyp file. Anyhow, please take a look
:-)

[Ken Rockot(use gerrit already), 2016-06-23 15:04:58]

I'll fix this

On Thu, Jun 23, 2016 at 8:02 AM, <thakis@chromium.org> wrote:

> Oh, broken in release component builds too:
>
>
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dll%29/buil...
>
> I think it's broken only in gyp builds, not in gn builds. So I guess it's
> not a
> missing _EXPORT but some missed change in a gyp file. Anyhow, please take
> a look
> :-)
>
> https://codereview.chromium.org/2069803003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Anand Mistry (off Chromium), 2016-06-23 22:55:50]

Thanks Ken for fixing this.

On 24 June 2016 at 01:04, Ken Rockot <rockot@chromium.org> wrote:

> I'll fix this
>
> On Thu, Jun 23, 2016 at 8:02 AM, <thakis@chromium.org> wrote:
>
>> Oh, broken in release component builds too:
>>
>>
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dll%29/buil...
>>
>> I think it's broken only in gyp builds, not in gn builds. So I guess it's
>> not a
>> missing _EXPORT but some missed change in a gyp file. Anyhow, please take
>> a look
>> :-)
>>
>> https://codereview.chromium.org/2069803003/
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.