Only do aggressive metro viewer termination in tests

ash/test/ash_test_base.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 "ash/test/ash_test_base.h"
 
 #include <string>
 #include <vector>
 
 #include "ash/ash_switches.h"
@@ skipping 129 matching lines @@
 
 #if defined(OS_WIN)
   if (!command_line->HasSwitch(ash::switches::kForceAshToDesktop)) {
     if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
       ipc_thread_.reset(new base::Thread("test_metro_viewer_ipc_thread"));
       base::Thread::Options options;
       options.message_loop_type = base::MessageLoop::TYPE_IO;
       ipc_thread_->StartWithOptions(options);
       metro_viewer_host_.reset(
           new TestMetroViewerProcessHost(ipc_thread_->message_loop_proxy()));
-      CHECK(metro_viewer_host_->LaunchViewerAndWaitForConnection(
+      CHECK(metro_viewer_host_->LaunchViewerAndWaitForConnectionForTests(
           win8::test::kDefaultTestAppUserModelId));
       aura::RemoteWindowTreeHostWin* window_tree_host =
           aura::RemoteWindowTreeHostWin::Instance();
       CHECK(window_tree_host != NULL);
     }
     ash::WindowPositioner::SetMaximizeFirstWindow(true);
   }
 #endif
 }
 
@@ skipping 14 matching lines @@
   ash_test_helper_->TearDown();
 #if defined(OS_WIN)
   ui::test::SetUsePopupAsRootWindowForTest(false);
   // Kill the viewer process if we spun one up.
   metro_viewer_host_.reset();
 
   // Clean up any dangling viewer processes as the metro APIs sometimes leave
   // zombies behind. A default browser process in metro will have the
   // following command line arg so use that to avoid killing all processes named
   // win8::test::kDefaultTestExePath.
   const wchar_t kViewerProcessArgument[] = L"DefaultBrowserServer";

[ananta, 2014/09/19 19:55:16]

I think we can get rid of the code below which kills viewers. The destructor is
already doing that with this patch.

[Shrikant Kelkar, 2014/09/19 20:03:12]

I feel it's better to call kill/terminate from test rather than conditionally
from MetroViewerHost.. Your call.

   base::KillAllNamedProcessesWithArgument(win8::test::kDefaultTestExePath,
                                           kViewerProcessArgument);

[Shrikant Kelkar, 2014/09/19 19:42:28]

We already have some cleanup logic here, wondering why it wouldn't work? On the
other hand do we need to put this logic before test as well?

[scottmg, 2014/09/19 19:44:15]

Hmm, good point. Yes, we probably could just move this higher (because it's the
host.reset() that blocks).

Do we need to do the KillAll here, or is terminating the viewer that we know is
ours sufficient?

[Shrikant Kelkar, 2014/09/19 19:49:08]

KillAll with DefaultBrowserServer should terminate only viewer, I mean all
viewers.
So is it possible that somewhere Teardown path is not getting executed if some
middle test is waiting or is it always the first test that ends up waiting?

 #endif
 
   event_generator_.reset();
   // Some tests set an internal display id,
   // reset it here, so other tests will continue in a clean environment.
   gfx::Display::SetInternalDisplayId(gfx::Display::kInvalidDisplayID);
 }
 
 ui::test::EventGenerator& AshTestBase::GetEventGenerator() {
   if (!event_generator_) {
@@ skipping 138 matching lines @@
 
 void AshTestBase::UnblockUserSession() {
   Shell::GetInstance()->session_state_delegate()->UnlockScreen();
   SetSessionStarted(true);
   SetUserAddingScreenRunning(false);
 }
 
 
 }  // namespace test
 }  // namespace ash