Make all SafeBrowsing* unit_tests work under AddressSanitizer on Windows

AddressSanitizer (ASan) uses code patching of every module of an application at startup / module load.
As a result, many tests that check the number of modified functions misbehave.
I'm disabling most of them and adjusting one that's easy to generalize.

I've also replaced some EXPECT_* macros with ASSERT_* per
https://code.google.com/p/googletest/wiki/V1_7_Primer#Assertions
"you should use ASSERT_* if it doesn't make sense to continue when the assertion in question fails"

BUG=345874

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

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

chrome/browser/safe_browsing/incident_reporting/environment_data_collection_win_unittest.cc

@@ -182,10 +182,10 @@ TEST(SafeBrowsingEnvironmentDataCollectionWinTest, VerifyLoadedModules) {
   // Edit the first byte of the function exported by the first module. Calling
   // GetModuleHandle so we do not increment the library ref count.
   HMODULE module_handle = GetModuleHandle(safe_browsing::kTestDllNames[0]);
-  EXPECT_NE(reinterpret_cast<HANDLE>(NULL), module_handle);
+  ASSERT_NE(reinterpret_cast<HANDLE>(NULL), module_handle);
   uint8_t* export_addr = reinterpret_cast<uint8_t*>(
       GetProcAddress(module_handle, safe_browsing::kTestExportName));
-  EXPECT_NE(reinterpret_cast<uint8_t*>(NULL), export_addr);
+  ASSERT_NE(reinterpret_cast<uint8_t*>(NULL), export_addr);
 
   uint8_t new_val = (*export_addr) + 1;
   SIZE_T bytes_written = 0;
@@ -194,7 +194,7 @@ TEST(SafeBrowsingEnvironmentDataCollectionWinTest, VerifyLoadedModules) {
                      reinterpret_cast<void*>(&new_val),
                      1,
                      &bytes_written);
-  EXPECT_EQ(1, bytes_written);
+  ASSERT_EQ(1, bytes_written);
 
   safe_browsing::ClientIncidentReport_EnvironmentData_Process process_report;
   safe_browsing::CollectModuleVerificationData(
@@ -205,7 +205,14 @@ TEST(SafeBrowsingEnvironmentDataCollectionWinTest, VerifyLoadedModules) {
   // CollectModuleVerificationData should return the single modified module and
   // its modified export.  The other module, being unmodified, is omitted from
   // the returned list of modules.
+  // AddressSanitizer build is special though, as it patches the code at
+  // startup, which makes every single module modified and introduces extra
+  // exports.
+  ASSERT_LE(1, process_report.module_state_size());
+#if !defined(ADDRESS_SANITIZER)
   EXPECT_EQ(1, process_report.module_state_size());
+  EXPECT_EQ(1, process_report.module_state(0).modified_export_size());
+#endif
 
   EXPECT_EQ(base::WideToUTF8(safe_browsing::kTestDllNames[0]),
             process_report.module_state(0).name());
@@ -213,7 +220,6 @@ TEST(SafeBrowsingEnvironmentDataCollectionWinTest, VerifyLoadedModules) {
       safe_browsing::ClientIncidentReport_EnvironmentData_Process_ModuleState::
           MODULE_STATE_MODIFIED,
       process_report.module_state(0).modified_state());
-  EXPECT_EQ(1, process_report.module_state(0).modified_export_size());
   EXPECT_EQ(std::string(safe_browsing::kTestExportName),
             process_report.module_state(0).modified_export(0));
 }

chrome/browser/safe_browsing/incident_reporting/module_integrity_verifier_win_unittest.cc

@@ -86,12 +86,15 @@ class SafeBrowsingModuleVerifierWinTest : public testing::Test {
   scoped_ptr<base::win::PEImage> mem_peimage_ptr_;
 };
 
+// Don't run these tests under AddressSanitizer as it patches the modules on
+// startup, thus interferes with all these test expectations.
+#if !defined(ADDRESS_SANITIZER)
 TEST_F(SafeBrowsingModuleVerifierWinTest, VerifyModuleUnmodified) {
   std::set<std::string> modified_exports;
   // Call VerifyModule before the module has been loaded, should fail.
-  EXPECT_EQ(MODULE_STATE_UNKNOWN,
+  ASSERT_EQ(MODULE_STATE_UNKNOWN,
             VerifyModule(kTestDllNames[0], &modified_exports));
-  EXPECT_EQ(0, modified_exports.size());
+  ASSERT_EQ(0, modified_exports.size());
 
   // On loading, the module should be identical (up to relocations) in memory as
   // on disk.
@@ -105,13 +108,13 @@ TEST_F(SafeBrowsingModuleVerifierWinTest, VerifyModuleModified) {
   std::set<std::string> modified_exports;
   // Confirm the module is identical in memory as on disk before we begin.
   SetUpTestDllAndPEImages();
-  EXPECT_EQ(MODULE_STATE_UNMODIFIED,
+  ASSERT_EQ(MODULE_STATE_UNMODIFIED,
             VerifyModule(kTestDllNames[0], &modified_exports));
 
   uint8_t* mem_code_addr = NULL;
   uint8_t* disk_code_addr = NULL;
   uint32_t code_size = 0;
-  EXPECT_TRUE(GetCodeAddrsAndSize(*mem_peimage_ptr_,
+  ASSERT_TRUE(GetCodeAddrsAndSize(*mem_peimage_ptr_,
                                   *disk_peimage_ptr_,
                                   &mem_code_addr,
                                   &disk_code_addr,
@@ -137,7 +140,7 @@ TEST_F(SafeBrowsingModuleVerifierWinTest, VerifyModuleExportModified) {
   std::set<std::string> modified_exports;
   // Confirm the module is identical in memory as on disk before we begin.
   SetUpTestDllAndPEImages();
-  EXPECT_EQ(MODULE_STATE_UNMODIFIED,
+  ASSERT_EQ(MODULE_STATE_UNMODIFIED,
             VerifyModule(kTestDllNames[0], &modified_exports));
   modified_exports.clear();
 
@@ -146,8 +149,9 @@ TEST_F(SafeBrowsingModuleVerifierWinTest, VerifyModuleExportModified) {
   EditExport();
   EXPECT_EQ(MODULE_STATE_MODIFIED,
             VerifyModule(kTestDllNames[0], &modified_exports));
-  EXPECT_EQ(1, modified_exports.size());
+  ASSERT_EQ(1, modified_exports.size());
   EXPECT_EQ(0, std::string(kTestExportName).compare(*modified_exports.begin()));
 }
+#endif  // ADDRESS_SANITIZER
 
 }  // namespace safe_browsing