Run certain chrome_elf_unittests only in Release.

These tests depend on various parts of base not getting included in chrome_elf.dll as described in the comment added to the unit test file. This doesn't work in GN debug build because the GN build uses source sets rather than shared libraries. This change runs the test only in release mode.

This replaces the dependencies on base_static with base and removes the comment about not depending on all of base. All of base is getting linked in via the sandbox dependency, so this effort is wasted. The regular base dependency is clearer.

Slight iterator improvements in the update unit tests for readability.

Independently, this fixes the linking of chrome_child with symbols on Windows. The conditions were messed up. The code meant to force incremental linking off when full symbols are enabled, but actually did the reverse.

BUG=505062

Review URL: https://codereview.chromium.org/1485343002

Cr-Commit-Position: refs/heads/master@{#362767}

3 files changed, 27 insertions, 15 deletions

diff --git a/chrome_elf/BUILD.gn b/chrome_elf/BUILD.gn
index 4f9c747..3afa19b 100644
--- a/chrome_elf/BUILD.gn
+++ b/chrome_elf/BUILD.gn
@@ -106,12 +106,9 @@ static_library("blacklist") {
     "blacklist/blacklist_interceptions.h",
   ]
   deps = [
-    # Depend on base_static, but do NOT take a dependency on base.gyp:base
-    # as that would risk pulling in base's link-time dependencies which
-    # chrome_elf cannot do.
     ":breakpad",
     ":constants",
-    "//base:base_static",
+    "//base",
     "//sandbox:sandbox",
   ]
 }
diff --git a/chrome_elf/blacklist.gypi b/chrome_elf/blacklist.gypi
index 50d1a26..f552186 100644
--- a/chrome_elf/blacklist.gypi
+++ b/chrome_elf/blacklist.gypi
@@ -17,10 +17,7 @@
         'blacklist/blacklist_interceptions.h',
       ],
       'dependencies': [
-        # Depend on base_static, but do NOT take a dependency on base.gyp:base
-        # as that would risk pulling in base's link-time dependencies which
-        # chrome_elf cannot do.
-        '../base/base.gyp:base_static',
+        '../base/base.gyp:base',
         '../chrome_elf/chrome_elf.gyp:chrome_elf_breakpad',
         '../chrome_elf/chrome_elf.gyp:chrome_elf_constants',
         '../sandbox/sandbox.gyp:sandbox',
diff --git a/chrome_elf/elf_imports_unittest.cc b/chrome_elf/elf_imports_unittest.cc
index 941c641..87a1f59 100644
--- a/chrome_elf/elf_imports_unittest.cc
+++ b/chrome_elf/elf_imports_unittest.cc
@@ -47,20 +47,35 @@ class ELFImportsTest : public testing::Test {
   }
 };
 
+// Run this test only in Release builds.
+//
+// This test makes sure that chrome_elf.dll has only certain types of imports.
+// However, it directly and indirectly depends on base, which has lots more
+// imports than are allowed here.
+//
+// In release builds, the offending imports are all stripped since this
+// depends on a relatively small portion of base. In GYP, this works in debug
+// builds as well because static libraries are used for the sandbox and base
+// targets and the files that use e.g. user32.dll happen to not get brought
+// into the build in the first place (due to the way static libraries are
+// linked where only the required .o files are included). But we don't bother
+// differentiating GYP and GN builds for this purpose.
+//
+// If you break this test, you may have changed base or the Windows sandbox
+// such that more system imports are required to link.
+#ifdef NDEBUG
 TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
-  std::vector<std::string> elf_imports;
-
   base::FilePath dll;
   ASSERT_TRUE(PathService::Get(base::DIR_EXE, &dll));
   dll = dll.Append(L"chrome_elf.dll");
+
+  std::vector<std::string> elf_imports;
   GetImports(dll, &elf_imports);
 
   // Check that ELF has imports.
   ASSERT_LT(0u, elf_imports.size()) << "Ensure the chrome_elf_unittests "
     "target was built, instead of chrome_elf_unittests.exe";
 
-  std::vector<std::string>::iterator it(elf_imports.begin());
-
   static const char* const kValidFilePatterns[] = {
     "KERNEL32.dll",
     "MSVC*",
@@ -74,15 +89,18 @@ TEST_F(ELFImportsTest, ChromeElfSanityCheck) {
   };
 
   // Make sure all of ELF's imports are in the valid imports list.
-  for (; it != elf_imports.end(); it++) {
+  for (const std::string& import : elf_imports) {
     bool match = false;
     for (int i = 0; i < arraysize(kValidFilePatterns); ++i) {
-      if (base::MatchPattern(*it, kValidFilePatterns[i]))
+      if (base::MatchPattern(import, kValidFilePatterns[i])) {
         match = true;
+        break;
+      }
     }
-    ASSERT_TRUE(match) << "Illegal import in chrome_elf.dll: " << *it;
+    ASSERT_TRUE(match) << "Illegal import in chrome_elf.dll: " << import;
   }
 }
+#endif  // NDEBUG
 
 TEST_F(ELFImportsTest, ChromeExeSanityCheck) {
   std::vector<std::string> exe_imports;