osmesa, cdmadapter: drop workaround for VS2013 allocator shim

osmesa, cdmadapter: drop workaround for VS2013 allocator shim

This is a partial revert of crrev.com/1616793003 .

crrev.com/1616793003 introduced a workaround to the clearkeycdmadapter
and osmesa libs to deal with a breakage on windows. The breakage was
because in the days of MSVS2013 and gyp the win shim was working by:
1. removing the dependency to libcrt from any target that would depend
   directly or indirectly on //base.
2. giving them a replacement shim which did redefine allocator syms.

However, all this was working only in the case that the target
was ending up linking //base, which is the case for *almost* any target
in chrome. It was known to break if we had a case of a target depending
on //base only via a shared library dependency (so depending on, but not
linking, //base).
In such case only 1 would happen but not 2, leading to a build failure
on win, as highlighted in the commit message of crrev.com/1616793003 .
We estimated this to be very rare in the codebase, and effectively did
affect only osmesa and cdmadaptr, so we added a workaround there.

Now that both gyp and MSVS2013 are no more, we can go back to the
more sensible case where only the root executable (chrome, or whatever
else executable linking base) is the only thing defining the allocator
symbols. No workaround should be needed anymore.

BUG=617732

[Primiano Tucci (use gerrit), 2016-10-18 08:01:49]

The CQ bit was checked by primiano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 08:02:02]

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

[Primiano Tucci (use gerrit), 2016-10-18 08:03:04]

Description was changed from

==========
allocator: drop workaround for MSVS2013 shim.

This is a partial revert of crrev.com/1616793003 .

crrev.com/1616793003 introduced a workaround to the clearkeycdmadapter
and osmesa libs to deal with a breakage on windows. The breakage was
because in the days of MSVS2013 and gyp the win shim was working by:
1. removing the dependency to libcrt from any target that would depend
   directly or indirectly on //base.
2. giving them a replacement shim which did redefine allocator syms.

However, all this was working only in the case that the target
was ending up linking //base, which is the case for *almost* any target
in chrome. It was known to break if we had a case of a target depending
on //base only via a shared library dependency (so depending on, but not
linking, //base).
In such case only 1 would happen but not 2, leading to a build failure
on win, as highlighted in the commit message of crrev.com/1616793003 .
We estimated this to be very rare in the codebase, and effectively did
affect only osmesa and cdmadaptr, so we added a workaround there.

Now that both gyp and MSVS2013 are no more, we can go back to the
more sensible case where only the root executable (chrome, or whatever
else executable linking base) is the only thing defining the allocator
symbols. No workaround should be needed anymore.

BUG=617732
==========

to

==========
osmesa, cdmadapter: drop workaround for VS2013 allocator shim

This is a partial revert of crrev.com/1616793003 .

crrev.com/1616793003 introduced a workaround to the clearkeycdmadapter
and osmesa libs to deal with a breakage on windows. The breakage was
because in the days of MSVS2013 and gyp the win shim was working by:
1. removing the dependency to libcrt from any target that would depend
   directly or indirectly on //base.
2. giving them a replacement shim which did redefine allocator syms.

However, all this was working only in the case that the target
was ending up linking //base, which is the case for *almost* any target
in chrome. It was known to break if we had a case of a target depending
on //base only via a shared library dependency (so depending on, but not
linking, //base).
In such case only 1 would happen but not 2, leading to a build failure
on win, as highlighted in the commit message of crrev.com/1616793003 .
We estimated this to be very rare in the codebase, and effectively did
affect only osmesa and cdmadaptr, so we added a workaround there.

Now that both gyp and MSVS2013 are no more, we can go back to the
more sensible case where only the root executable (chrome, or whatever
else executable linking base) is the only thing defining the allocator
symbols. No workaround should be needed anymore.

BUG=617732
==========

[Primiano Tucci (use gerrit), 2016-10-18 08:03:04]

primiano@chromium.org changed reviewers:
+ dalecurtis@chromium.org, kbr@chromium.org

[Primiano Tucci (use gerrit), 2016-10-18 08:05:50]

+wfh FYI

[commit-bot: I haz the power, 2016-10-18 08:05:51]

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

[commit-bot: I haz the power, 2016-10-18 08:05:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Primiano Tucci (use gerrit), 2016-10-18 08:31:04]

The CQ bit was checked by primiano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 08:31:46]

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

[Primiano Tucci (use gerrit), 2016-10-18 08:34:21]

primiano@chromium.org changed reviewers:
+ xhwang@chromium.org

[Primiano Tucci (use gerrit), 2016-10-18 08:34:21]

Dale/xhwang, adding a bunch of extra nognchecks because:
after I added that //base dependency new code that requires trivial base headers
was added. My //base dep there masked those #include checks from gn.
Since these are trivial headers that don't require any link time dependency on
base there are two ways around this:

1) copy those headers in cdm, similary to what you people did with cdm_logging
2) just use //gnnocheck for those trivial headers.

There seems to be precendent within the same folder for //nogncheck so I just
went for that road.

[commit-bot: I haz the power, 2016-10-18 08:40:52]

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

[commit-bot: I haz the power, 2016-10-18 08:40:53]

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

[xhwang, 2016-10-18 17:01:13]

cdm lgtm

[Ken Russell (switch to Gerrit), 2016-10-18 18:30:18]

lgtm

[Will Harris, 2016-10-18 18:33:52]

the vs2015 shim still overrides the allocator functions but relies on link time
ordering rather than stripping them from the ucrt.

do the source files here have target that is its own DLL or EXE, and do they
include any third party code? If so it would be nice to actually check that they
are correctly importing the shim somewhere in the main() path.

[Primiano Tucci (use gerrit), 2016-12-13 20:20:34]

On 2016/10/18 18:33:52, Will Harris wrote:
> the vs2015 shim still overrides the allocator functions but relies on link
time
> ordering rather than stripping them from the ucrt.
> 
> do the source files here have target that is its own DLL or EXE, and do they
> include any third party code? If so it would be nice to actually check that
they
> are correctly importing the shim somewhere in the main() path.

Apologies I completely forgot about this.

kbr@ correct me if I am wrong but the osmesa target is some sw emulation that we
don't really ship and use only for tests, right?

xhwang: is clearkeycdmadapter.so something that we ship for real?

[Ken Russell (switch to Gerrit), 2016-12-13 21:35:19]

On 2016/12/13 20:20:34, Primiano Tucci wrote:
> On 2016/10/18 18:33:52, Will Harris wrote:
> > the vs2015 shim still overrides the allocator functions but relies on link
> time
> > ordering rather than stripping them from the ucrt.
> > 
> > do the source files here have target that is its own DLL or EXE, and do they
> > include any third party code? If so it would be nice to actually check that
> they
> > are correctly importing the shim somewhere in the main() path.
> 
> Apologies I completely forgot about this.
> 
> kbr@ correct me if I am wrong but the osmesa target is some sw emulation that
we
> don't really ship and use only for tests, right?

That's correct.

> xhwang: is clearkeycdmadapter.so something that we ship for real?