Fix ImageDecoderBrowserTest.StartAndKillProcess flakyness and re-enable.

chrome/browser/image_decoder_browsertest.cc

 // Copyright 2015 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/image_decoder.h"
 
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/browser/browser_child_process_observer.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 90 matching lines @@
     base::ProcessHandle out_handle;
     if (!::DuplicateHandle(GetCurrentProcess(), handle,
                            GetCurrentProcess(), &out_handle,
                            0, FALSE, DUPLICATE_SAME_ACCESS)) {
       return;
     }
     handle = out_handle;
 #endif
 
     // Use a non-zero exit code so it counts as a crash.
-    EXPECT_TRUE(base::Process(handle).Terminate(1, true));
+    // Don't wait for the process after sending the termination signal
+    // (SIGTERM). According to POSIX, doing so causes the resulting zombie to be
+    // removed from the process table. However, Chromium treats an error on
+    // |waitpid| (in this case, ECHILD) as a "normal" termination and doesn't
+    // invoke the process host delegate's |OnProcessCrashed|.

[Lei Zhang, 2015/05/12 02:23:24]

nit: OnProcessCrashes()

[Anand Mistry (off Chromium), 2015/05/12 03:12:20]

Done. And I double-checked, it is OnProcessCrashed, not Crashes.

+    EXPECT_TRUE(base::Process(handle).Terminate(1, false));
     did_kill_ = true;
   }
 
   bool did_kill_;
   const base::string16 utility_process_name_;
 
   DISALLOW_COPY_AND_ASSIGN(KillProcessObserver);
 };
 
 }  // namespace
@@ skipping 28 matching lines @@
   test_request.reset();
   runner->Run();
 }
 
 // Killing the utility process counts as a crash. Thus the request fails.
 // If ImageDecoder did not handle the crash properly, the request never finishes
 // and this test would hang.
 // Note: This test is inherently racy because KillProcessObserver lives on the
 // UI thread but ImageDecoder does its work mainly on the IO thread. So the test
 // checks for both possible valid outcomes.
-// BUG(486194): Disabled due to flakyness.
-IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, DISABLED_StartAndKillProcess) {
+IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, StartAndKillProcess) {
   KillProcessObserver observer;
   scoped_refptr<content::MessageLoopRunner> runner =
       new content::MessageLoopRunner;
   TestImageRequest test_request(runner->QuitClosure());
   ImageDecoder::Start(&test_request, GetValidPngString());
   runner->Run();
   if (!test_request.decode_succeeded()) {
     // The UI thread won the race. Make sure the utility process did get killed.
     EXPECT_TRUE(observer.did_kill());
   }
   // Else the IO thread won the race and the image got decoded. Oh well.
 }