Fix ImageDecoderBrowserTest.StartAndKillProcess flakyness and re-enable.

chrome/browser/image_decoder_browsertest.cc

Index: chrome/browser/image_decoder_browsertest.cc
diff --git a/chrome/browser/image_decoder_browsertest.cc b/chrome/browser/image_decoder_browsertest.cc
index 19e5937b37e06dae83ec38372dcbb1a06da2b637..01785b3a5a3fc4ed497762466b794f243cdda6fb 100644
--- a/chrome/browser/image_decoder_browsertest.cc
+++ b/chrome/browser/image_decoder_browsertest.cc
@@ -108,7 +108,12 @@ class KillProcessObserver : public content::BrowserChildProcessObserver {
 #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 kill signal. According to

[Lei Zhang, 2015/05/11 23:09:19]

By 'kill signal', did you mean SIGTERM?

[Anand Mistry (off Chromium), 2015/05/12 00:05:31]

Yes.

+    // 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 detegate's |OnProcessCrashed|.

[Lei Zhang, 2015/05/11 23:09:19]

nit: typo

[Anand Mistry (off Chromium), 2015/05/12 00:05:31]

Done.

+    EXPECT_TRUE(base::Process(handle).Terminate(1, false));

[Lei Zhang, 2015/05/11 23:09:19]

Maybe we should post this to the IO thread, so the kill() calls don't race
against each other.

[Anand Mistry (off Chromium), 2015/05/12 00:05:31]

I don't think it's possible for the two to race. If this functions runs, it
means the UI message loop is still looping, which means the request's response
functions haven't been called (and quit the loop), which means there are still
requests in the image decoder's request map, which means the image decoder won't
try to kill the decoder process. Phew, that was a mouthful.

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

I was thinking if we had left this as true, then the code runs waitpid() on the
UI thread, but at the same time, the IO thread is calling
GetChildTerminationStatus() which may also involve calling kill() and waitpid().
But ya, I guess we don't really need to wait, so n/m.

     did_kill_ = true;
   }
 
@@ -157,8 +162,7 @@ IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, StartAndDestroy) {
 // 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;