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.


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

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

6 files changed, 182 insertions, 144 deletions

diff --git a/chrome/browser/ui/views/html_dialog_view.cc b/chrome/browser/ui/views/html_dialog_view.cc
index 5c21639..a9ce20f 100644
--- a/chrome/browser/ui/views/html_dialog_view.cc
+++ b/chrome/browser/ui/views/html_dialog_view.cc
@@ -52,7 +52,7 @@ HtmlDialogView::HtmlDialogView(Profile* profile,
                                HtmlDialogUIDelegate* delegate)
     : DOMView(),
       HtmlDialogTabContentsDelegate(profile),
-      state_(NONE),
+      initialized_(false),
       delegate_(delegate) {
 }
 
@@ -79,8 +79,8 @@ bool HtmlDialogView::AcceleratorPressed(const views::Accelerator& accelerator) {
 void HtmlDialogView::ViewHierarchyChanged(
     bool is_add, View* parent, View* child) {
   DOMView::ViewHierarchyChanged(is_add, parent, child);
-  if (is_add && GetWidget() && state_ == NONE) {
-    state_ = INITIALIZED;
+  if (is_add && GetWidget() && !initialized_) {
+    initialized_ = true;
 #if defined(OS_CHROMEOS) && defined(TOOLKIT_USES_GTK)
     CHECK(
         static_cast<views::NativeWidgetGtk*>(
@@ -250,49 +250,27 @@ void HtmlDialogView::InitDialog() {
   // the comment above HtmlDialogUI in its header file for why.
   HtmlDialogUI::GetPropertyAccessor().SetProperty(
       tab_contents->property_bag(), this);
-  notification_registrar_.Add(
-      this,
-      content::NOTIFICATION_RENDER_VIEW_HOST_CREATED_FOR_TAB,
-      content::Source<TabContents>(tab_contents));
-  notification_registrar_.Add(
-      this,
-      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
-      content::Source<TabContents>(tab_contents));
+  tab_watcher_.reset(new TabFirstRenderWatcher(tab_contents, this));
 
   DOMView::LoadURL(GetDialogContentURL());
 }
 
-void HtmlDialogView::Observe(int type,
-                             const content::NotificationSource& source,
-                             const content::NotificationDetails& details) {
-  switch (type) {
-    case content::NOTIFICATION_RENDER_VIEW_HOST_CREATED_FOR_TAB: {
-      RenderWidgetHost* rwh = content::Details<RenderWidgetHost>(details).ptr();
-      notification_registrar_.Add(
-          this,
-          content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT,
-          content::Source<RenderWidgetHost>(rwh));
-      break;
-    }
-    case content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME:
-      if (state_ == INITIALIZED)
-        state_ = LOADED;
-      break;
-    case content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT:
-      if (state_ == LOADED) {
-        state_ = PAINTED;
-#if defined(OS_CHROMEOS) && defined(TOOLKIT_USES_GTK)
-        views::NativeWidgetGtk::UpdateFreezeUpdatesProperty(
-            GTK_WINDOW(GetWidget()->GetNativeView()), false);
-#endif
-      }
-      break;
-    default:
-      NOTREACHED() << "unknown type" << type;
-  }
-}
-
 void HtmlDialogView::RegisterDialogAccelerators() {
   // Pressing the ESC key will close the dialog.
   AddAccelerator(views::Accelerator(ui::VKEY_ESCAPE, false, false, false));
 }
+
+void HtmlDialogView::OnRenderHostCreated(RenderViewHost* host) {
+}
+
+void HtmlDialogView::OnTabMainFrameLoaded() {
+}
+
+void HtmlDialogView::OnTabMainFrameFirstRender() {
+#if defined(OS_CHROMEOS) && defined(TOOLKIT_USES_GTK)
+  if (initialized_) {
+    views::NativeWidgetGtk::UpdateFreezeUpdatesProperty(
+        GTK_WINDOW(GetWidget()->GetNativeView()), false);
+  }
+#endif
+}
diff --git a/chrome/browser/ui/views/html_dialog_view.h b/chrome/browser/ui/views/html_dialog_view.h
index da8c3ad..5877c72 100644
--- a/chrome/browser/ui/views/html_dialog_view.h
+++ b/chrome/browser/ui/views/html_dialog_view.h
@@ -10,11 +10,11 @@
 #include <vector>
 
 #include "base/gtest_prod_util.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/tab_first_render_watcher.h"
 #include "chrome/browser/ui/views/dom_view.h"
 #include "chrome/browser/ui/webui/html_dialog_tab_contents_delegate.h"
 #include "chrome/browser/ui/webui/html_dialog_ui.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 #include "ui/gfx/size.h"
 #include "views/widget/widget_delegate.h"
 
@@ -38,7 +38,7 @@ class HtmlDialogView
       public HtmlDialogTabContentsDelegate,
       public HtmlDialogUIDelegate,
       public views::WidgetDelegate,
-      public content::NotificationObserver {
+      public TabFirstRenderWatcher::Delegate {
  public:
   HtmlDialogView(Profile* profile, HtmlDialogUIDelegate* delegate);
   virtual ~HtmlDialogView();
@@ -84,30 +84,25 @@ class HtmlDialogView
       OVERRIDE;
   virtual void CloseContents(TabContents* source) OVERRIDE;
 
-  // Overridden from content::NotificationObserver
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
  protected:
   // Register accelerators for this dialog.
   virtual void RegisterDialogAccelerators();
 
+  // TabFirstRenderWatcher::Delegate implementation.
+  virtual void OnRenderHostCreated(RenderViewHost* host) OVERRIDE;
+  virtual void OnTabMainFrameLoaded() OVERRIDE;
+  virtual void OnTabMainFrameFirstRender() OVERRIDE;
+
  private:
-  FRIEND_TEST_ALL_PREFIXES(HtmlDialogBrowserTest, TestStateTransition);
-
-  // A state used to ensure that we show the window only after the
-  // renderer painted the full page.
-  enum DialogState {
-    NONE,
-    INITIALIZED,  // FreezeUpdates property is set to prevent WM from showing
-                  // the window until the property is remoevd.
-    LOADED,       // Renderer loaded the page.
-    PAINTED,      // 1st paint event after the page is loaded.
-                  // FreezeUpdates property is removed to tell WM to shows
-                  // the window.
-  };
-  DialogState state_;
+  FRIEND_TEST_ALL_PREFIXES(HtmlDialogBrowserTest, WebContentRendered);
+
+  // Whether the view is initialized. That is, dialog acceleartors is registered
+  // and FreezeUpdates property is set to prevent WM from showing the window
+  // until the property is removed.
+  bool initialized_;
+
+  // Watches for TabContents rendering.
+  scoped_ptr<TabFirstRenderWatcher> tab_watcher_;
 
   // This view is a delegate to the HTML content since it needs to get notified
   // about when the dialog is closing. For all other actions (besides dialog
@@ -115,8 +110,6 @@ class HtmlDialogView
   // using this variable.
   HtmlDialogUIDelegate* delegate_;
 
-  content::NotificationRegistrar notification_registrar_;
-
   DISALLOW_COPY_AND_ASSIGN(HtmlDialogView);
 };
 
diff --git a/chrome/browser/ui/views/html_dialog_view_browsertest.cc b/chrome/browser/ui/views/html_dialog_view_browsertest.cc
index ab27321..7fc77bd 100644
--- a/chrome/browser/ui/views/html_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/html_dialog_view_browsertest.cc
@@ -8,7 +8,7 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/html_dialog_view.h"
-#include "chrome/browser/ui/webui/html_dialog_ui.h"
+#include "chrome/browser/ui/webui/test_html_dialog_ui_delegate.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -28,37 +28,33 @@ namespace {
 const int kMinimumWidthToTestFor = 20;
 const int kMinimumHeightToTestFor = 30;
 
-class TestHtmlDialogUIDelegate : public HtmlDialogUIDelegate {
- public:
-  TestHtmlDialogUIDelegate() {}
-  virtual ~TestHtmlDialogUIDelegate() {}
+// Initial size of HTMLDialog for SizeWindow test case. They must be different
+// from the above kMinimumWidthToTestFor/kMinimumHeightToTestFor.
+const int kInitialWidth = 40;
+const int kInitialHeight = 40;
 
-  // HTMLDialogUIDelegate implementation:
-  virtual bool IsDialogModal() const OVERRIDE {
-    return true;
-  }
-  virtual string16 GetDialogTitle() const OVERRIDE {
-    return ASCIIToUTF16("Test");
-  }
-  virtual GURL GetDialogContentURL() const OVERRIDE {
-    return GURL(chrome::kChromeUIChromeURLsURL);
-  }
-  virtual void GetWebUIMessageHandlers(
-      std::vector<WebUIMessageHandler*>* handlers) const OVERRIDE { }
-  virtual void GetDialogSize(gfx::Size* size) const OVERRIDE {
-    size->set_width(40);
-    size->set_height(40);
+class TestHtmlDialogView: public HtmlDialogView {
+ public:
+  TestHtmlDialogView(Profile* profile, HtmlDialogUIDelegate* delegate)
+      : HtmlDialogView(profile, delegate),
+        painted_(false) {
   }
-  virtual std::string GetDialogArgs() const OVERRIDE {
-    return std::string();
+
+  bool painted() const {
+    return painted_;
   }
-  virtual void OnDialogClosed(const std::string& json_retval) OVERRIDE { }
-  virtual void OnCloseContents(TabContents* source, bool* out_close_dialog)
-      OVERRIDE {
-    if (out_close_dialog)
-      *out_close_dialog = true;
+
+ protected:
+  virtual void OnTabMainFrameFirstRender() OVERRIDE {
+    HtmlDialogView::OnTabMainFrameFirstRender();
+    painted_ = true;
+    MessageLoop::current()->Quit();
   }
-  virtual bool ShouldShowDialogTitle() const OVERRIDE { return true; }
+
+ private:
+  bool painted_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestHtmlDialogView);
 };
 
 }  // namespace
@@ -132,7 +128,9 @@ class HtmlDialogBrowserTest : public InProcessBrowserTest {
 #endif
 
 IN_PROC_BROWSER_TEST_F(HtmlDialogBrowserTest, MAYBE_SizeWindow) {
-  HtmlDialogUIDelegate* delegate = new TestHtmlDialogUIDelegate();
+  test::TestHtmlDialogUIDelegate* delegate = new test::TestHtmlDialogUIDelegate(
+      GURL(chrome::kChromeUIChromeURLsURL));
+  delegate->set_size(kInitialWidth, kInitialHeight);
 
   HtmlDialogView* html_view =
       new HtmlDialogView(browser()->profile(), delegate);
@@ -214,29 +212,25 @@ IN_PROC_BROWSER_TEST_F(HtmlDialogBrowserTest, MAYBE_SizeWindow) {
 }
 
 // This is timing out about 5~10% of runs. See crbug.com/86059.
-IN_PROC_BROWSER_TEST_F(HtmlDialogBrowserTest, DISABLED_TestStateTransition) {
-  HtmlDialogUIDelegate* delegate = new TestHtmlDialogUIDelegate();
+IN_PROC_BROWSER_TEST_F(HtmlDialogBrowserTest, DISABLED_WebContentRendered) {
+  HtmlDialogUIDelegate* delegate = new test::TestHtmlDialogUIDelegate(
+      GURL(chrome::kChromeUIChromeURLsURL));
 
-  HtmlDialogView* html_view =
-      new HtmlDialogView(browser()->profile(), delegate);
+  TestHtmlDialogView* html_view =
+      new TestHtmlDialogView(browser()->profile(), delegate);
   TabContents* tab_contents = browser()->GetSelectedTabContents();
   ASSERT_TRUE(tab_contents != NULL);
   views::Widget::CreateWindowWithParent(html_view,
                                         tab_contents->GetDialogRootWindow());
-  // Test if the state transitions from INITIALIZED to -> PAINTED
-  EXPECT_EQ(HtmlDialogView::INITIALIZED, html_view->state_);
+  EXPECT_TRUE(html_view->initialized_);
 
   html_view->InitDialog();
   html_view->GetWidget()->Show();
 
-  MessageLoopForUI::current()->AddObserver(
-      WindowChangedObserver::GetInstance());
-  // We use busy loop because the state is updated in notifications.
-  while (html_view->state_ != HtmlDialogView::PAINTED)
-    MessageLoop::current()->RunAllPending();
+  // TestHtmlDialogView::OnTabMainFrameFirstRender() will Quit().
+  MessageLoopForUI::current()->Run();
 
-  EXPECT_EQ(HtmlDialogView::PAINTED, html_view->state_);
+  EXPECT_TRUE(html_view->painted());
 
-  MessageLoopForUI::current()->RemoveObserver(
-      WindowChangedObserver::GetInstance());
+  html_view->GetWidget()->Close();
 }
diff --git a/chrome/browser/ui/webui/constrained_html_ui_browsertest.cc b/chrome/browser/ui/webui/constrained_html_ui_browsertest.cc
index c70ad0a..e70534a 100644
--- a/chrome/browser/ui/webui/constrained_html_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/constrained_html_ui_browsertest.cc
@@ -9,7 +9,7 @@
 #include "chrome/browser/ui/constrained_window_tab_helper.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
 #include "chrome/browser/ui/webui/constrained_html_ui.h"
-#include "chrome/browser/ui/webui/html_dialog_ui.h"
+#include "chrome/browser/ui/webui/test_html_dialog_ui_delegate.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -18,39 +18,6 @@
 
 namespace {
 
-class TestHtmlDialogUIDelegate : public HtmlDialogUIDelegate {
- public:
-  TestHtmlDialogUIDelegate() {}
-  virtual ~TestHtmlDialogUIDelegate() {}
-
-  // HTMLDialogUIDelegate implementation:
-  virtual bool IsDialogModal() const OVERRIDE {
-    return true;
-  }
-  virtual string16 GetDialogTitle() const OVERRIDE {
-    return UTF8ToUTF16("Test");
-  }
-  virtual GURL GetDialogContentURL() const OVERRIDE {
-    return GURL(chrome::kChromeUIConstrainedHTMLTestURL);
-  }
-  virtual void GetWebUIMessageHandlers(
-      std::vector<WebUIMessageHandler*>* handlers) const OVERRIDE {}
-  virtual void GetDialogSize(gfx::Size* size) const OVERRIDE {
-    size->set_width(400);
-    size->set_height(400);
-  }
-  virtual std::string GetDialogArgs() const OVERRIDE {
-    return std::string();
-  }
-  virtual void OnDialogClosed(const std::string& json_retval) OVERRIDE { }
-  virtual void OnCloseContents(TabContents* source, bool* out_close_dialog)
-      OVERRIDE {
-    if (out_close_dialog)
-      *out_close_dialog = true;
-  }
-  virtual bool ShouldShowDialogTitle() const OVERRIDE { return true; }
-};
-
 class ConstrainedHtmlDialogBrowserTestObserver : public TabContentsObserver {
  public:
   explicit ConstrainedHtmlDialogBrowserTestObserver(TabContents* contents)
@@ -84,7 +51,8 @@ class ConstrainedHtmlDialogBrowserTest : public InProcessBrowserTest {
 // Tests that opening/closing the constrained window won't crash it.
 IN_PROC_BROWSER_TEST_F(ConstrainedHtmlDialogBrowserTest, BasicTest) {
   // The delegate deletes itself.
-  HtmlDialogUIDelegate* delegate = new TestHtmlDialogUIDelegate();
+  HtmlDialogUIDelegate* delegate = new test::TestHtmlDialogUIDelegate(
+      GURL(chrome::kChromeUIConstrainedHTMLTestURL));
   TabContentsWrapper* wrapper = browser()->GetSelectedTabContentsWrapper();
   ASSERT_TRUE(wrapper);
 
@@ -101,7 +69,8 @@ IN_PROC_BROWSER_TEST_F(ConstrainedHtmlDialogBrowserTest, BasicTest) {
 IN_PROC_BROWSER_TEST_F(ConstrainedHtmlDialogBrowserTest,
                        ReleaseTabContentsOnDialogClose) {
   // The delegate deletes itself.
-  TestHtmlDialogUIDelegate* delegate = new TestHtmlDialogUIDelegate();
+  HtmlDialogUIDelegate* delegate = new test::TestHtmlDialogUIDelegate(
+      GURL(chrome::kChromeUIConstrainedHTMLTestURL));
   TabContentsWrapper* wrapper = browser()->GetSelectedTabContentsWrapper();
   ASSERT_TRUE(wrapper);
 
diff --git a/chrome/browser/ui/webui/test_html_dialog_ui_delegate.cc b/chrome/browser/ui/webui/test_html_dialog_ui_delegate.cc
new file mode 100644
index 0000000..11eb078
--- /dev/null
+++ b/chrome/browser/ui/webui/test_html_dialog_ui_delegate.cc
@@ -0,0 +1,56 @@
+// 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/webui/test_html_dialog_ui_delegate.h"
+
+#include "base/utf_string_conversions.h"
+
+namespace test {
+
+TestHtmlDialogUIDelegate::TestHtmlDialogUIDelegate(const GURL& url)
+    : url_(url),
+      size_(400, 400) {
+}
+
+TestHtmlDialogUIDelegate::~TestHtmlDialogUIDelegate() {
+}
+
+bool TestHtmlDialogUIDelegate::IsDialogModal() const {
+  return true;
+}
+
+string16 TestHtmlDialogUIDelegate::GetDialogTitle() const {
+  return UTF8ToUTF16("Test");
+}
+
+GURL TestHtmlDialogUIDelegate::GetDialogContentURL() const {
+  return url_;
+}
+
+void TestHtmlDialogUIDelegate::GetWebUIMessageHandlers(
+    std::vector<WebUIMessageHandler*>* handlers) const {
+}
+
+void TestHtmlDialogUIDelegate::GetDialogSize(gfx::Size* size) const {
+  *size = size_;
+}
+
+std::string TestHtmlDialogUIDelegate::GetDialogArgs() const {
+  return std::string();
+}
+
+void TestHtmlDialogUIDelegate::OnDialogClosed(const std::string& json_retval) {
+}
+
+void TestHtmlDialogUIDelegate::OnCloseContents(TabContents* source,
+    bool* out_close_dialog) {
+  if (out_close_dialog)
+    *out_close_dialog = true;
+}
+
+bool TestHtmlDialogUIDelegate::ShouldShowDialogTitle() const {
+  return true;
+}
+
+}  // namespace test
diff --git a/chrome/browser/ui/webui/test_html_dialog_ui_delegate.h b/chrome/browser/ui/webui/test_html_dialog_ui_delegate.h
new file mode 100644
index 0000000..d9fdf0f
--- /dev/null
+++ b/chrome/browser/ui/webui/test_html_dialog_ui_delegate.h
@@ -0,0 +1,48 @@
+// 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.
+
+#ifndef CHROME_BROWSER_UI_WEBUI_TEST_HTML_DIALOG_UI_DELEGATE_H_
+#define CHROME_BROWSER_UI_WEBUI_TEST_HTML_DIALOG_UI_DELEGATE_H_
+#pragma once
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "chrome/browser/ui/webui/html_dialog_ui.h"
+#include "ui/gfx/size.h"
+
+namespace test {
+
+class TestHtmlDialogUIDelegate : public HtmlDialogUIDelegate {
+ public:
+  explicit TestHtmlDialogUIDelegate(const GURL& url);
+  virtual ~TestHtmlDialogUIDelegate();
+
+  void set_size(int width, int height) {
+    size_.SetSize(width, height);
+  }
+
+  // HTMLDialogUIDelegate implementation:
+  virtual bool IsDialogModal() const OVERRIDE;
+  virtual string16 GetDialogTitle() const OVERRIDE;
+  virtual GURL GetDialogContentURL() const OVERRIDE;
+  virtual void GetWebUIMessageHandlers(
+      std::vector<WebUIMessageHandler*>* handlers) const OVERRIDE;
+  virtual void GetDialogSize(gfx::Size* size) const OVERRIDE;
+  virtual std::string GetDialogArgs() const OVERRIDE;
+  virtual void OnDialogClosed(const std::string& json_retval) OVERRIDE;
+  virtual void OnCloseContents(TabContents* source, bool* out_close_dialog)
+      OVERRIDE;
+  virtual bool ShouldShowDialogTitle() const OVERRIDE;
+
+ protected:
+  const GURL url_;
+  gfx::Size size_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestHtmlDialogUIDelegate);
+};
+
+}  // namespace test
+
+#endif  // CHROME_BROWSER_UI_WEBUI_TEST_HTML_DIALOG_UI_DELEGATE_H_