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

base/BUILD.gn

 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # HOW TO WRITE CONDITIONALS IN THIS FILE
 # ======================================
 #
 # In many other places, one would write a conditional that expresses all the
 # cases when a source file is used or unused, and then either add or subtract
 # it from the sources list in that case
@@ skipping 104 matching lines @@
     visibility = [ ":base" ]
   }
 }
 
 if (is_android) {
   config("android_system_libs") {
     libs = [ "log" ]  # Used by logging.cc.
   }
 }
 
+# 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
 # few static initializers floating around means that dead code stripping
 # still leaves a lot of code behind that isn't always used. For example, this
 # saves more than 40K for a smaller target like chrome_elf.
 #
 # Use static libraries for the helper stuff as well like //base/debug since
 # those things refer back to base code, which will force base compilation units
 # to be linked in where they wouldn't have otherwise. This does not include
@@ skipping 941 matching lines @@
     "win/wait_chain.cc",
     "win/wait_chain.h",
     "win/win_util.cc",
     "win/win_util.h",
     "win/windows_version.cc",
     "win/windows_version.h",
     "win/wrapped_window_proc.cc",
     "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",
   ]
 
   public_deps = [
     ":base_paths",
     ":base_static",
     ":build_date",
     ":debugging_flags",
   ]
 
   # Needed for <atomic> if using newer C++ library than sysroot
   if (!use_sysroot && (is_android || (is_linux && !is_chromecast))) {
     libs = [ "atomic" ]
   }
 
   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
   # strings
   if (is_mac || is_ios || is_chromeos || is_chromecast) {
     defines += [ "SYSTEM_NATIVE_UTF8" ]
   }
 
   # Android.
   if (is_android) {
@@ skipping 240 matching lines @@
     # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
     configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
 
     libs = [
       "cfgmgr32.lib",
       "powrprof.lib",
       "setupapi.lib",
       "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" ]
   }
 
   # Desktop Mac.
   if (is_mac) {
     sources += [
       "mac/scoped_typeref.h",
       "power_monitor/power_monitor_device_source_mac.mm",
@@ skipping 36 matching lines @@
 
     # TODO(brettw) this will need to be parameterized at some point.
     linux_configs = []
     if (use_glib) {
       linux_configs += [ "//build/config/linux:glib" ]
     }
 
     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.
     deps += [
       "//base/third_party/symbolize",
       "//base/third_party/xdg_mime",
       "//base/third_party/xdg_user_dirs",
     ]
   } else {
     # Non-Linux.
@@ skipping 673 matching lines @@
     "win/wait_chain_unittest.cc",
     "win/win_util_unittest.cc",
     "win/windows_version_unittest.cc",
     "win/wrapped_window_proc_unittest.cc",
   ]
 
   defines = []
 
   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",
     "//base/third_party/dynamic_annotations",
     "//testing/gmock",
     "//testing/gtest",
     "//third_party/icu",
   ]
 
   data_deps = [
     "//base/test:test_shared_library",
   ]
 
   if (is_ios || is_mac) {
     deps += [ ":base_unittests_arc" ]
   }
 
   public_deps = [
     ":base_unittests_bundle_data",
   ]
 
   # Some unittests depend on the ALLOCATOR_SHIM macro.
-  configs += [ "//base/allocator:allocator_shim_define" ]
+  configs += [ ":base_allocator_shim_define" ]
 
   data = [
     "test/data/",
   ]
 
   # Allow more direct string conversions on platforms with native utf8
   # strings
   if (is_mac || is_ios || is_chromeos || is_chromecast) {
     defines += [ "SYSTEM_NATIVE_UTF8" ]
   }
@@ skipping 424 matching lines @@
 }
 
 fuzzer_test("base_json_correctness_fuzzer") {
   sources = [
     "json/correctness_fuzzer.cc",
   ]
   deps = [
     ":base",
   ]
 }