Tests for WebUI Hung Renderer Dialog

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.*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109984

[wyck, 2011-11-10 15:17:26]

Please review.

[flackr, 2011-11-10 16:49:00]

http://codereview.chromium.org/8462015/diff/1/chrome/browser/ui/webui/hung_re...
File chrome/browser/ui/webui/hung_renderer_dialog.h (right):

http://codereview.chromium.org/8462015/diff/1/chrome/browser/ui/webui/hung_re...
chrome/browser/ui/webui/hung_renderer_dialog.h:23: static void
ShowHungRendererDialog(TabContents* contents, bool is_enabled);
Don't include the testing paramter in the public namespace. Add a separate
private method to show it for testing and use friend class to allow the test
framework to use it (See screen_locker_browsertests for example).

[wyck, 2011-11-11 04:15:01]

PTAL

[wyck, 2011-11-11 04:15:52]

http://codereview.chromium.org/8462015/diff/1/chrome/browser/ui/webui/hung_re...
File chrome/browser/ui/webui/hung_renderer_dialog.h (right):

http://codereview.chromium.org/8462015/diff/1/chrome/browser/ui/webui/hung_re...
chrome/browser/ui/webui/hung_renderer_dialog.h:23: static void
ShowHungRendererDialog(TabContents* contents, bool is_enabled);
On 2011/11/10 16:49:00, flackr wrote:
> Don't include the testing paramter in the public namespace. Add a separate
> private method to show it for testing and use friend class to allow the test
> framework to use it (See screen_locker_browsertests for example).

Done.

[flackr, 2011-11-11 15:03:53]

LGTM

[wyck, 2011-11-14 15:30:39]

I added the TabContentsObserver as in hung_renderer_dialog_gtk.cc, as per
rbyers' request.

Please review additions.

[flackr, 2011-11-14 15:48:24]

http://codereview.chromium.org/8462015/diff/9001/chrome/browser/ui/webui/hung...
File chrome/browser/ui/webui/hung_renderer_dialog.h (right):

http://codereview.chromium.org/8462015/diff/9001/chrome/browser/ui/webui/hung...
chrome/browser/ui/webui/hung_renderer_dialog.h:53: };
Move function bodies to the .cc file. The general rule is to only declare short
performance critical functions in the header file so that they may be inlined
(typically accessors / mutators).

[wyck, 2011-11-14 18:08:38]

http://codereview.chromium.org/8462015/diff/9001/chrome/browser/ui/webui/hung...
File chrome/browser/ui/webui/hung_renderer_dialog.h (right):

http://codereview.chromium.org/8462015/diff/9001/chrome/browser/ui/webui/hung...
chrome/browser/ui/webui/hung_renderer_dialog.h:53: };
On 2011/11/14 15:48:24, flackr wrote:
> Move function bodies to the .cc file. The general rule is to only declare
short
> performance critical functions in the header file so that they may be inlined
> (typically accessors / mutators).

Done.

[flackr, 2011-11-14 18:21:28]

LGTM with nit.

http://codereview.chromium.org/8462015/diff/11004/chrome/browser/ui/webui/hun...
File chrome/browser/ui/webui/hung_renderer_dialog.h (right):

http://codereview.chromium.org/8462015/diff/11004/chrome/browser/ui/webui/hun...
chrome/browser/ui/webui/hung_renderer_dialog.h:55: ~HungRendererDialog();
nit: Constructors and destructors should be declared before methods:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Declaration_Order

[commit-bot: I haz the power, 2011-11-14 19:58:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wyck@chromium.org/8462015/14004

[commit-bot: I haz the power, 2011-11-14 19:58:31]

Can't apply patch for file chrome/chrome_tests.gypi.
While running patch -p1 --forward --force;
patching file chrome/chrome_tests.gypi
Hunk #1 FAILED at 2667.
1 out of 1 hunk FAILED -- saving rejects to file chrome/chrome_tests.gypi.rej

[commit-bot: I haz the power, 2011-11-14 21:16:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wyck@chromium.org/8462015/15008

[commit-bot: I haz the power, 2011-11-14 22:09:05]

Try job failure for 8462015-15008 (retry) on mac_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-11-14 22:38:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wyck@chromium.org/8462015/16015

[commit-bot: I haz the power, 2011-11-15 00:02:07]

Change committed as 109984

[wyck, 2011-11-15 01:56:43]

http://codereview.chromium.org/8462015/diff/16015/chrome/browser/ui/webui/hun...
File chrome/browser/ui/webui/hung_renderer_dialog.cc (right):

http://codereview.chromium.org/8462015/diff/16015/chrome/browser/ui/webui/hun...
chrome/browser/ui/webui/hung_renderer_dialog.cc:38:
HungRendererDialog::ShowHungRendererDialog(contents, true);
Aw Snap!  This shouldn't be here.  This caused the revert.  :(