Fold most logic from //base/allocator into //base.

base/BUILD.gn

Index: base/BUILD.gn
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 4dd7c690338fb4c9a8c948caf1240e54a784cd50..3a83e4dd76e85f1097dd7a5cf33a93988371ca40 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -122,6 +122,73 @@ if (is_android) {
   }
 }
 
+# The Windows-only allocator shim is only enabled for Release static builds, and

[Primiano Tucci (use gerrit), 2017/01/26 02:03:44]

Hmm wouldn't it make sense that this allocator-related stuff still lived in
//base/allocator/BUILD.gn?
correct me if I am wrong but the  actual problem you found here is the
allocator_shim being a source_set, which would cause cylic dependencies.
All these configs and groups could still live in /allocator and be reffered by
base (as it is already the case today, if I read the code correctly).
I think the only thing that you might need to move is the actual sources +=
assignments.

P.S. all this provided that the pattern of 
target(A) depending source_set(B) and B having references to things in A
(without explicitly depending on it) is undesirable.
Should we check with gn-dev whether this is actually a problem or we have other
instances of this? Which would make all this a non-problem

[erikchen, 2017/01/26 02:09:12]

win_use_allocator_shim is used by both this logic and the actual sources logic.
We could replicate the logic to create win_use_allocator_shim, or pull it into a
gni file, but that seems like overkill.
The documentation in base says that base should not depend on source_sets. So
let's say we turn it into a static library. Now we have references to undefined
symbols.

[Primiano Tucci (use gerrit), 2017/01/26 18:47:09]

What I am saying is that all the windows logic (and I think also the android
below) can stay (and IMHO shoud stay) in base/allocator/BUILD.gn
This is my line of reasoning, tell me if I am missing something:

The following targets depend on win_use_allocator_shim:
 1. source_set("base_allocator_shim")
 2. config(base_allocator_shim_define) 
 3. group(base_allocator)

1. is a pure leaf target and does not have any dependencies back to base. So
could live in allocator/BUILD.gn
2. is a pure config target, could live in allocator/BUILD.gn
3. depends in turn on :
   tcmalloc: which is a pure leaf target and can live (in fact you are correctly
leaving it there) in allocator/
   base_allocator_shim: see point 1 above.

So, if I am reasoning correctly all this hunk (lines 125 - 191) can stay in
allocator/BUILD.gn without requiring any gni or any logic duplication.

[erikchen, 2017/01/26 20:51:52]

Done.

[This was the first thing I tried, but for some reason I thought
win_use_allocator_shim would have to be duplicated.]

+# is mutually exclusive with the generalized shim.
+win_use_allocator_shim = is_win && !is_component_build && !is_debug &&
+                         !use_experimental_allocator_shim && !is_asan
+
+# This "base_allocator" meta-target will forward to the default allocator
+# according to the build settings.
+group("base_allocator") {
+  public_deps = []
+  deps = []
+  if (use_allocator == "tcmalloc") {
+    deps += [ "//base/allocator:tcmalloc" ]
+  }
+
+  if (win_use_allocator_shim) {
+    public_deps += [ ":base_allocator_shim" ]
+  }
+}
+
+# This config defines ALLOCATOR_SHIM in the same conditions that the allocator
+# shim will be used by the allocator target.
+#
+# TODO(brettw) this is only used in one place and is kind of mess, because it
+# assumes that the library using it will eventually be linked with
+# //base/allocator in the default way. Clean this up and delete this.
+config("base_allocator_shim_define") {
+  if (win_use_allocator_shim) {
+    defines += [ "ALLOCATOR_SHIM" ]
+  }
+}
+
+# This config is only used on Windows static release builds for the
+# allocator shim.
+if (win_use_allocator_shim) {
+  source_set("base_allocator_shim") {
+    sources = [
+      "allocator/allocator_shim_win.cc",
+      "allocator/allocator_shim_win.h",
+      "allocator/winheap_stubs_win.cc",
+      "allocator/winheap_stubs_win.h",
+    ]
+    configs += [ ":base_allocator_shim_define" ]
+  }
+}
+
+buildflag_header("base_allocator_features") {
+  header = "allocator/features.h"
+  flags = [
+    "USE_EXPERIMENTAL_ALLOCATOR_SHIM=$use_experimental_allocator_shim",
+    "ENABLE_WIN_ALLOCATOR_SHIM_TESTS=($use_experimental_allocator_shim || $win_use_allocator_shim)",
+  ]
+}
+
+# Used to shim malloc symbols on Android. see //base/allocator/README.md.
+config("base_allocator_wrap_malloc_symbols") {
+  ldflags = [
+    "-Wl,-wrap,calloc",
+    "-Wl,-wrap,free",
+    "-Wl,-wrap,malloc",
+    "-Wl,-wrap,memalign",
+    "-Wl,-wrap,posix_memalign",
+    "-Wl,-wrap,pvalloc",
+    "-Wl,-wrap,realloc",
+    "-Wl,-wrap,valloc",
+  ]
+}
+
 # Base and everything it depends on should be a static library rather than
 # a source set. Base is more of a "library" in the classic sense in that many
 # small parts of it are used in many different contexts. This combined with a
