Revert 109984 - Tests for WebUI Hung Renderer Dialog

Adds a WebUIBrowserTest for WebUI Hung Renderer Dialog.

Also modifies HungRendererDialog to allow it to be called in a test without killing processes or restarting hang timers.

See previous attempt: http://codereview.chromium.org/8372042
See revert of that attempt: http://codereview.chromium.org/8497001/

BUG=102073
TEST=browser_tests HungRendererDialogUITest.*


Review URL: http://codereview.chromium.org/8462015

TBR=wyck@chromium.org
Review URL: http://codereview.chromium.org/8510071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@109989 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/resources/hung_renderer_dialog.js

@@ -56,7 +56,7 @@ cr.define('hungRendererDialog', function() {
 
     if (cr.isViews)
       forEach(document.querySelectorAll('.button-strip'), reverseChildren);
-    chrome.send('requestTabContentsList');
+    chrome.send('requestTabContentsList')
   }
 
   /**
@@ -74,7 +74,7 @@ cr.define('hungRendererDialog', function() {
       listItem.style.backgroundImage = url('chrome://favicon/size/32/' +
                                            tabDetailsList[i].url);
       listItem.textContent = tabDetailsList[i].title;
-      $('tab-table').appendChild(listItem);
+      $('tab-table').appendChild(listItem)
     }
   }
 

chrome/browser/ui/webui/hung_renderer_dialog.cc

@@ -35,7 +35,7 @@ namespace browser {
 
 void ShowHungRendererDialog(TabContents* contents) {
 #if defined(OS_CHROMEOS) || defined(USE_AURA)
-  HungRendererDialog::ShowHungRendererDialog(contents, true);
+  HungRendererDialog::ShowHungRendererDialog(contents);
 #else
   // TODO(rbyers): Remove IsMoreWebUI check once we decide for sure which
   // platforms will use the WebUI version of this dialog.
@@ -63,7 +63,12 @@ void HideHungRendererDialog(TabContents* contents) {
 // HungRendererDialog public static methods
 
 void HungRendererDialog::ShowHungRendererDialog(TabContents* contents) {
-  ShowHungRendererDialogInternal(contents, true);
+  if (!logging::DialogsAreSuppressed()) {
+    if (g_instance)
+      return;
+    g_instance = new HungRendererDialog();
+    g_instance->ShowDialog(contents);
+  }
 }
 
 void HungRendererDialog::HideHungRendererDialog(TabContents* contents) {
@@ -71,49 +76,16 @@ void HungRendererDialog::HideHungRendererDialog(TabContents* contents) {
     g_instance->HideDialog(contents);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// HungRendererDialog::TabContentsObserverImpl
-
-HungRendererDialog::TabContentsObserverImpl::TabContentsObserverImpl(
-    HungRendererDialog* dialog,
-    TabContents* contents)
-    : TabContentsObserver(contents),
-      contents_(contents),
-      dialog_(dialog) {
-}
-
-void HungRendererDialog::TabContentsObserverImpl::RenderViewGone() {
-  dialog_->HideDialog(contents_);
-}
-
-void HungRendererDialog::TabContentsObserverImpl::TabContentsDestroyed(
-    TabContents* tab) {
-  dialog_->HideDialog(contents_);
-}
 
 ////////////////////////////////////////////////////////////////////////////////
 // HungRendererDialog private methods
 
-HungRendererDialog::HungRendererDialog(bool is_enabled)
+HungRendererDialog::HungRendererDialog()
     : contents_(NULL),
       handler_(NULL),
-      is_enabled_(is_enabled),
       window_(NULL) {
 }
 
-HungRendererDialog::~HungRendererDialog() {
-}
-
-void HungRendererDialog::ShowHungRendererDialogInternal(TabContents* contents,
-                                                        bool is_enabled) {
-  if (!logging::DialogsAreSuppressed()) {
-    if (g_instance)
-      return;
-    g_instance = new HungRendererDialog(is_enabled);
-    g_instance->ShowDialog(contents);
-  }
-}
-
 void HungRendererDialog::ShowDialog(TabContents* contents) {
   DCHECK(contents);
   contents_ = contents;
@@ -121,7 +93,6 @@ void HungRendererDialog::ShowDialog(TabContents* contents) {
   DCHECK(browser);
   handler_ = new HungRendererDialogHandler(contents_);
   window_ = browser->BrowserShowHtmlDialog(this, NULL);
-  contents_observer_.reset(new TabContentsObserverImpl(this, contents_));
 }
 
 void HungRendererDialog::HideDialog(TabContents* contents) {
@@ -133,7 +104,6 @@ void HungRendererDialog::HideDialog(TabContents* contents) {
   // Settings |contents_| to NULL prevents the hang monitor from restarting.
   // We do this because the close dialog handler runs whether it is trigged by
   // the user closing the box, or by being closed externally with widget->Close.
-  contents_observer_.reset();
   contents_ = NULL;
   DCHECK(handler_);
   handler_->CloseDialog();
@@ -165,27 +135,25 @@ std::string HungRendererDialog::GetDialogArgs() const {
 }
 
 void HungRendererDialog::OnDialogClosed(const std::string& json_retval) {
-  if (is_enabled_) {
-    // Figure out what the response was.
-    scoped_ptr<Value> root(base::JSONReader::Read(json_retval, false));
-    bool response = false;
-    ListValue* list = NULL;
-    // If the dialog closes because of a button click then the json is a list
-    // containing a single bool.  If the dialog closes some other way, then we
-    // assume it means no permission was given to kill tabs.
-    if (root.get() && root->GetAsList(&list) && list &&
-        list->GetBoolean(0, &response) && response) {
-      // The user indicated that it is OK to kill the renderer process.
-      if (contents_ && contents_->GetRenderProcessHost()) {
-        base::KillProcess(contents_->GetRenderProcessHost()->GetHandle(),
-                          content::RESULT_CODE_HUNG, false);
-      }
-    } else {
-      // No indication from the user that it is ok to kill anything. Just wait.
-      // Start waiting again for responsiveness.
-      if (contents_ && contents_->render_view_host())
-        contents_->render_view_host()->RestartHangMonitorTimeout();
+  // Figure out what the response was.
+  scoped_ptr<Value> root(base::JSONReader::Read(json_retval, false));
+  bool response = false;
+  ListValue* list = NULL;
+  // If the dialog closes because of a button click then the json is a list
+  // containing a single bool.  If the dialog closes some other way, then we
+  // assume it means no permission was given to kill tabs.
+  if (root.get() && root->GetAsList(&list) && list &&
+      list->GetBoolean(0, &response) && response) {
+    // The user indicated that it is OK to kill the renderer process.
+    if (contents_ && contents_->GetRenderProcessHost()) {
+      base::KillProcess(contents_->GetRenderProcessHost()->GetHandle(),
+                        content::RESULT_CODE_HUNG, false);
     }
+  } else {
+    // No indication from the user that it is ok to kill anything. Just wait.
+    // Start waiting again for responsiveness.
+    if (contents_ && contents_->render_view_host())
+      contents_->render_view_host()->RestartHangMonitorTimeout();
   }
   g_instance = NULL;
   delete this;

chrome/browser/ui/webui/hung_renderer_dialog.h

@@ -9,11 +9,9 @@
 #include <string>
 #include <vector>
 
-#include "base/memory/scoped_ptr.h"
 #include "base/string16.h"
 #include "base/values.h"
 #include "chrome/browser/ui/webui/html_dialog_ui.h"
-#include "content/browser/tab_contents/tab_contents_observer.h"
 #include "ui/gfx/native_widget_types.h"
 
 class TabContents;
@@ -28,31 +26,7 @@ class HungRendererDialog : private HtmlDialogUIDelegate {
   static void HideHungRendererDialog(TabContents* contents);
 
  private:
-  class TabContentsObserverImpl : public TabContentsObserver {
-   public:
-    TabContentsObserverImpl(HungRendererDialog* dialog,
-                            TabContents* contents);
-
-    // TabContentsObserver overrides:
-    virtual void RenderViewGone() OVERRIDE;
-    virtual void TabContentsDestroyed(TabContents* tab) OVERRIDE;
-
-   private:
-    TabContents* contents_;  // weak
-    HungRendererDialog* dialog_;  // weak
-
-    DISALLOW_COPY_AND_ASSIGN(TabContentsObserverImpl);
-  };
-
-  friend class HungRendererDialogUITest;
-
-  explicit HungRendererDialog(bool is_enabled);
-  virtual ~HungRendererDialog();
-
-  // Shows a hung renderer dialog that, if not enabled, won't kill processes
-  // or restart hang timers.
-  static void ShowHungRendererDialogInternal(TabContents* contents,
-                                             bool is_enabled);
+  HungRendererDialog();
 
   // Shows the hung renderer dialog.
   void ShowDialog(TabContents* contents);
@@ -79,17 +53,9 @@ class HungRendererDialog : private HtmlDialogUIDelegate {
   // The dialog handler.
   HungRendererDialogHandler* handler_;
 
-  // A safety switch that must be enabled to allow actual killing of processes
-  // or restarting of the hang timer.  This is necessary so that tests can
-  // create a disabled version of this dialog that won't kill processes or
-  // restart timers when the dialog closes at the end of the test.
-  bool is_enabled_;
-
   // The dialog window.
   gfx::NativeWindow window_;
 
-  scoped_ptr<TabContentsObserverImpl> contents_observer_;
-
   DISALLOW_COPY_AND_ASSIGN(HungRendererDialog);
 };
 

chrome/chrome_tests.gypi

@@ -2680,8 +2680,6 @@
         'test/data/webui/chrome_send_browsertest.cc',
         'test/data/webui/chrome_send_browsertest.h',
         'test/data/webui/chrome_send_browsertest.js',
-        'test/data/webui/hung_renderer_dialog_test.js',
-        'test/data/webui/hung_renderer_dialog_ui_test-inl.h',
         'test/data/webui/mock4js_browsertest.js',
         'test/data/webui/ntp4.js',
         'test/data/webui/print_preview.js',

chrome/test/data/webui/hung_renderer_dialog_test.js

-// 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.
-
-/**
- * Test fixture for generated tests.
- * @extends {testing.Test}
- */
-function HungRendererDialogUITest() {};
-
-HungRendererDialogUITest.prototype = {
-  __proto__: testing.Test.prototype,
-
-  /**
-   * Define the C++ fixture class and include it.
-   * @type {?string}
-   * @override
-   */
-  typedefCppFixture: 'HungRendererDialogUITest',
-};
-
-// Include the bulk of c++ code.
-GEN('#include "chrome/test/data/webui/hung_renderer_dialog_ui_test-inl.h"');
-
-// Constructors and destructors must be provided in .cc to prevent clang errors.
-GEN('HungRendererDialogUITest::HungRendererDialogUITest() {}');
-GEN('HungRendererDialogUITest::~HungRendererDialogUITest() {}');
-
-/**
- * Confirms that the URL is correct.
- */
-TEST_F('HungRendererDialogUITest', 'testURL', function() {
-  expectEquals(chrome.expectedUrl, window.location.href);
-});
-
-/**
- * Confirms that the theme graphic is loaded and is of a reasonable size. This
- * validates that we are integrating properly with the theme source.
- */
-TEST_F('HungRendererDialogUITest', 'testThemeGraphicIntegration', function() {
-  var themeGraphic = $('theme-graphic');
-  assertNotEquals(null, themeGraphic);
-  expectGT(themeGraphic.width, 0);
-  expectGT(themeGraphic.height, 0);
-});
-
-/**
- * Confirms that the DOM was updated such that a list item representing the
- * frozen tab was added to the list of frozen tabs.  Also confirms that the list
- * item text content is the title of the frozen tab.
- */
-TEST_F('HungRendererDialogUITest', 'testTabTable', function() {
-  var tabTable = $('tab-table');
-  assertNotEquals(null, tabTable);
-  assertGT(tabTable.childElementCount, 0);
-
-  var children = tabTable.getElementsByTagName('*');
-  assertNotEquals(null, children);
-  var titleElement = children[0];
-  assertNotEquals(null, titleElement);
-  expectEquals(chrome.expectedTitle, titleElement.textContent);
-});
-

chrome/test/data/webui/hung_renderer_dialog_ui_test-inl.h

-// 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 "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/webui/chrome_web_ui.h"
-#include "chrome/browser/ui/webui/hung_renderer_dialog.h"
-#include "chrome/browser/ui/webui/web_ui_browsertest.h"
-#include "chrome/common/url_constants.h"
-#include "chrome/test/base/test_html_dialog_observer.h"
-#include "content/browser/renderer_host/render_view_host.h"
-#include "content/browser/tab_contents/tab_contents.h"
-#include "content/browser/webui/web_ui.h"
-
-// Test framework for chrome/test/data/webui/hung_renderer_dialog_test.js.
-class HungRendererDialogUITest : public WebUIBrowserTest {
- public:
-  HungRendererDialogUITest();
-  virtual ~HungRendererDialogUITest();
-
- private:
-  virtual void SetUpOnMainThread() OVERRIDE;
-};
-
-void HungRendererDialogUITest::SetUpOnMainThread() {
-  // Force the flag so that we will use the WebUI version of the Dialog.
-  ChromeWebUI::OverrideMoreWebUI(true);
-
-  // Choose which tab contents to report as hung.  In this case, the default
-  // tab contents will be about:blank.
-  ASSERT_TRUE(browser() != NULL);
-  TabContents* tab_contents = browser()->GetSelectedTabContents();
-
-  // The TestHtmlDialogObserver will catch our dialog when it gets created.
-  TestHtmlDialogObserver dialog_observer;
-
-  // Show a disabled Hung Renderer Dialog that won't kill processes or restart
-  // hang timers.
-  HungRendererDialog::ShowHungRendererDialogInternal(tab_contents, false);
-
-  // Now we can get the WebUI object from the observer, and make some details
-  // about our test available to the JavaScript.
-  WebUI* webui = dialog_observer.GetWebUI();
-  webui->tab_contents()->render_view_host()->SetWebUIProperty(
-      "expectedUrl", chrome::kChromeUIHungRendererDialogURL);
-  webui->tab_contents()->render_view_host()->SetWebUIProperty(
-      "expectedTitle", "about:blank");
-
-  // Tell the test which WebUI instance we are dealing with and complete
-  // initialization of this test.
-  SetWebUIInstance(webui);
-  WebUIBrowserTest::SetUpOnMainThread();
-}