DevTools: don't show the unresponsive dialog for inspected tabs

content/browser/debugger/devtools_manager_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/time.h"
 #include "content/browser/debugger/devtools_client_host.h"
 #include "content/browser/debugger/devtools_manager.h"
 #include "content/browser/renderer_host/test_render_view_host.h"
+#include "content/browser/tab_contents/tab_contents_delegate.h"
+#include "content/browser/tab_contents/test_tab_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using base::TimeDelta;
+
 namespace {
 
 class TestDevToolsClientHost : public DevToolsClientHost {
  public:
   TestDevToolsClientHost()
       : last_sent_message(NULL),
         closed_(false) {
   }
 
   virtual ~TestDevToolsClientHost() {
@@ skipping 31 matching lines @@
  private:
   bool closed_;
 
   virtual void FrameNavigating(const std::string& url) {}
 
   DISALLOW_COPY_AND_ASSIGN(TestDevToolsClientHost);
 };
 
 int TestDevToolsClientHost::close_counter = 0;
 
+
+class TestTabContentsDelegate : public TabContentsDelegate {
+ public:
+  TestTabContentsDelegate() : renderer_unresponsive_received_(false) {}
+
+  // Notification that the tab is hung.
+  virtual void RendererUnresponsive(TabContents* source) {
+    renderer_unresponsive_received_ = true;
+  }
+
+  bool renderer_unresponsive_received() const {
+    return renderer_unresponsive_received_;
+  }
+
+ private:
+  bool renderer_unresponsive_received_;
+};
+
 }  // namespace
 
 class DevToolsManagerTest : public RenderViewHostTestHarness {
  public:
   DevToolsManagerTest() : RenderViewHostTestHarness() {
   }
 
  protected:
   virtual void SetUp() {
     RenderViewHostTestHarness::SetUp();
@@ skipping 32 matching lines @@
   manager->RegisterDevToolsClientHostFor(rvh(), &client_host);
   EXPECT_EQ(0, TestDevToolsClientHost::close_counter);
 
   IPC::Message m;
   manager->ForwardToDevToolsClient(rvh(), m);
   EXPECT_TRUE(&m == client_host.last_sent_message);
 
   client_host.Close();
   EXPECT_EQ(1, TestDevToolsClientHost::close_counter);
 }
+
+TEST_F(DevToolsManagerTest, NoUnresponsiveDialogInInspectedTab) {
+  TestRenderViewHost* inspected_rvh = rvh();
+  inspected_rvh->set_render_view_created(true);
+  EXPECT_FALSE(contents()->delegate());
+  TestTabContentsDelegate delegate;
+  contents()->set_delegate(&delegate);
+
+  scoped_refptr<DevToolsManager> manager(new DevToolsManager());
+  testing_browser_process_.get()->SetDevToolsManager(manager.get());

[Paweł Hajdan Jr., 2011/08/03 16:50:30]

How about making TestingBrowserProcess own DevToolsManager? That seems much
safer to me and also more following the existing convention.

[yurys, 2011/08/04 08:59:15]

Done.

+  ASSERT_EQ(manager.get(), DevToolsManager::GetInstance());
+
+  TestDevToolsClientHost client_host;
+  manager->RegisterDevToolsClientHostFor(inspected_rvh, &client_host);
+
+  // Start with a short timeout.
+  inspected_rvh->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(10));
+  // Wait long enough for first timeout and see if it fired.
+  MessageLoop::current()->PostDelayedTask(FROM_HERE,
+                                          new MessageLoop::QuitTask(), 10);

[pfeldman1, 2011/08/03 16:28:50]

I suspect it'll flake due to timer resolution/error. Is there a way to use mock
timer?

[Paweł Hajdan Jr., 2011/08/03 16:50:30]

Yeah, this is a bit suspicious. I'd suggest removing the "not received" test
here (there's not really a good way to write those), and also see my suggestion
below.

[yurys, 2011/08/04 08:59:15]

The whole point of this test is to check that the unresponsive dialog is not
shown when the hung tab is being debugged. Removing this means that we can
remove this test completely and leave this functionality untested.

[yurys, 2011/08/04 08:59:15]

This is how StopAndStartHangMonitorTimeout
test(http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/content/browser/renderer_host/render_widget_host_unittest.cc&exact_package=chromium&q=StopAndStartHangMonitorTimeout&type=cs&l=722)
works and the only guarantee we need  here is that the task posted later will
fire later.

+  MessageLoop::current()->Run();
+  EXPECT_FALSE(delegate.renderer_unresponsive_received());
+
+  // Now close devtools and check that the notification is delivered.
+  client_host.Close();
+  // Start with a short timeout.
+  inspected_rvh->StartHangMonitorTimeout(TimeDelta::FromMilliseconds(10));
+  // Wait long enough for first timeout and see if it fired.
+  MessageLoop::current()->PostDelayedTask(FROM_HERE,

[Paweł Hajdan Jr., 2011/08/03 16:50:30]

How about just waiting for the hang monitor to kick in, i.e. wait for an event
and not for a fixed amount of time?

[yurys, 2011/08/04 08:59:15]

It would work if we were testing the watchdog timer but here we'd like to make
sure that the event is handled by the tab_contets properly after devtools client
has detached from the tab.

+                                          new MessageLoop::QuitTask(), 10);
+  MessageLoop::current()->Run();
+  EXPECT_TRUE(delegate.renderer_unresponsive_received());
+
+  testing_browser_process_.get()->SetDevToolsManager(NULL);
+  contents()->set_delegate(NULL);
+}