Increase timer in render_widget_host_unittest to reduce flakiness in valgrind

content/browser/renderer_host/render_widget_host_unittest.cc

 // Copyright (c) 2011 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 "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/shared_memory.h"
 #include "base/timer.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/renderer_host/backing_store.h"
@@ skipping 731 matching lines @@
   // Start with a short timeout, then stop it.
   host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(10));
   host_->StopHangMonitorTimeout();
 
   // Start it again to ensure it still works.
   EXPECT_FALSE(host_->unresponsive_timer_fired());
   host_->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(10));
 
   // Wait long enough for first timeout and see if it fired.
   MessageLoop::current()->PostDelayedTask(FROM_HERE,
-                                          new MessageLoop::QuitTask(), 10);
+                                          new MessageLoop::QuitTask(), 40);

[jam, 2011/11/15 00:14:03]

this is really really flaky, independent of the timeout value picked. Whatever
machine this runs on can be doing other stuff in the background, and this would
fail randomly, adding work to sheriffs/trybot runners/commit queue etc. It's
better not to have a test than to have one that picks a constant to sleep. Is
there no other event/notification to watch for?

[Charlie Reis, 2011/11/15 00:23:05]

The point of this test is to test a timer.  If you set the timer to expire in N
milliseconds (line 748), then set another timer to expire in M >= N
milliseconds, the second timer shouldn't fire first.

If there are notifications to listen for, though, I agree that would be better.

[jam, 2011/11/15 00:37:58]

Excuse the naive question, but if the point of the test is to test timer
support, shouldn't we be testing this at the lowest level, which is Timer's
unittest in base/timer_unittest.cc?

[Charlie Reis, 2011/11/15 00:45:05]

Hopefully Timer is already tested there.

The RenderWidget's logic for starting and stopping its hang monitor timer is
complex in itself, which is why this test was added.  Thus, this test starts,
stops, and starts the hang monitor timer and makes sure that it still fires. 
Just testing Timer would miss bugs in RenderWidget (which we've had).

[Paweł Hajdan Jr., 2011/11/15 09:01:31]

Thanks for adding me. I don't know the code tested well. I think that if the
timer runs on a separate thread, it's definitely racy no matter the timeout you
pick here, and if it runs on the same thread it depends on the semantics of
MessageLoop and delayed tasks, which I also don't know very well, but then it
has a chance to work (and is probably very desirable to work that way).

In general I'm not a fan of tests that use timers or are otherwise
timing-dependent, but after reading the discussion it seems we need some kind of
"integration test" here.

LGTM, and feel free to let me know how the hang monitor timeout works, I might
have some ideas to improve testability.

   MessageLoop::current()->Run();
   EXPECT_TRUE(host_->unresponsive_timer_fired());
 }