Componentize zombies to silence ObjC runtime errors in component builds

Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both /b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and /b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both /b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and /b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
TEST=Build Chromium for Mac with gn and `is_component_build = true`, run it from a terminal, and look for warnings like the above.

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

[Sidney San Martín, 2016-07-13 16:38:55]

Description was changed from

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
==========

to

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
==========

[Sidney San Martín, 2016-07-13 16:38:55]

sdy@chromium.org changed reviewers:
+ rsesek@chromium.org

[Sidney San Martín, 2016-07-13 16:43:38]

Description was changed from

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
==========

to

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
TEST=Build Chromium for Mac with gn and `is_component_build = true`, run it from
a terminal, and look for warnings like the above.
==========

[Sidney San Martín, 2016-07-13 16:45:23]

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/DEPS
File components/crash/core/DEPS (right):

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/DEPS#...
components/crash/core/DEPS:5: "+components/crash",
I'm not sure if this is the right move here — what do you think?

[Robert Sesek, 2016-07-13 20:40:58]

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/DEPS
File components/crash/core/DEPS (right):

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/DEPS#...
components/crash/core/DEPS:5: "+components/crash",
On 2016/07/13 16:45:23, Sidney San Martín wrote:
> I'm not sure if this is the right move here — what do you think?

I don't see any other layered components (those with a core/ and content/
directory
http://dev.chromium.org/developers/design-documents/layered-components-design)
with _export macros.

So let's move the crash_export.h file closer to where it's required, and that
obviate this. Move components/crash/crash_export.h into
components/crash/core/common.

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/commo...
File components/crash/core/common/BUILD.gn (right):

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/commo...
components/crash/core/common/BUILD.gn:28: sources = [
List the crash_export.h here, too, after it is moved.

[Sidney San Martín, 2016-07-13 21:11:53]

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/DEPS
File components/crash/core/DEPS (right):

https://codereview.chromium.org/2148803003/diff/1/components/crash/core/DEPS#...
components/crash/core/DEPS:5: "+components/crash",
On 2016/07/13 20:40:58, Robert Sesek wrote:
> On 2016/07/13 16:45:23, Sidney San Martín wrote:
> > I'm not sure if this is the right move here — what do you think?
> 
> I don't see any other layered components (those with a core/ and content/
> directory
> http://dev.chromium.org/developers/design-documents/layered-components-design)
> with _export macros.
> 
> So let's move the crash_export.h file closer to where it's required, and that
> obviate this. Move components/crash/crash_export.h into
> components/crash/core/common.

Done.

[Robert Sesek, 2016-07-13 22:15:14]

Nice CL description, btw.

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
File components/crash/core/common/BUILD.gn (right):

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
components/crash/core/common/BUILD.gn:20: 
Maybe set visibility = [ ":common" ] too in both the new targets.

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
components/crash/core/common/BUILD.gn:28: sources = [
List the crash_export.h here.

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
components/crash/core/common/BUILD.gn:33: defines = [
"COMPONENTS_CRASH_CORE_COMMON_CRASH_IMPLEMENTATION" ]
If you wanted a shorter string, CRASH_CORE_COMMON_IMPLEMENTATION is fine.

[Sidney San Martín, 2016-07-13 22:29:23]

On 2016/07/13 22:15:14, Robert Sesek wrote:
> Nice CL description, btw.

Thanks!

[Sidney San Martín, 2016-07-13 22:32:47]

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
File components/crash/core/common/BUILD.gn (right):

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
components/crash/core/common/BUILD.gn:20: 
On 2016/07/13 22:15:14, Robert Sesek wrote:
> Maybe set visibility = [ ":common" ] too in both the new targets.

Done. (Using ':*' instead as discussed offline.)

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
components/crash/core/common/BUILD.gn:28: sources = [
On 2016/07/13 22:15:14, Robert Sesek wrote:
> List the crash_export.h here.

Done.

https://codereview.chromium.org/2148803003/diff/20001/components/crash/core/c...
components/crash/core/common/BUILD.gn:33: defines = [
"COMPONENTS_CRASH_CORE_COMMON_CRASH_IMPLEMENTATION" ]
On 2016/07/13 22:15:14, Robert Sesek wrote:
> If you wanted a shorter string, CRASH_CORE_COMMON_IMPLEMENTATION is fine.

Done.

[Robert Sesek, 2016-07-13 22:38:51]

LGTM

[Sidney San Martín, 2016-07-13 22:40:34]

The CQ bit was checked by sdy@chromium.org

[commit-bot: I haz the power, 2016-07-13 22:41:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 23:53:45]

Description was changed from

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
TEST=Build Chromium for Mac with gn and `is_component_build = true`, run it from
a terminal, and look for warnings like the above.
==========

to

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
TEST=Build Chromium for Mac with gn and `is_component_build = true`, run it from
a terminal, and look for warnings like the above.
==========

[commit-bot: I haz the power, 2016-07-13 23:53:46]

Committed patchset #3 (id:40001)

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

CQ bit was unchecked.

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

Description was changed from

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
TEST=Build Chromium for Mac with gn and `is_component_build = true`, run it from
a terminal, and look for warnings like the above.
==========

to

==========
Componentize zombies to silence ObjC runtime errors in component builds

With //components/crash/core/common as a static library, its classes can
end up in multiple components and frustrate the ObjC runtime:

    objc[82840]: Class CrZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.
    objc[82840]: Class CrFatZombie is implemented in both
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/./libsessions.dylib and
/b/swarm_slave/work/isolated/runpsh5q4/out/Debug/libchrome_dll.dylib. One of the
two will be used. Which one is undefined.

Making the implementation its own component stops this.

(rsesek@chromium.org did all of the thinking on this — my main
contribution was getting tired enough of the warnings to grab him.)

BUG=611894
TEST=Build Chromium for Mac with gn and `is_component_build = true`, run it from
a terminal, and look for warnings like the above.

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

[commit-bot: I haz the power, 2016-07-13 23:56:07]

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