Revise on-top window data collection for interative_ui_tests.

Tests in the interative_ui_tests fixture are known to timeout if there's
an always-on-top window on the desktop. This change introduces a few
improvements to the existing data collection strategy to help diagnose
the provenance of these always-on-top windows.

- Checks for always-on-top windows are moved out of the test child
  processes and up to the parent test proc. This is a more reliable
  place to log any info, as such log messages are more likely to make it
  into the main swarming log output. This is a result of the way
  GetTestOutputSnippet pulls what it thinks is the only relevant test
  output -- it drops messages emitted outside of the test body.

- Scans for always-on-top windows now take place before running any test
  in the shard and after any test that times out.

- A screen snapshot is saved if any always-on-top window is found, even
  in the case of dialogs that will be closed.

- An attempt is made to find and emit the command lines for all
  processes in the process hierarchy of the proc owning an always-on-top
  window.

- All details are emitted in a single log message. For example:

There is an always on top window on the desktop after this test timed out. This may have been caused by this test or a previous test and may cause flakes; window class name: ConsoleWindowClass; subprocess command line: "".\interactive_ui_tests.exe" --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=AutofillInteractiveTest.AutofillViaClick --single_process --snapshot-output-dir="e:\b\s\w\iomhwbd8" --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir6396_16240\results6396_26680\test_results.xml" --test-launcher-summary-output="e:\b\s\w\iomhwbd8\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir6396_16240\d6396_27306""; owning process lineage: (process_id: 2000, command_line: "C:\Windows\system32\cmd.exe /c ""C:\Users\chrome-bot\AppData\Roaming\Microsoft\Windows\Start Menu\Programs\Startup\run_swarm_bot.bat" ""), (process_id: 1496, command_line: "C:\Windows\Explorer.EXE"); screen snapshot saved to file: "e:\b\s\w\iomhwbd8\ss_20171030114011_636.png";

BUG=764415
R=sky@chromium.org

Change-Id: I1aed3a41542680b197c7856e1f5d9ec9af6e3774
Reviewed-on: https://chromium-review.googlesource.com/737641Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513095}

chrome/test/BUILD.gn