@@ -1083,19 +1150,20 @@ component("base") {
     "win/wrapped_window_proc.h",
   ]
 
+  all_dependent_configs = []
   defines = []
   data = []
 
   configs += [
     ":base_flags",
     ":base_implementation",
-    "//base/allocator:allocator_shim_define",  # for allocator_check.cc.
+    ":base_allocator_shim_define",  # for allocator_check.cc.
     "//build/config:precompiled_headers",
   ]
 
   deps = [
-    "//base/allocator",
-    "//base/allocator:features",
+    ":base_allocator",
+    ":base_allocator_features",
     "//base/third_party/dynamic_annotations",
     "//third_party/modp_b64",
   ]
@@ -1113,9 +1181,37 @@ component("base") {
   }
 
   if (use_experimental_allocator_shim) {
-    # The allocator shim is part of the base API. This is to allow clients of
-    # base should to install hooks into the allocator path.
-    public_deps += [ "//base/allocator:unified_allocator_shim" ]
+    # TODO(primiano): support other platforms, currently this works only on
+    # Linux/CrOS/Android. http://crbug.com/550886 .
+    sources += [
+      "allocator/allocator_shim.cc",
+      "allocator/allocator_shim.h",
+      "allocator/allocator_shim_internals.h",
+      "allocator/allocator_shim_override_cpp_symbols.h",
+      "allocator/allocator_shim_override_libc_symbols.h",
+    ]
+    if (is_win) {
+      sources += [
+        "allocator/allocator_shim_default_dispatch_to_winheap.cc",
+        "allocator/allocator_shim_override_ucrt_symbols_win.h",
+        "allocator/winheap_stubs_win.cc",
+        "allocator/winheap_stubs_win.h",
+      ]
+    } else if (is_linux && use_allocator == "tcmalloc") {
+      sources += [
+        "allocator/allocator_shim_default_dispatch_to_tcmalloc.cc",
+        "allocator/allocator_shim_override_glibc_weak_symbols.h",
+      ]
+      deps += [ "//base/allocator:tcmalloc" ]
+    } else if (is_linux && use_allocator == "none") {
+      sources += [ "allocator/allocator_shim_default_dispatch_to_glibc.cc" ]
+    } else if (is_android && use_allocator == "none") {
+      sources += [
+        "allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc",
+        "allocator/allocator_shim_override_linker_wrapped_symbols.h",
+      ]
+      all_dependent_configs += [ ":base_allocator_wrap_malloc_symbols" ]
+    }
   }
 
   # Allow more direct string conversions on platforms with native utf8
@@ -1376,7 +1472,7 @@ component("base") {
       "userenv.lib",
       "winmm.lib",
     ]
-    all_dependent_configs = [ ":base_win_linker_flags" ]
+    all_dependent_configs += [ ":base_win_linker_flags" ]
   } else if (!is_nacl || is_nacl_nonsfi) {
     # Non-Windows.
     deps += [ "//base/third_party/libevent" ]
@@ -1433,7 +1529,7 @@ component("base") {
     defines += [ "USE_SYMBOLIZE" ]
 
     configs += linux_configs
-    all_dependent_configs = linux_configs
+    all_dependent_configs += linux_configs
 
     # These dependencies are not required on Android, and in the case
     # of xdg_mime must be excluded due to licensing restrictions.
@@ -2127,9 +2223,9 @@ test("base_unittests") {
 
   deps = [
     ":base",
+    ":base_allocator_features",
     ":i18n",
     ":message_loop_tests",
-    "//base/allocator:features",
     "//base/test:native_library_test_utils",
     "//base/test:run_all_base_unittests",
     "//base/test:test_support",
@@ -2152,7 +2248,7 @@ test("base_unittests") {
   ]
 
   # Some unittests depend on the ALLOCATOR_SHIM macro.
-  configs += [ "//base/allocator:allocator_shim_define" ]
+  configs += [ ":base_allocator_shim_define" ]
 
   data = [
     "test/data/",