Merge "metric: overhaul process associations in startup metric"
diff --git a/src/base/file_utils.cc b/src/base/file_utils.cc
index 7ccbad4..8f998dd 100644
--- a/src/base/file_utils.cc
+++ b/src/base/file_utils.cc
@@ -237,15 +237,14 @@
     if (glob_path.length() + 1 > MAX_PATH)
       return base::ErrStatus("Directory path %s is too long", dir_path.c_str());
     WIN32_FIND_DATAA ffd;
-    // We do not use a ScopedResource for the HANDLE from FindFirstFile because
-    // the invalid value INVALID_HANDLE_VALUE is not a constexpr under some
-    // compile configurations, and thus cannot be used as a template argument.
-    HANDLE hFind = FindFirstFileA(glob_path.c_str(), &ffd);
-    if (hFind == INVALID_HANDLE_VALUE) {
+
+    base::ScopedResource<HANDLE, FindClose, nullptr, false,
+                         base::PlatformHandleChecker>
+        hFind(FindFirstFileA(glob_path.c_str(), &ffd));
+    if (!hFind) {
       // For empty directories, there should be at least one entry '.'.
       // If FindFirstFileA returns INVALID_HANDLE_VALUE, this means directory
       // couldn't be accessed.
-      FindClose(hFind);
       return base::ErrStatus("Failed to open directory %s", cur_dir.c_str());
     }
     do {
@@ -254,13 +253,12 @@
       if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
         std::string subdir_path = cur_dir + ffd.cFileName + '/';
         dir_queue.push_back(subdir_path);
-      } else if (ffd.dwFileAttributes & FILE_ATTRIBUTE_NORMAL) {
+      } else {
         const std::string full_path = cur_dir + ffd.cFileName;
         PERFETTO_CHECK(full_path.length() > root_dir_path.length());
         output.push_back(full_path.substr(root_dir_path.length()));
       }
-    } while (FindNextFileA(hFind, &ffd));
-    FindClose(hFind);
+    } while (FindNextFileA(*hFind, &ffd));
 #else
     ScopedDir dir = ScopedDir(opendir(cur_dir.c_str()));
     if (!dir) {
diff --git a/src/base/http/http_server.cc b/src/base/http/http_server.cc
index 17a1652..1a90339 100644
--- a/src/base/http/http_server.cc
+++ b/src/base/http/http_server.cc
@@ -464,11 +464,12 @@
   append("HTTP/1.1 ");
   append(http_code);
   append("\r\n");
-  for (const char* hdr : headers) {
-    if (strlen(hdr) == 0)
+  for (const char* hdr_cstr : headers) {
+    StringView hdr = (hdr_cstr);
+    if (hdr.empty())
       continue;
-    has_connection_header |= strncasecmp(hdr, "connection:", 11) == 0;
-    append(hdr);
+    has_connection_header |= hdr.substr(0, 11).CaseInsensitiveEq("connection:");
+    append(hdr_cstr);
     append("\r\n");
   }
   content_len_actual_ = 0;
diff --git a/src/base/http/http_server_unittest.cc b/src/base/http/http_server_unittest.cc
index 9726ab6..be65198 100644
--- a/src/base/http/http_server_unittest.cc
+++ b/src/base/http/http_server_unittest.cc
@@ -66,16 +66,17 @@
     auto checkpoint = task_runner_->CreateCheckpoint(checkpoint_name);
     std::string rxbuf;
     sock.SetBlocking(false);
-    task_runner_->AddFileDescriptorWatch(sock.fd(), [&] {
+    task_runner_->AddFileDescriptorWatch(sock.watch_handle(), [&] {
       char buf[1024]{};
       auto rsize = PERFETTO_EINTR(sock.Receive(buf, sizeof(buf)));
-      ASSERT_GE(rsize, 0);
+      if (rsize < 0)
+        return;
       rxbuf.append(buf, static_cast<size_t>(rsize));
       if (rsize == 0 || (min_bytes && rxbuf.length() >= min_bytes))
         checkpoint();
     });
     task_runner_->RunUntilCheckpoint(checkpoint_name);
-    task_runner_->RemoveFileDescriptorWatch(sock.fd());
+    task_runner_->RemoveFileDescriptorWatch(sock.watch_handle());
     return rxbuf;
   }
 
@@ -289,7 +290,9 @@
   srv_.AddAllowedOrigin("notallowed.com");
 
   HttpCli cli(&task_runner_);
-  EXPECT_CALL(handler_, OnHttpConnectionClosed(_));
+  auto close_checkpoint = task_runner_.CreateCheckpoint("close");
+  EXPECT_CALL(handler_, OnHttpConnectionClosed(_))
+      .WillOnce(InvokeWithoutArgs(close_checkpoint));
   EXPECT_CALL(handler_, OnHttpRequest(_))
       .WillOnce(Invoke([&](const HttpRequest& req) {
         EXPECT_EQ(req.origin.ToStdString(), "http://notallowed.com");
@@ -314,6 +317,7 @@
 
   EXPECT_EQ(cli.Recv(expected_resp.size()), expected_resp);
   cli.sock.Shutdown();
+  task_runner_.RunUntilCheckpoint("close");
 }
 
 }  // namespace
diff --git a/src/base/http/sha1.cc b/src/base/http/sha1.cc
index da3f753..f3e1e5d 100644
--- a/src/base/http/sha1.cc
+++ b/src/base/http/sha1.cc
@@ -30,7 +30,7 @@
 #if defined(__GNUC__)
   return __builtin_bswap32(x);
 #elif defined(_MSC_VER)
-  return _byteswap_ulong(val);
+  return _byteswap_ulong(x);
 #else
   return (((x & 0xff000000u) >> 24) | ((x & 0x00ff0000u) >> 8) |
           ((x & 0x0000ff00u) << 8) | ((x & 0x000000ffu) << 24));
diff --git a/src/base/unix_socket_unittest.cc b/src/base/unix_socket_unittest.cc
index c838663..45c09d5 100644
--- a/src/base/unix_socket_unittest.cc
+++ b/src/base/unix_socket_unittest.cc
@@ -332,53 +332,6 @@
   tx_thread.join();
 }
 
-// Regression test for b/193234818. SO_SNDTIMEO is unreliable on most systems.
-// It doesn't guarantee that the whole send() call blocks for at most X, as the
-// kernel rearms the timeout if the send buffers frees up and allows a partial
-// send. This test reproduces the issue 100% on Mac. Unfortunately on Linux the
-// repro seem to happen only when a suspend happens in the middle.
-TEST_F(UnixSocketTest, BlockingSendTimeout) {
-  TestTaskRunner ttr;
-  UnixSocketRaw send_sock;
-  UnixSocketRaw recv_sock;
-  std::tie(send_sock, recv_sock) =
-      UnixSocketRaw::CreatePairPosix(kTestSocket.family(), SockType::kStream);
-
-  auto blocking_send_done = ttr.CreateCheckpoint("blocking_send_done");
-
-  std::thread tx_thread([&] {
-    // Fill the tx buffer in non-blocking mode.
-    send_sock.SetBlocking(false);
-    char buf[1024 * 16]{};
-    while (send_sock.Send(buf, sizeof(buf)) > 0) {
-    }
-
-    // Then do a blocking send. It should return a partial value within the tx
-    // timeout.
-    send_sock.SetBlocking(true);
-    send_sock.SetTxTimeout(10);
-    ASSERT_LT(send_sock.Send(buf, sizeof(buf)),
-              static_cast<ssize_t>(sizeof(buf)));
-    ttr.PostTask(blocking_send_done);
-  });
-
-  // This task needs to be slow enough so that doesn't unblock the send, but
-  // fast enough so that within a blocking cycle, the send re-attempts and
-  // re-arms the timeout.
-  PeriodicTask read_slowly_task(&ttr);
-  PeriodicTask::Args args;
-  args.period_ms = 1;  // Read 1 byte every ms (1 KiB/s).
-  args.task = [&] {
-    char rxbuf[1]{};
-    recv_sock.Receive(rxbuf, sizeof(rxbuf));
-  };
-  read_slowly_task.Start(args);
-
-  ttr.RunUntilCheckpoint("blocking_send_done");
-  read_slowly_task.Reset();
-  tx_thread.join();
-}
-
 // Regression test for b/76155349 . If the receiver end disconnects while the
 // sender is in the middle of a large send(), the socket should gracefully give
 // up (i.e. Shutdown()) but not crash.
@@ -939,6 +892,53 @@
   ASSERT_EQ(hdr.msg_iov, nullptr);
   ASSERT_EQ(memcmp(&send_buf[0], &recv_buf[0], send_buf.size()), 0);
 }
+
+// Regression test for b/193234818. SO_SNDTIMEO is unreliable on most systems.
+// It doesn't guarantee that the whole send() call blocks for at most X, as the
+// kernel rearms the timeout if the send buffers frees up and allows a partial
+// send. This test reproduces the issue 100% on Mac. Unfortunately on Linux the
+// repro seem to happen only when a suspend happens in the middle.
+TEST_F(UnixSocketTest, BlockingSendTimeout) {
+  TestTaskRunner ttr;
+  UnixSocketRaw send_sock;
+  UnixSocketRaw recv_sock;
+  std::tie(send_sock, recv_sock) =
+      UnixSocketRaw::CreatePairPosix(kTestSocket.family(), SockType::kStream);
+
+  auto blocking_send_done = ttr.CreateCheckpoint("blocking_send_done");
+
+  std::thread tx_thread([&] {
+    // Fill the tx buffer in non-blocking mode.
+    send_sock.SetBlocking(false);
+    char buf[1024 * 16]{};
+    while (send_sock.Send(buf, sizeof(buf)) > 0) {
+    }
+
+    // Then do a blocking send. It should return a partial value within the tx
+    // timeout.
+    send_sock.SetBlocking(true);
+    send_sock.SetTxTimeout(10);
+    ASSERT_LT(send_sock.Send(buf, sizeof(buf)),
+              static_cast<ssize_t>(sizeof(buf)));
+    ttr.PostTask(blocking_send_done);
+  });
+
+  // This task needs to be slow enough so that doesn't unblock the send, but
+  // fast enough so that within a blocking cycle, the send re-attempts and
+  // re-arms the timeout.
+  PeriodicTask read_slowly_task(&ttr);
+  PeriodicTask::Args args;
+  args.period_ms = 1;  // Read 1 byte every ms (1 KiB/s).
+  args.task = [&] {
+    char rxbuf[1]{};
+    recv_sock.Receive(rxbuf, sizeof(rxbuf));
+  };
+  read_slowly_task.Start(args);
+
+  ttr.RunUntilCheckpoint("blocking_send_done");
+  read_slowly_task.Reset();
+  tx_thread.join();
+}
 #endif  // !OS_WIN
 
 }  // namespace
diff --git a/src/trace_processor/timestamped_trace_piece.h b/src/trace_processor/timestamped_trace_piece.h
index ca61546..1b1a0f6 100644
--- a/src/trace_processor/timestamped_trace_piece.h
+++ b/src/trace_processor/timestamped_trace_piece.h
@@ -77,9 +77,19 @@
   std::array<double, kMaxNumExtraCounters> extra_counter_values = {};
 };
 