@@ -493,6 +493,10 @@ if (!is_android) {
       "base/interactive_test_utils_mac.mm",
       "base/interactive_test_utils_win.cc",
       "base/interactive_ui_tests_main.cc",
+      "base/process_inspector_win.cc",
+      "base/process_inspector_win.h",
+      "base/save_desktop_snapshot_win.cc",
+      "base/save_desktop_snapshot_win.h",
       "base/view_event_test_platform_part.h",
       "base/view_event_test_platform_part_chromeos.cc",
       "base/view_event_test_platform_part_default.cc",

chrome/test/base/always_on_top_window_killer_win.h

@@ -5,16 +5,23 @@
 #ifndef CHROME_TEST_BASE_ALWAYS_ON_TOP_WINDOW_KILLER_WIN_H_
 #define CHROME_TEST_BASE_ALWAYS_ON_TOP_WINDOW_KILLER_WIN_H_
 
+namespace base {
+class CommandLine;
+}
+
 enum class RunType {
-  // Indicates cleanup is happening before the test run.
-  BEFORE_TEST,
+  // Indicates cleanup is happening before sharded tests are run.
+  BEFORE_SHARD,
 
-  // Indicates cleanup is happening after the test run.
-  AFTER_TEST,
+  // Indicates cleanup is happening after a test subprocess has timed out.
+  AFTER_TEST_TIMEOUT,
 };
 
 // Logs if there are any always on top windows, and if one is a system dialog
-// closes it.
-void KillAlwaysOnTopWindows(RunType run_type);
+// closes it. |child_command_line|, if non-null, is the command line of the
+// test subprocess that timed out.
+void KillAlwaysOnTopWindows(
+    RunType run_type,
+    const base::CommandLine* child_command_line = nullptr);
 
 #endif  // CHROME_TEST_BASE_ALWAYS_ON_TOP_WINDOW_KILLER_WIN_H_

chrome/test/base/interactive_ui_tests_main.cc

@@ -51,7 +51,6 @@ class InteractiveUITestSuite : public ChromeTestSuite {
 #elif defined(USE_AURA)
 #if defined(OS_WIN)
     com_initializer_.reset(new base::win::ScopedCOMInitializer());
-    KillAlwaysOnTopWindows(RunType::BEFORE_TEST);
 #endif
 
 #if defined(OS_LINUX)
@@ -70,7 +69,6 @@ class InteractiveUITestSuite : public ChromeTestSuite {
 
   void Shutdown() override {
 #if defined(OS_WIN)
-    KillAlwaysOnTopWindows(RunType::AFTER_TEST);
     com_initializer_.reset();
 #endif
   }
@@ -81,6 +79,37 @@ class InteractiveUITestSuite : public ChromeTestSuite {
 #endif
 };
 
+class InteractiveUITestLauncherDelegate : public ChromeTestLauncherDelegate {
+ public:
+  explicit InteractiveUITestLauncherDelegate(ChromeTestSuiteRunner* runner)
+      : ChromeTestLauncherDelegate(runner) {}
+
+ protected:
+  // content::TestLauncherDelegate:
+  void PreSharding() override {
+    ChromeTestLauncherDelegate::PreSharding();
+#if defined(OS_WIN)
+    // Check for any always-on-top windows present before any tests are run.
+    // Take a snapshot if any are found and attempt to close any that are system
+    // dialogs.
+    KillAlwaysOnTopWindows(RunType::BEFORE_SHARD);
+#endif
+  }
+
+  void OnTestTimedOut(const base::CommandLine& command_line) override {
+#if defined(OS_WIN)
+    // Check for any always-on-top windows present before terminating the test.
+    // Take a snapshot if any are found and attempt to close any that are system
+    // dialogs.
+    KillAlwaysOnTopWindows(RunType::AFTER_TEST_TIMEOUT, &command_line);
+#endif
+    ChromeTestLauncherDelegate::OnTestTimedOut(command_line);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(InteractiveUITestLauncherDelegate);
+};
+
 class InteractiveUITestSuiteRunner : public ChromeTestSuiteRunner {
  public:
   int RunTestSuite(int argc, char** argv) override {
@@ -106,6 +135,6 @@ int main(int argc, char** argv) {
   size_t parallel_jobs = 1U;
 
   InteractiveUITestSuiteRunner runner;
-  ChromeTestLauncherDelegate delegate(&runner);
+  InteractiveUITestLauncherDelegate delegate(&runner);
   return LaunchChromeTests(parallel_jobs, &delegate, argc, argv);
 }

chrome/test/base/process_inspector_win.cc

+// Copyright 2017 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.
+
+#include "chrome/test/base/process_inspector_win.h"
+
+#include <winternl.h>
+
+#include "base/process/process.h"
+#include "base/win/windows_version.h"
+
+namespace {
+
+// Certain Windows types that depend on the word size of the OS (rather than the
+// size of the current process) are defined here.
+
+// PROCESS_BASIC_INFORMATION.
+template <class Traits>
+struct ProcessInformation {
+  using RemotePointer = typename Traits::RemotePointer;
+
+  DWORD exit_status() const { return static_cast<DWORD>(exit_status_); }
+  RemotePointer peb_base_address() const { return peb_base_address_; }
+  RemotePointer affinity_mask() const { return affinity_mask_; }
+  int base_priority() const { return static_cast<int>(base_priority_); }
+  DWORD unique_process_id() const { return static_cast<DWORD>(unique_pid_); }
+  DWORD inherited_from_unique_process_id() const {
+    return static_cast<DWORD>(inherited_from_unique_process_id_);
+  }
+
+ private:
+  RemotePointer exit_status_;
+  RemotePointer peb_base_address_;
+  RemotePointer affinity_mask_;
+  RemotePointer base_priority_;
+  RemotePointer unique_pid_;
+  RemotePointer inherited_from_unique_process_id_;
+};
+
+// A subset of a process's environment block.
+template <class Traits>
+struct ProcessExecutionBlock {
+  using RemotePointer = typename Traits::RemotePointer;
+
+  uint8_t InheritedAddressSpace;
+  uint8_t ReadImageFileExecOptions;
+  uint8_t BeingDebugged;
+  uint8_t ProcessFlags;
+  uint8_t Padding[4];
+  RemotePointer Mutant;
+  RemotePointer ImageBaseAddress;
+  RemotePointer Ldr;
+  RemotePointer ProcessParameters;  // RtlUserProcessParameters
+};
+
+// UNICODE_STRING.
+template <class Traits>
+struct UnicodeString {
+  using RemotePointer = typename Traits::RemotePointer;
+
+  uint16_t Length;
+  uint16_t MaximumLength;
+  RemotePointer Buffer;
+};
+
+// CURDIR.
+template <class Traits>
+struct CurDir {
+  using RemotePointer = typename Traits::RemotePointer;
+
+  UnicodeString<Traits> DosPath;
+  RemotePointer Handle;
+};
+
+// RTL_USER_PROCESS_PARAMETERS.
+template <class Traits>
+struct RtlUserProcessParameters {
+  using RemotePointer = typename Traits::RemotePointer;
+
+  uint32_t MaximumLength;
+  uint32_t Length;
+  uint32_t Flags;
+  uint32_t DebugFlags;
+  RemotePointer ConsoleHandle;
+  uint32_t ConsoleFlags;
+  RemotePointer StandardInput;
+  RemotePointer StandardOutput;
+  RemotePointer StandardError;
+  CurDir<Traits> CurrentDirectory;
+  UnicodeString<Traits> DllPath;
+  UnicodeString<Traits> ImagePathName;
+  UnicodeString<Traits> CommandLine;
+};
+
+// A concrete ProcessInspector that can read from another process based on the
+// architecture. |Traits| specifies traits based on the OS architecture.
+template <class Traits>
+class Inspector : public ProcessInspector {
+ public:
+  Inspector();
+
+  // ProcessInspector:
+  DWORD GetParentPid() const override;
+  const base::string16& command_line() const override;
+
+ private:
+  // ProcessInspector:
+  bool Inspect(const base::Process& process) override;
+
+  ProcessInformation<Traits> process_basic_information_;
+  ProcessExecutionBlock<Traits> peb_;
+  RtlUserProcessParameters<Traits> process_parameters_;
+  base::string16 command_line_;
+
+  DISALLOW_COPY_AND_ASSIGN(Inspector);
+};
+
+#if !defined(_WIN64)
+// Traits for a 32-bit process running in WoW.
+struct Wow64Traits {
+  // The name of the ntdll function to query process information.
+  static const char kQueryProcessInformationFunctionName[];
+
+  // The type of a pointer to the read process memory function.
+  using ReadMemoryFn =
+      NTSTATUS(NTAPI*)(HANDLE, uint64_t, void*, uint64_t, uint64_t*);
+
+  // An unsigned integer type matching the size of a pointer in the remote
+  // process.
+  using RemotePointer = uint64_t;
+
+  // Returns the function to read memory from a remote process.
+  static ReadMemoryFn GetReadMemoryFn() {
+    return reinterpret_cast<ReadMemoryFn>(::GetProcAddress(
+        ::GetModuleHandle(L"ntdll.dll"), "NtWow64ReadVirtualMemory64"));
+  }
+
+  // Reads |buffer_size| bytes from |handle|'s process at |address| into
+  // |buffer| using |fn|. Returns true on success.
+  static bool ReadMemory(ReadMemoryFn fn,
+                         HANDLE handle,
+                         RemotePointer address,
+                         void* buffer,
+                         RemotePointer buffer_size) {
+    NTSTATUS status = fn(handle, address, buffer, buffer_size, nullptr);
+    if (NT_SUCCESS(status))
+      return true;
+    LOG(ERROR) << "Failed to read process memory with status " << std::hex
+               << status << ".";
+    return false;
+  }
+};
+
+// static
+constexpr char Wow64Traits::kQueryProcessInformationFunctionName[] =
+    "NtWow64QueryInformationProcess64";
+
+#endif
+
+// Traits for a 32-bit process running on 32-bit Windows, or a 64-bit process
+// running on 64-bit Windows.
+struct NormalTraits {
+  // The name of the ntdll function to query process information.
+  static const char kQueryProcessInformationFunctionName[];
+
+  // The type of a pointer to the read process memory function.
+  using ReadMemoryFn = decltype(&::ReadProcessMemory);
+
+  // An unsigned integer type matching the size of a pointer in the remote
+  // process.
+  using RemotePointer = uintptr_t;
+
+  // Returns the function to read memory from a remote process.
+  static ReadMemoryFn GetReadMemoryFn() { return &::ReadProcessMemory; }
+
+  // Reads |buffer_size| bytes from |handle|'s process at |address| into
+  // |buffer| using |fn|. Returns true on success.
+  static bool ReadMemory(ReadMemoryFn fn,
+                         HANDLE handle,
+                         RemotePointer address,
+                         void* buffer,
+                         RemotePointer buffer_size) {
+    BOOL result = fn(handle, reinterpret_cast<const void*>(address), buffer,
+                     buffer_size, nullptr);
+    if (result)
+      return true;
+    PLOG(ERROR) << "Failed to read process memory";
+    return false;
+  }
+};
+
+// static
+constexpr char NormalTraits::kQueryProcessInformationFunctionName[] =
+    "NtQueryInformationProcess";
+
+template <class Traits>
+Inspector<Traits>::Inspector() = default;
+
+template <class Traits>
+DWORD Inspector<Traits>::GetParentPid() const {
+  return process_basic_information_.inherited_from_unique_process_id();
+}
+
+template <class Traits>
+const base::string16& Inspector<Traits>::command_line() const {
+  return command_line_;
+}
+
+template <class Traits>
+bool Inspector<Traits>::Inspect(const base::Process& process) {
+  auto query_information_process_fn =
+      reinterpret_cast<decltype(&::NtQueryInformationProcess)>(
+          ::GetProcAddress(::GetModuleHandle(L"ntdll.dll"),
+                           Traits::kQueryProcessInformationFunctionName));
+  typename Traits::ReadMemoryFn read_memory_fn = Traits::GetReadMemoryFn();
+
+  if (!query_information_process_fn)
+    return false;
+  ULONG in_len = sizeof(process_basic_information_);
+  ULONG out_len = 0;
+  NTSTATUS status = query_information_process_fn(
+      process.Handle(), ProcessBasicInformation, &process_basic_information_,
+      in_len, &out_len);
+  if (NT_ERROR(status) || out_len != in_len)
+    return false;
+
+  if (!Traits::ReadMemory(read_memory_fn, process.Handle(),
+                          process_basic_information_.peb_base_address(), &peb_,
+                          sizeof(peb_))) {
+    return false;
+  }
+  if (!Traits::ReadMemory(read_memory_fn, process.Handle(),
+                          peb_.ProcessParameters, &process_parameters_,
+                          sizeof(process_parameters_))) {
+    return false;
+  }
+  if (process_parameters_.CommandLine.Length) {
+    command_line_.resize(process_parameters_.CommandLine.Length /
+                         sizeof(wchar_t));
+    if (!Traits::ReadMemory(read_memory_fn, process.Handle(),
+                            process_parameters_.CommandLine.Buffer,
+                            &command_line_[0],
+                            process_parameters_.CommandLine.Length)) {
+      command_line_.clear();
+      return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
+// static
+std::unique_ptr<ProcessInspector> ProcessInspector::Create(
+    const base::Process& process) {
+  std::unique_ptr<ProcessInspector> inspector;
+#if !defined(_WIN64)
+  using base::win::OSInfo;
+  if (OSInfo::GetInstance()->wow64_status() == OSInfo::WOW64_ENABLED)
+    inspector = std::make_unique<Inspector<Wow64Traits>>();
+#endif
+  if (!inspector)
+    inspector = std::make_unique<Inspector<NormalTraits>>();
+  if (!inspector->Inspect(process))
+    inspector.reset();
+  return inspector;
+}

chrome/test/base/process_inspector_win.h

+// Copyright 2017 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.
+
+#ifndef CHROME_TEST_BASE_PROCESS_INSPECTOR_WIN_H_
+#define CHROME_TEST_BASE_PROCESS_INSPECTOR_WIN_H_
+
+#include <windows.h>
+
+#include <memory>
+
+#include "base/macros.h"
+#include "base/strings/string16.h"
+
+namespace base {
+class Process;
+}
+
+// An inspector that can read properties of a remote process.
+class ProcessInspector {
+ public:
+  // Returns an instance that reads data from |process|, which must have been
+  // opened with at least PROCESS_VM_READ access rights. Returns null in case of
+  // any error.
+  static std::unique_ptr<ProcessInspector> Create(const base::Process& process);
+
+  virtual ~ProcessInspector() = default;
+
+  // Returns the parent process PID of the process.
+  virtual DWORD GetParentPid() const = 0;
+
+  // Returns the command line of the process.
+  virtual const base::string16& command_line() const = 0;
+
+ protected:
+  ProcessInspector() = default;
+
+ private:
+  // Inspects |process|, returning true if all inspections succeed.
+  virtual bool Inspect(const base::Process& process) = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(ProcessInspector);
+};
+
+#endif  // CHROME_TEST_BASE_PROCESS_INSPECTOR_WIN_H_

chrome/test/base/save_desktop_snapshot_win.cc

+// Copyright 2017 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.
+
+#include "chrome/test/base/save_desktop_snapshot_win.h"
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/files/file.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/run_loop.h"
+#include "base/strings/stringprintf.h"
+#include "base/time/time.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h"
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
+#include "ui/gfx/codec/png_codec.h"
+
+namespace {
+
+// A worker that captures a single frame from a webrtc::DesktopCapturer and then
+// runs a callback when done.
+class CaptureWorker : public webrtc::DesktopCapturer::Callback {
+ public:
+  CaptureWorker(std::unique_ptr<webrtc::DesktopCapturer> capturer,
+                base::Closure on_done)
+      : capturer_(std::move(capturer)), on_done_(std::move(on_done)) {
+    capturer_->Start(this);
+    capturer_->CaptureFrame();
+  }
+
+  // Returns the frame that was captured or null in case of failure.
+  std::unique_ptr<webrtc::DesktopFrame> TakeFrame() {
+    return std::move(frame_);
+  }
+
+ private:
+  // webrtc::DesktopCapturer::Callback:
+  void OnCaptureResult(webrtc::DesktopCapturer::Result result,
+                       std::unique_ptr<webrtc::DesktopFrame> frame) override {
+    if (result == webrtc::DesktopCapturer::Result::SUCCESS)
+      frame_ = std::move(frame);
+    on_done_.Run();
+  }
+
+  std::unique_ptr<webrtc::DesktopCapturer> capturer_;
+  base::Closure on_done_;
+  std::unique_ptr<webrtc::DesktopFrame> frame_;
+  DISALLOW_COPY_AND_ASSIGN(CaptureWorker);
+};
+
+// Captures and returns a snapshot of the screen, or an empty bitmap in case of
+// error.
+SkBitmap CaptureScreen() {
+  auto options = webrtc::DesktopCaptureOptions::CreateDefault();
+  options.set_disable_effects(false);
+  options.set_allow_directx_capturer(true);
+  options.set_allow_use_magnification_api(false);
+  std::unique_ptr<webrtc::DesktopCapturer> capturer(
+      webrtc::DesktopCapturer::CreateScreenCapturer(options));
+
+  // Grab a single frame.
+  std::unique_ptr<webrtc::DesktopFrame> frame;
+  {
+    // While webrtc::DesktopCapturer seems to be synchronous, comments in its
+    // implementation seem to indicate that it may require a UI message loop on
+    // its thread.
+    base::RunLoop run_loop;
+    CaptureWorker worker(std::move(capturer), run_loop.QuitClosure());
+    run_loop.Run();
+    frame = worker.TakeFrame();
+  }
+
+  if (!frame)
+    return SkBitmap();
+
+  // Create an image from the frame.
+  SkBitmap result;
+  result.allocN32Pixels(frame->size().width(), frame->size().height(), true);
+  memcpy(result.getAddr32(0, 0), frame->data(),
+         frame->size().width() * frame->size().height() *
+             webrtc::DesktopFrame::kBytesPerPixel);
+  return result;
+}
+
+}  // namespace
+
+base::FilePath SaveDesktopSnapshot(const base::FilePath& output_dir) {
+  // Create the output file.
+  base::Time::Exploded exploded;
+  base::Time::Now().LocalExplode(&exploded);
+  base::FilePath output_path(
+      output_dir.Append(base::FilePath(base::StringPrintf(
+          L"ss_%4d%02d%02d%02d%02d%02d_%03d.png", exploded.year, exploded.month,
+          exploded.day_of_month, exploded.hour, exploded.minute,
+          exploded.second, exploded.millisecond))));
+  base::File file(output_path, base::File::FLAG_CREATE |
+                                   base::File::FLAG_WRITE |
+                                   base::File::FLAG_SHARE_DELETE |
+                                   base::File::FLAG_CAN_DELETE_ON_CLOSE);
+  if (!file.IsValid()) {
+    if (file.error_details() == base::File::FILE_ERROR_EXISTS) {
+      LOG(INFO) << "Skipping screen snapshot since it is already present: "
+                << output_path.BaseName();
+    } else {
+      LOG(ERROR) << "Failed to create snapshot output file \"" << output_path
+                 << "\" with error " << file.error_details();
+    }
+    return base::FilePath();
+  }
+
+  // Delete the output file in case of any error.
+  file.DeleteOnClose(true);
+
+  // Take the snapshot and encode it.
+  SkBitmap screen = CaptureScreen();
+  if (screen.drawsNothing()) {
+    LOG(ERROR) << "Failed to capture a frame of the screen for a snapshot.";
+    return base::FilePath();
+  }
+
+  std::vector<unsigned char> encoded;
+  if (!gfx::PNGCodec::EncodeBGRASkBitmap(CaptureScreen(), false, &encoded)) {
+    LOG(ERROR) << "Failed to PNG encode screen snapshot.";
+    return base::FilePath();
+  }
+
+  // Write it to disk.
+  const int to_write = base::checked_cast<int>(encoded.size());
+  int written =
+      file.WriteAtCurrentPos(reinterpret_cast<char*>(encoded.data()), to_write);
+  if (written != to_write) {
+    LOG(ERROR) << "Failed to write entire snapshot to file";
+    return base::FilePath();
+  }
+
+  // Keep the output file.
+  file.DeleteOnClose(false);
+  return output_path;
+}

chrome/test/base/save_desktop_snapshot_win.h

+// Copyright 2017 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.
+
+#ifndef CHROME_TEST_BASE_SAVE_DESKTOP_SNAPSHOT_WIN_H_
+#define CHROME_TEST_BASE_SAVE_DESKTOP_SNAPSHOT_WIN_H_
+
+#include "base/files/file_path.h"
+
+// Saves a snapshot of the desktop to a file in |output_dir|, returning the path
+// to the file if created. An empty path is returned if no new snapshot is
+// created.
+base::FilePath SaveDesktopSnapshot(const base::FilePath& output_dir);
+
+#endif  // CHROME_TEST_BASE_SAVE_DESKTOP_SNAPSHOT_WIN_H_