Attempt to fix the flakiness on the win8 builder on the waterfall.

There appear to be cases where in the metro viewer chrome process does not exit causing subsequent
ash tests to fail. One of the errors we have seen in this context is E_APPLICATION_ACTIVATION_TIMED_OUT.

Currently ash_unittests does not clean shutdown the viewer. It closes the channel and terminates the viewer
process via TerminateProcess. That call could fail if the viewer process has a thread in the kernel stack
which could lead to subsequent tests to fail.

Fix is to attempt a clean shutdown of the viewer process. For this we maintain the message filter for the
lifetime of the MetroViewerProcessHost class. We signal an event when the channel is actually closed in
the hosting process which should ensure that the viewer exits cleanly and also wait for the viewer to die. This code
only executes for tests.

BUG=340422
R=cpu@chromium.org, scottmg@chromium.org, cpu

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259080 0039d316-1c4b-4281-b951-d872f2087c98

win8/viewer/metro_viewer_process_host.cc

@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/process/process.h"
+#include "base/process/process_handle.h"
 #include "base/strings/string16.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/time/time.h"
@@ -21,6 +22,12 @@
 #include "ui/metro_viewer/metro_viewer_messages.h"
 #include "win8/viewer/metro_viewer_constants.h"
 
+namespace {
+
+const int kViewerProcessConnectionTimeoutSecs = 60;
+
+}  // namespace
+
 namespace win8 {
 
 MetroViewerProcessHost::InternalMessageFilter::InternalMessageFilter(
@@ -43,6 +50,26 @@ MetroViewerProcessHost::MetroViewerProcessHost(
 }
 
 MetroViewerProcessHost::~MetroViewerProcessHost() {
+  if (!channel_)
+    return;
+
+  base::ProcessId viewer_process_id = GetViewerProcessId();
+  channel_->Close();
+  if (message_filter_) {
+    // Wait for the viewer process to go away.
+    if (viewer_process_id != base::kNullProcessId) {
+      base::ProcessHandle viewer_process = NULL;
+      base::OpenProcessHandleWithAccess(
+          viewer_process_id,
+          PROCESS_QUERY_INFORMATION | SYNCHRONIZE,
+          &viewer_process);
+      if (viewer_process) {
+        ::WaitForSingleObject(viewer_process, INFINITE);
+        ::CloseHandle(viewer_process);
+      }
+    }
+    channel_->RemoveFilter(message_filter_);
+  }
 }
 
 base::ProcessId MetroViewerProcessHost::GetViewerProcessId() {
@@ -57,9 +84,8 @@ bool MetroViewerProcessHost::LaunchViewerAndWaitForConnection(
 
   channel_connected_event_.reset(new base::WaitableEvent(false, false));
 
-  scoped_refptr<InternalMessageFilter> message_filter(
-      new InternalMessageFilter(this));
-  channel_->AddFilter(message_filter);
+  message_filter_ = new InternalMessageFilter(this);
+  channel_->AddFilter(message_filter_);
 
   base::win::ScopedComPtr<IApplicationActivationManager> activator;
   HRESULT hr = activator.CreateInstance(CLSID_ApplicationActivationManager);
@@ -75,13 +101,9 @@ bool MetroViewerProcessHost::LaunchViewerAndWaitForConnection(
 
   // Having launched the viewer process, now we wait for it to connect.
   bool success =
-      channel_connected_event_->TimedWait(base::TimeDelta::FromSeconds(60));
+      channel_connected_event_->TimedWait(base::TimeDelta::FromSeconds(
+          kViewerProcessConnectionTimeoutSecs));
   channel_connected_event_.reset();
-
-  // |message_filter| is only used to signal |channel_connected_event_| above
-  // and can thus be removed after |channel_connected_event_| is no longer
-  // waiting.
-  channel_->RemoveFilter(message_filter);
   return success;
 }
 

win8/viewer/metro_viewer_process_host.h

@@ -96,6 +96,7 @@ class MetroViewerProcessHost : public IPC::Listener,
 
   scoped_ptr<IPC::ChannelProxy> channel_;
   scoped_ptr<base::WaitableEvent> channel_connected_event_;
+  scoped_refptr<InternalMessageFilter> message_filter_;
 
   DISALLOW_COPY_AND_ASSIGN(MetroViewerProcessHost);
 };