+// On Windows std::aligned_storage was broken before VS 2017 15.8 and the
+// compiler (even clang-cl) requires -D_ENABLE_EXTENDED_ALIGNED_STORAGE. Given
+// the alignment here is purely a performance enhancment with no other
+// functional requirement, disable it on Win.
+#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+#define PERFETTO_TTS_ALIGNMENT alignas(64)
+#else
+#define PERFETTO_TTS_ALIGNMENT
+#endif
+
 // A TimestampedTracePiece is (usually a reference to) a piece of a trace that
 // is sorted by TraceSorter.
-struct alignas(64) TimestampedTracePiece {
+struct PERFETTO_TTS_ALIGNMENT TimestampedTracePiece {
   enum class Type {
     kInvalid = 0,
     kFtraceEvent,
diff --git a/src/tracing/test/api_test_support.cc b/src/tracing/test/api_test_support.cc
index 0142d28..a03135d 100644
--- a/src/tracing/test/api_test_support.cc
+++ b/src/tracing/test/api_test_support.cc
@@ -27,6 +27,10 @@
 #include "test/test_helper.h"
 #endif
 
+#if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+#include <Windows.h>
+#endif
+
 namespace perfetto {
 namespace test {
 
diff --git a/tools/test_data b/tools/test_data
index cf8ae14..7dd8975 100755
--- a/tools/test_data
+++ b/tools/test_data
@@ -141,7 +141,7 @@
     subprocess.check_call(cmd)
     with open(fs.path + SUFFIX + '.swp', 'w') as f:
       f.write(fs.actual_digest)
-    os.rename(fs.path + SUFFIX + '.swp', fs.path + SUFFIX)
+    os.replace(fs.path + SUFFIX + '.swp', fs.path + SUFFIX)
     return fs
 
   map_concurrently(upload_one_file, files_to_upload)
@@ -194,7 +194,7 @@
     if digest != fs.expected_digest:
       raise Exception('Mismatching digest for %s. expected=%s, actual=%s' %
                       (uri, fs.expected_digest, digest))
-    os.rename(tmp_path, fs.path)
+    os.replace(tmp_path, fs.path)
     return fs
 
   map_concurrently(download_one_file, files_to_download)