Remove allocator_extension_thunks since this layer is not required

Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason. See design doc in the bug.

This CL removes the target and stores the functions in base.

Landed as https://codereview.chromium.org/1487403002/
BUG=564618

[ssid, 2015-12-02 12:12:37]

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

[ssid, 2015-12-02 12:12:38]

PTAL, Thanks.

[ssid, 2015-12-02 13:28:44]

Fixed bots now.

[ssid, 2015-12-02 13:32:29]

Description was changed from

==========
Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason.

This CL removes the target and stores the functions in base.

BUG=
==========

to

==========
Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason.

This CL removes the target and stores the functions in base.

BUG=564618
==========

[ssid, 2015-12-02 13:47:20]

Description was changed from

==========
Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason.

This CL removes the target and stores the functions in base.

BUG=564618
==========

to

==========
Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason. See design doc in the bug.

This CL removes the target and stores the functions in base.

BUG=564618
==========

[Primiano Tucci (use gerrit), 2015-12-02 13:50:41]

primiano@chromium.org changed reviewers:
+ thakis@chromium.org, wfh@chromium.org

[Primiano Tucci (use gerrit), 2015-12-02 13:50:42]

This CL overall looks good, adding +wfh and +thakis for an authoritative review.

In the meantime, base/trace_event/* LGTM
Thanks for starting this cleanup. It's going to be a long December.

https://codereview.chromium.org/1487403002/diff/40001/base/allocator/allocato...
File base/allocator/allocator_extension.cc (right):

https://codereview.chromium.org/1487403002/diff/40001/base/allocator/allocato...
base/allocator/allocator_extension.cc:13: ReleaseFreeMemoryFunction
g_release_free_memory_function = NULL;
s/NULL/nullptr/ since you are touching this file you get the honour of making it
C++11 :)

https://codereview.chromium.org/1487403002/diff/40001/base/allocator/allocato...
base/allocator/allocator_extension.cc:23: return g_get_numeric_property_function
!= NULL &&
s/!= NULL//

[ssid, 2015-12-02 14:03:33]

ssid@chromium.org changed reviewers:
+ jamesr@chromium.org

[ssid, 2015-12-02 14:03:34]

On 2015/12/02 13:50:42, Primiano Tucci wrote:
> This CL overall looks good, adding +wfh and +thakis for an authoritative
review.
> 
+jamesr who added this target.

> In the meantime, base/trace_event/* LGTM
> Thanks for starting this cleanup. It's going to be a long December.
> 

Just like the last December :)

https://codereview.chromium.org/1487403002/diff/40001/base/allocator/allocato...
File base/allocator/allocator_extension.cc (right):

https://codereview.chromium.org/1487403002/diff/40001/base/allocator/allocato...
base/allocator/allocator_extension.cc:13: ReleaseFreeMemoryFunction
g_release_free_memory_function = NULL;
On 2015/12/02 13:50:42, Primiano Tucci wrote:
> s/NULL/nullptr/ since you are touching this file you get the honour of making
it
> C++11 :)

Done.

https://codereview.chromium.org/1487403002/diff/40001/base/allocator/allocato...
base/allocator/allocator_extension.cc:23: return g_get_numeric_property_function
!= NULL &&
On 2015/12/02 13:50:42, Primiano Tucci wrote:
> s/!= NULL//

Done.

[Nico, 2015-12-02 14:21:31]

This lgtm (unless jamesr shouts). But please check that allocator_unittests
still builds and passes on Windows with this change as much as without it.

As far as I know, no bot builds or runs allocator_unittests. I tried to delete
them for that reason a while ago (https://codereview.chromium.org/922863002/)
but wfh said some of the tests are useful and he'd move them into base_unittests
(https://code.google.com/p/chromium/issues/detail?id=458309). Since that hasn't
happened yet, I suppose they aren't _that_ important and if they do cause issues
maybe we should just delete them this time round.

[ssid, 2015-12-02 14:31:24]

On 2015/12/02 14:21:31, Nico wrote:
> This lgtm (unless jamesr shouts). But please check that allocator_unittests
> still builds and passes on Windows with this change as much as without it.
> 
> As far as I know, no bot builds or runs allocator_unittests. I tried to delete
> them for that reason a while ago (https://codereview.chromium.org/922863002/)
> but wfh said some of the tests are useful and he'd move them into
base_unittests
> (https://code.google.com/p/chromium/issues/detail?id=458309). Since that
hasn't
> happened yet, I suppose they aren't _that_ important and if they do cause
issues
> maybe we should just delete them this time round.

Yes I wanted to remove them as well. But then left it since some tests might be
useful. It is missing in gn as well.
win_chromium_x64_rel_ng and win_chromium_rel_ng builds the test.
I cannot find any bot running that the test but it is being built since it is
added in target "All". I can't understand why i don't find logs of test being
run.
And yes, it builds with this change now and this was added since it was not
building when it was added.

[Nico, 2015-12-02 14:34:38]

Do they pass too?

On Wed, Dec 2, 2015 at 9:31 AM, <ssid@chromium.org> wrote:

> On 2015/12/02 14:21:31, Nico wrote:
>
>> This lgtm (unless jamesr shouts). But please check that
>> allocator_unittests
>> still builds and passes on Windows with this change as much as without it.
>>
>
> As far as I know, no bot builds or runs allocator_unittests. I tried to
>> delete
>> them for that reason a while ago (
>> https://codereview.chromium.org/922863002/)
>> but wfh said some of the tests are useful and he'd move them into
>>
> base_unittests
>
>> (https://code.google.com/p/chromium/issues/detail?id=458309). Since that
>>
> hasn't
>
>> happened yet, I suppose they aren't _that_ important and if they do cause
>>
> issues
>
>> maybe we should just delete them this time round.
>>
>
> Yes I wanted to remove them as well. But then left it since some tests
> might be
> useful. It is missing in gn as well.
> win_chromium_x64_rel_ng and win_chromium_rel_ng builds the test.
> I cannot find any bot running that the test but it is being built since it
> is
> added in target "All". I can't understand why i don't find logs of test
> being
> run.
> And yes, it builds with this change now and this was added since it was not
> building when it was added.
>
> https://codereview.chromium.org/1487403002/
>

-- 
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.

[Primiano Tucci (use gerrit), 2015-12-02 15:37:44]

On 2015/12/02 14:34:38, Nico wrote:
> Do they pass too?

O__o They are definitely not run (at least I couldn't find a bot which runs
them) which is the first scary thing.

On top of that, I'm confused infra-wise:
The target itself looks to be built (see [1]) but here I'm honestly a bit lost:
- allocator_unittests is mentioned only by gyp_remanining which in turn is
mentioned only by gyp_all
- I cannot understand what's the deal with gyp_all, nothing seems to refer to
that in cs.
- So I'd conclude that compile.py --all would just build "all" not "gyp_all",
which should not include allocator_unittest.
- As a matter of facts, however, I see allocator_unittests being built by:
  - This clobber bot [2], which invokes compile.py all
  - This win_clang_rel [1] here allocator_unittests is explicitly passed to the
compile.py cmdline. I cannot find anything in build (or the internal one)
mentioning allocator_unittests.

So now on top of being worried for that unittest not being run, I am confused to
see that allocator_unittest gets build at all. 
\_(ツ)_/¯


[1]
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang_rel/bu...
[2]
https://build.chromium.org/p/chromium/builders/Win/builds/37995/steps/compile...

[Nico, 2015-12-02 15:40:46]

As far as I know it's not run anywhere, yes (hence me wanting to delete
it). Since it doesn't run anywhere, I wouldn't be surprised if it's broken
-- but since one of the deleted lines in this CL says "this is for
allocator_unittest" it'd be good to check that this CL doesn't break it
more than it already is (and who knows, maybe the tests even pass -- it's a
tiny test binary after all)

On Wed, Dec 2, 2015 at 10:37 AM, <primiano@chromium.org> wrote:

> On 2015/12/02 14:34:38, Nico wrote:
>
>> Do they pass too?
>>
>
> O__o They are definitely not run (at least I couldn't find a bot which runs
> them) which is the first scary thing.
>
> On top of that, I'm confused infra-wise:
> The target itself looks to be built (see [1]) but here I'm honestly a bit
> lost:
> - allocator_unittests is mentioned only by gyp_remanining which in turn is
> mentioned only by gyp_all
> - I cannot understand what's the deal with gyp_all, nothing seems to refer
> to
> that in cs.
> - So I'd conclude that compile.py --all would just build "all" not
> "gyp_all",
> which should not include allocator_unittest.
> - As a matter of facts, however, I see allocator_unittests being built by:
>   - This clobber bot [2], which invokes compile.py all
>   - This win_clang_rel [1] here allocator_unittests is explicitly passed
> to the
> compile.py cmdline. I cannot find anything in build (or the internal one)
> mentioning allocator_unittests.
>
> So now on top of being worried for that unittest not being run, I am
> confused to
> see that allocator_unittest gets build at all.
> \_(ツ)_/¯
>
>
> [1]
>
>
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang_rel/bu...
> [2]
>
>
https://build.chromium.org/p/chromium/builders/Win/builds/37995/steps/compile...
>
> https://codereview.chromium.org/1487403002/
>

-- 
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.

[ssid, 2015-12-02 15:44:23]

On 2015/12/02 15:40:46, Nico wrote:
> As far as I know it's not run anywhere, yes (hence me wanting to delete
> it). Since it doesn't run anywhere, I wouldn't be surprised if it's broken
> -- but since one of the deleted lines in this CL says "this is for
> allocator_unittest" it'd be good to check that this CL doesn't break it
> more than it already is (and who knows, maybe the tests even pass -- it's a
> tiny test binary after all)
> 
> On Wed, Dec 2, 2015 at 10:37 AM, <mailto:primiano@chromium.org> wrote:
> 
> > On 2015/12/02 14:34:38, Nico wrote:
> >
> >> Do they pass too?
> >>
> >
> > O__o They are definitely not run (at least I couldn't find a bot which runs
> > them) which is the first scary thing.
> >
> > On top of that, I'm confused infra-wise:
> > The target itself looks to be built (see [1]) but here I'm honestly a bit
> > lost:
> > - allocator_unittests is mentioned only by gyp_remanining which in turn is
> > mentioned only by gyp_all
> > - I cannot understand what's the deal with gyp_all, nothing seems to refer
> > to
> > that in cs.
> > - So I'd conclude that compile.py --all would just build "all" not
> > "gyp_all",
> > which should not include allocator_unittest.
> > - As a matter of facts, however, I see allocator_unittests being built by:
> >   - This clobber bot [2], which invokes compile.py all
> >   - This win_clang_rel [1] here allocator_unittests is explicitly passed
> > to the
> > compile.py cmdline. I cannot find anything in build (or the internal one)
> > mentioning allocator_unittests.
> >
> > So now on top of being worried for that unittest not being run, I am
> > confused to
> > see that allocator_unittest gets build at all.
> > \_(ツ)_/¯
> >
> >
> > [1]
> >
> >
>
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang_rel/bu...
> > [2]
> >
> >
>
https://build.chromium.org/p/chromium/builders/Win/builds/37995/steps/compile...
> >
> > https://codereview.chromium.org/1487403002/
> >
> 
> -- 
> 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 mailto:chromium-reviews+unsubscribe@chromium.org.

I am sure it being built. Since
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang_rel/bui...
failed because of allocator_unittest.cc
I have started try job for this CL in win_clang bots. This should build
allocator_unittests.

[Primiano Tucci (use gerrit), 2015-12-02 15:48:46]

On 2015/12/02 15:40:46, Nico wrote:
> As far as I know it's not run anywhere, yes (hence me wanting to delete
> it). Since it doesn't run anywhere, I wouldn't be surprised if it's broken
> -- but since one of the deleted lines in this CL says "this is for
> allocator_unittest" it'd be good to check that this CL doesn't break it
> more than it already is (and who knows, maybe the tests even pass -- it's a
> tiny test binary after all)

So it looks to me that before this CL flies we have to first put
allocator_unittest in a state where it's either: running or not existent. I
understand now Nico's previous comment.
Would personally prefer the former (make them running).

I think the main issue here is that
https://codereview.chromium.org/922863002/#msg10
"I can move all these tests into base_unittests and remove the target once I'm
done." never happened (and crbug.com/458309 is still open)

ssid while you get for wfh review here can you revive allocator_unittests and
make sure they run on the bot, unless anybody disagrees here?
(I guess you want to do that in another CL to let it propagate to the various
fyi bot before than landing this)

[Nico, 2015-12-02 15:48:48]

Oh, the win_clang bots are special in that they build all targets -- most
other bots don't, and the win clang bots are only fyi bots. But you're
right, they check that the target builds fine. They don't tell you if the
tests pass when run though.

On Wed, Dec 2, 2015 at 10:44 AM, <ssid@chromium.org> wrote:

> On 2015/12/02 15:40:46, Nico wrote:
>
>> As far as I know it's not run anywhere, yes (hence me wanting to delete
>> it). Since it doesn't run anywhere, I wouldn't be surprised if it's broken
>> -- but since one of the deleted lines in this CL says "this is for
>> allocator_unittest" it'd be good to check that this CL doesn't break it
>> more than it already is (and who knows, maybe the tests even pass -- it's
>> a
>> tiny test binary after all)
>>
>
> On Wed, Dec 2, 2015 at 10:37 AM, <mailto:primiano@chromium.org> wrote:
>>
>
> > On 2015/12/02 14:34:38, Nico wrote:
>> >
>> >> Do they pass too?
>> >>
>> >
>> > O__o They are definitely not run (at least I couldn't find a bot which
>> runs
>> > them) which is the first scary thing.
>> >
>> > On top of that, I'm confused infra-wise:
>> > The target itself looks to be built (see [1]) but here I'm honestly a
>> bit
>> > lost:
>> > - allocator_unittests is mentioned only by gyp_remanining which in turn
>> is
>> > mentioned only by gyp_all
>> > - I cannot understand what's the deal with gyp_all, nothing seems to
>> refer
>> > to
>> > that in cs.
>> > - So I'd conclude that compile.py --all would just build "all" not
>> > "gyp_all",
>> > which should not include allocator_unittest.
>> > - As a matter of facts, however, I see allocator_unittests being built
>> by:
>> >   - This clobber bot [2], which invokes compile.py all
>> >   - This win_clang_rel [1] here allocator_unittests is explicitly passed
>> > to the
>> > compile.py cmdline. I cannot find anything in build (or the internal
>> one)
>> > mentioning allocator_unittests.
>> >
>> > So now on top of being worried for that unittest not being run, I am
>> > confused to
>> > see that allocator_unittest gets build at all.
>> > \_(ツ)_/¯
>> >
>> >
>> > [1]
>> >
>> >
>>
>
>
>
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang_rel/bu...
>
>> > [2]
>> >
>> >
>>
>
>
>
https://build.chromium.org/p/chromium/builders/Win/builds/37995/steps/compile...
>
>> >
>> > https://codereview.chromium.org/1487403002/
>> >
>>
>
> --
>> 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 mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> I am sure it being built. Since
>
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang_rel/bui...
> failed because of allocator_unittest.cc
> I have started try job for this CL in win_clang bots. This should build
> allocator_unittests.
>
> https://codereview.chromium.org/1487403002/
>

-- 
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.

[Nico, 2015-12-02 15:57:41]

I think it's much easier to move these (2 iirc? very few for sure) tests
into base_unittests instead of making sure the new test binary gets added
to all the bots that would have to run it...

On Wed, Dec 2, 2015 at 10:48 AM, <primiano@chromium.org> wrote:

> On 2015/12/02 15:40:46, Nico wrote:
>
>> As far as I know it's not run anywhere, yes (hence me wanting to delete
>> it). Since it doesn't run anywhere, I wouldn't be surprised if it's broken
>> -- but since one of the deleted lines in this CL says "this is for
>> allocator_unittest" it'd be good to check that this CL doesn't break it
>> more than it already is (and who knows, maybe the tests even pass -- it's
>> a
>> tiny test binary after all)
>>
>
> So it looks to me that before this CL flies we have to first put
> allocator_unittest in a state where it's either: running or not existent. I
> understand now Nico's previous comment.
> Would personally prefer the former (make them running).
>
> I think the main issue here is that
> https://codereview.chromium.org/922863002/#msg10
> "I can move all these tests into base_unittests and remove the target once
> I'm
> done." never happened (and crbug.com/458309 is still open)
>
> ssid while you get for wfh review here can you revive allocator_unittests
> and
> make sure they run on the bot, unless anybody disagrees here?
> (I guess you want to do that in another CL to let it propagate to the
> various
> fyi bot before than landing this)
>
> https://codereview.chromium.org/1487403002/
>

-- 
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.

[Primiano Tucci (use gerrit), 2015-12-02 15:59:11]

On 2015/12/02 15:57:41, Nico wrote:
> I think it's much easier to move these (2 iirc? very few for sure) tests
> into base_unittests instead of making sure the new test binary gets added
> to all the bots that would have to run it...

Yeah right, I think I actually meant: ssid can you plz take over
crbug.com/458309 as-is ?

[ssid, 2015-12-02 16:13:00]

On 2015/12/02 15:48:48, Nico wrote:
> Oh, the win_clang bots are special in that they build all targets -- most
> other bots don't, and the win clang bots are only fyi bots. But you're
> right, they check that the target builds fine. They don't tell you if the
> tests pass when run though.
> 

I tested it locally and it builds and runs with and without the patch.

I guess we want to just test if it builds on the bots, since the tests are not
run in any bots.

[Primiano Tucci (use gerrit), 2015-12-02 16:14:52]

On 2015/12/02 16:13:00, ssid wrote:
> I guess we want to just test if it builds on the bots, since the tests are not
> run in any bots.

Considering that we are going to make some refactoring (again, this is going to
be a long December), I'd like the idea that this test runs reliably and tells us
if we accidentally break something.
Since we have a test, and, as you say, looks to be even running locally with
success, can we plz move it to base_unittests so it runs continuously?

[Will Harris, 2015-12-02 17:49:18]

The allocator_unittests are mostly to test edge case behavior that is specific
to tcmalloc - they are originally taken from tests in
third_party/tcmalloc/vendor/src/tests/ e.g. realloc_unittest.cc so while they
are /useful/ on platforms that still use tcmalloc, they are very tcmalloc
specific... I think at some point where tcmalloc was being used on every
platform these just became the allocator_unittests.

The most important tests for the allocators (e.g. overflow protection, make sure
uncheckedmalloc actually works) are in base/process/memory_unittest.cc

[ssid, 2015-12-02 18:13:07]

On 2015/12/02 17:49:18, Will Harris wrote:
> The allocator_unittests are mostly to test edge case behavior that is specific
> to tcmalloc - they are originally taken from tests in
> third_party/tcmalloc/vendor/src/tests/ e.g. realloc_unittest.cc so while they
> are /useful/ on platforms that still use tcmalloc, they are very tcmalloc
> specific... I think at some point where tcmalloc was being used on every
> platform these just became the allocator_unittests.
> 
> The most important tests for the allocators (e.g. overflow protection, make
sure
> uncheckedmalloc actually works) are in base/process/memory_unittest.cc

I am thinking of having allocator_unittests in allocator and making it platform
independent. Since we are going to introduce a shim layer which hooks malloc
calls, it is better to have such a basic test in place. The ones which are
specific to tcmalloc will be moved to tcmalloc_unittest.cc.

[jamesr, 2015-12-02 18:56:45]

This scheme was to support having a runtime-switchable allocator on windows
which was removed some time ago.  It's unlikely any of the remaining allocator
indirections are useful today.

[Will Harris, 2015-12-04 21:09:02]

lgtm

One thing I noticed while reviewing this CL is that the comment in
content_main_runner.cc where the functions are initialized from tcmalloc is
wrong - _heap_init in the allocator shim no longer sets up these thunks, since
they are now completely tcmalloc specific

https://code.google.com/p/chromium/codesearch#chromium/src/content/app/conten...

https://codereview.chromium.org/1487403002/diff/60001/base/allocator/allocato...
File base/allocator/allocator_extension.h (right):

https://codereview.chromium.org/1487403002/diff/60001/base/allocator/allocato...
base/allocator/allocator_extension.h:25: // malloc implementation.
nit: s/malloc/allocator/

[ssid, 2015-12-07 21:14:34]

Patchset #5 (id:80001) has been deleted

[ssid, 2015-12-07 21:16:15]

Description was changed from

==========
Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason. See design doc in the bug.

This CL removes the target and stores the functions in base.

BUG=564618
==========

to

==========
Remove allocator_extension_thunks since this layer is not required

crrev.com/10239012 added thunks target. There allocator_shim.cc wanted
to set function pointers when heap_init_() was called. To do this,
allocator_shim.cc (part of allocator target) used thunks to store the
functions without actually depending on thunks (relying on the fact
that base would finally link thunks). Since allocator_shim.cc included
thunks.h and allocator_unittests included allocator, but not base,
thunks target was introduced to avoid depending on base. Current
situation is that thunks is no longer used by the shim layer and not
used by allocator_unittests either (after shim was removed). Thunks was
left around for no reason. See design doc in the bug.

This CL removes the target and stores the functions in base.

Landed as https://codereview.chromium.org/1487403002/
BUG=564618
==========