Capture a report on failed browser rendez-vous.

chrome/browser/process_singleton_win.cc

 // Copyright (c) 2012 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/browser/process_singleton.h"
 
 #include <shellapi.h>
 #include <stddef.h>
 
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/process/process.h"
 #include "base/process/process_info.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/chrome_process_finder_win.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/ui/simple_message_box.h"
+#include "chrome/chrome_watcher/kasko_util.h"
+#include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/chromium_strings.h"
+#include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/wmi.h"
+#include "components/version_info/version_info.h"
 #include "content/public/common/result_codes.h"
 #include "net/base/escape.h"
+#include "third_party/kasko/kasko_features.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/win/hwnd_util.h"
 
 namespace {
 
 const char kLockfile[] = "lockfile";
 
 // A helper class that acquires the given |mutex| while the AutoLockMutex is in
 // scope.
 class AutoLockMutex {
@@ skipping 195 matching lines @@
     case chrome::NOTIFY_SUCCESS:
       return PROCESS_NOTIFIED;
     case chrome::NOTIFY_FAILED:
       remote_window_ = NULL;
       return PROCESS_NONE;
     case chrome::NOTIFY_WINDOW_HUNG:
       // Fall through and potentially terminate the hung browser.
       break;
   }
 
+  // The window is hung.
   DWORD process_id = 0;
   DWORD thread_id = ::GetWindowThreadProcessId(remote_window_, &process_id);
   if (!thread_id || !process_id) {
     remote_window_ = NULL;
     return PROCESS_NONE;
   }
+
+  // Get a handle to the process that created the window. This is racy. It's
+  // possible the original process exited and that the process id was recycled.
+  // Double check the window is still associated with an id.
   base::Process process = base::Process::Open(process_id);
+  process_id = 0;
+  thread_id = ::GetWindowThreadProcessId(remote_window_, &process_id);

[Sigurður Ásgeirsson, 2016/04/01 17:27:17]

this is still racy, as you have the same problem with the window handle.

Essentially the question you need to answer is whether the window handle you
have is really still the singleton for the profile dir, and then whether it's
owned by the process you opened.

[Sigurður Ásgeirsson, 2016/04/01 17:27:17]

It's also worth noting here that we nominate the owning thread of this window as
the root of the search for the culprit thread?

[manzagop (departed), 2016/04/04 21:38:37]

As discussed, it's hard to avoid the issues with HWND and/or PID recycling.
Instead, we compare the executable names, which is a sane defense against false
positives.

[manzagop (departed), 2016/04/04 21:38:38]

Done.

+  if (!thread_id || !process_id) {
+    remote_window_ = NULL;
+    return PROCESS_NONE;
+  }
+  DCHECK_EQ(process.Pid(), process_id);
 
-  // The window is hung. Scan for every window to find a visible one.
+#if BUILDFLAG(ENABLE_KASKO)
+#if BUILDFLAG(ENABLE_KASKO_FAILED_RDV_REPORTS)
+  // Capture a failed rendez-vous hang report of the other process. Kasko needs
+  // the exception context to live either in the dumper of the dumpee. This
+  // means we cannot rely on kasko reporters from either browser watcher, and
+  // instead spin up a new reporter. Note that we will exit shortly after
+  // capturing the report, meaning the report may not get uploaded by this
+  // reporter.
+  // TODO(manzagop): add a metric for the number of captured hang reports, for
+  //   comparison with uploaded count?
+
+  // Only report on canary (or unspecified).
+  const version_info::Channel channel = chrome::GetChannel();
+  if (channel == version_info::Channel::UNKNOWN ||
+      channel == version_info::Channel::CANARY) {
+    base::FilePath data_base_dir;
+    chrome::GetDefaultUserDataDirectory(&data_base_dir);
+    base::string16 endpoint =
+        L"chrome_kasko_hangs_" +
+        base::UintToString16(base::Process::Current().Pid());
+
+    bool launched_kasko =

[Sigurður Ásgeirsson, 2016/04/01 17:27:17]

it's worth adding some running commentary here, will e.g. ShutdownKaskoReporter
block for upload, or do you rely on another process or another instance of kasko
to do taht?

[manzagop (departed), 2016/04/04 21:38:37]

Done.

+        InitializeKaskoReporter(endpoint, data_base_dir.value().c_str());
+    if (launched_kasko) {
+      DumpHungProcess(thread_id, installer::kChromeChannelCanary, L"failed-rdv",
+                      process);
+      ShutdownKaskoReporter();
+    }
+  }
+#endif  // BUILDFLAG(ENABLE_KASKO_FAILED_RDV_REPORTS)
+#endif  // BUILDFLAG(ENABLE_KASKO)
+
+  // Scan for every window to find a visible one.
   bool visible_window = false;
   ::EnumThreadWindows(thread_id,
                       &BrowserWindowEnumeration,
                       reinterpret_cast<LPARAM>(&visible_window));
 
   // If there is a visible browser window, ask the user before killing it.
   if (visible_window && !should_kill_remote_process_callback_.Run()) {
     // The user denied. Quit silently.
     return PROCESS_NOTIFIED;
   }
@@ skipping 77 matching lines @@
   return window_.hwnd() != NULL;
 }
 
 void ProcessSingleton::Cleanup() {
 }
 
 void ProcessSingleton::OverrideShouldKillRemoteProcessCallbackForTesting(
     const ShouldKillRemoteProcessCallback& display_dialog_callback) {
   should_kill_remote_process_callback_ = display_dialog_callback;
 }