Share TabFirstRenderWatcher with HtmlDialogView.

Share TabFirstRenderWatcher with HtmlDialogView.

- Move TabFirstRenderWatcher out of chromeos and combine the logic with HTMLDialgoView.
- Update HtmlDialogBrowserTest.TestStateTransition and move state transition test into TabFirstRenderWatcherTest.TestStateTransition;
- HtmlDialogBrowserTest.TestStateTransition -> WebContentRendered as it only tests OnTabMainFrameFirstRender is callled now;
- Conslidate two TestHtmlDialogUIDelegate into one;

Will share it with the auro app list window to avoid initial jankiness.

BUG=98308, 86059
TEST=Hold until all app list changes are in.


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

[oshima, 2011-10-27 20:49:30]

LGTM

http://codereview.chromium.org/8417005/diff/1/chrome/browser/ui/views/html_di...
File chrome/browser/ui/views/html_dialog_view.cc (right):

http://codereview.chromium.org/8417005/diff/1/chrome/browser/ui/views/html_di...
chrome/browser/ui/views/html_dialog_view.cc:255: 
nit: remove empty line

[xiyuan, 2011-10-27 21:48:07]

I missed an important step in patch set #1 - to setup the TabFirstRenderWatcher.
And it now works for real. :)

Please take another look. Thanks.

http://codereview.chromium.org/8417005/diff/1/chrome/browser/ui/views/html_di...
File chrome/browser/ui/views/html_dialog_view.cc (right):

http://codereview.chromium.org/8417005/diff/1/chrome/browser/ui/views/html_di...
chrome/browser/ui/views/html_dialog_view.cc:255: 
On 2011/10/27 20:49:30, oshima wrote:
> nit: remove empty line

Done.

[oshima, 2011-10-28 00:19:19]

LGTM

[commit-bot: I haz the power, 2011-10-28 02:28:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/8417005/2002

[commit-bot: I haz the power, 2011-10-28 03:23:45]

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

[xiyuan, 2011-10-31 17:39:46]

CL updated.
- Moved state transition tests from HtmlDialogBrowserTest into new
TabFirstRenderWatcherTest;
- Renamed HtmlDialogBrowserTest.TestStateTransition to WebContentRendered since
it only tests for rendered notification received now;
- Conslidate two TestHtmlDialogUIDelegate into one;

Should I remove the DISABLED_ and let the test run? It seems okay on my dev box
and never hit error.

[oshima, 2011-11-01 18:28:27]

Can you run the test through try bot a few times? I could repro the issue
locally before, so if it doesn't happen to you, it might have been fixed by some
other change.

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view.h (right):

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.h:93: virtual void
OnTabMainFrameLoaded() OVERRIDE {}
no inline for virtual functions

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view_browsertest.cc (right):

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view_browsertest.cc:31: const int
kInitialWidth = 40;
add documents for these variables (simple description is enough)

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view_browsertest.cc:54: };
DISALLOW_COPY_AND_ASSIGN

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/webui/test...
File chrome/browser/ui/webui/test_html_dialog_ui_delegate.h (right):

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/webui/test...
chrome/browser/ui/webui/test_html_dialog_ui_delegate.h:15: class
TestHtmlDialogUIDelegate : public HtmlDialogUIDelegate {
put this in test namespace. (it's getting common pattern)

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/webui/test...
chrome/browser/ui/webui/test_html_dialog_ui_delegate.h:38: GURL url_;
const? (not sure)

[xiyuan, 2011-11-01 20:18:15]

Cl updated to address comments in patch set #7.

Per discussion, I have marked HtmlDialogBrowserTest.WebContentRendered and
TabFirstRenderWatcherTest.TestStateTransition disabled for now and will
investigate them afterwards.

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view.h (right):

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view.h:93: virtual void
OnTabMainFrameLoaded() OVERRIDE {}
On 2011/11/01 18:28:27, oshima wrote:
> no inline for virtual functions

Done.

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
File chrome/browser/ui/views/html_dialog_view_browsertest.cc (right):

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view_browsertest.cc:31: const int
kInitialWidth = 40;
On 2011/11/01 18:28:27, oshima wrote:
> add documents for these variables (simple description is enough)

Done.

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/views/html...
chrome/browser/ui/views/html_dialog_view_browsertest.cc:54: };
On 2011/11/01 18:28:27, oshima wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/webui/test...
File chrome/browser/ui/webui/test_html_dialog_ui_delegate.h (right):

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/webui/test...
chrome/browser/ui/webui/test_html_dialog_ui_delegate.h:15: class
TestHtmlDialogUIDelegate : public HtmlDialogUIDelegate {
On 2011/11/01 18:28:27, oshima wrote:
> put this in test namespace. (it's getting common pattern)

Done.

http://codereview.chromium.org/8417005/diff/8017/chrome/browser/ui/webui/test...
chrome/browser/ui/webui/test_html_dialog_ui_delegate.h:38: GURL url_;
On 2011/11/01 18:28:27, oshima wrote:
> const? (not sure)

Done. Think it should be const as we are not supposed to changing it on the fly.

[oshima, 2011-11-01 22:07:10]

LGTM

[commit-bot: I haz the power, 2011-11-01 23:11:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/8417005/16002

[commit-bot: I haz the power, 2011-11-01 23:11:20]

Can't process patch for file chrome/chrome_browser.gypi.
File's status is None, patchset upload is incomplete.

[commit-bot: I haz the power, 2011-11-01 23:20:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/8417005/16007

[commit-bot: I haz the power, 2011-11-02 00:29:06]

Change committed as 108206