TabFirstRenderWatcher -> TabRenderWatcher.

Page content often isn't completely loaded or laid out when
the tab is first rendered.  This makes the class
additionally notify about subsequent renders so the delegate
can hold off until it sees one after it's e.g. received
notification via a WebUI message that the page is truly
ready.

BUG=chromium-os:26210
TEST=manual; also un-disabled the tests and checked that they still pass
TBR=ben@chromium.org

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

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

8 files changed, 52 insertions, 59 deletions

diff --git a/chrome/browser/chromeos/login/webui_login_view.cc b/chrome/browser/chromeos/login/webui_login_view.cc
index 6d1bd82..636dcb1 100644
--- a/chrome/browser/chromeos/login/webui_login_view.cc
+++ b/chrome/browser/chromeos/login/webui_login_view.cc
@@ -158,7 +158,7 @@ void WebUILoginView::Init(views::Widget* login_window) {
   WebContents* web_contents = webui_login_->dom_contents()->web_contents();
   web_contents->SetDelegate(this);
 
-  tab_watcher_.reset(new TabFirstRenderWatcher(web_contents, this));
+  tab_watcher_.reset(new TabRenderWatcher(web_contents, this));
 }
 
 std::string WebUILoginView::GetClassName() const {
@@ -289,8 +289,9 @@ void WebUILoginView::OnTabMainFrameLoaded() {
   VLOG(1) << "WebUI login main frame loaded.";
 }
 
-void WebUILoginView::OnTabMainFrameFirstRender() {
+void WebUILoginView::OnTabMainFrameRender() {
   VLOG(1) << "WebUI login main frame rendered.";
+  tab_watcher_.reset();
   StatusAreaViewChromeos::SetScreenMode(GetScreenMode());
   // In aura there's a global status area shown already.
 #if defined(USE_AURA)
diff --git a/chrome/browser/chromeos/login/webui_login_view.h b/chrome/browser/chromeos/login/webui_login_view.h
index 81cb14e..62bc37b 100644
--- a/chrome/browser/chromeos/login/webui_login_view.h
+++ b/chrome/browser/chromeos/login/webui_login_view.h
@@ -12,7 +12,7 @@
 #include "chrome/browser/chromeos/login/login_html_dialog.h"
 #include "chrome/browser/chromeos/status/status_area_button.h"
 #include "chrome/browser/chromeos/status/status_area_view_chromeos.h"
-#include "chrome/browser/tab_first_render_watcher.h"
+#include "chrome/browser/tab_render_watcher.h"
 #include "chrome/browser/ui/views/unhandled_keyboard_event_handler.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "ui/views/widget/widget.h"
@@ -39,7 +39,7 @@ namespace chromeos {
 class WebUILoginView : public views::WidgetDelegateView,
                        public StatusAreaButton::Delegate,
                        public content::WebContentsDelegate,
-                       public TabFirstRenderWatcher::Delegate {
+                       public TabRenderWatcher::Delegate {
  public:
   static const int kStatusAreaCornerPadding;
 
@@ -93,10 +93,10 @@ class WebUILoginView : public views::WidgetDelegateView,
   virtual StatusAreaButton::TextStyle GetStatusAreaTextStyle() const OVERRIDE;
   virtual void ButtonVisibilityChanged(views::View* button_view) OVERRIDE;
 
-  // TabFirstRenderWatcher::Delegate implementation.
+  // TabRenderWatcher::Delegate implementation.
   virtual void OnRenderHostCreated(RenderViewHost* host) OVERRIDE;
   virtual void OnTabMainFrameLoaded() OVERRIDE;
-  virtual void OnTabMainFrameFirstRender() OVERRIDE;
+  virtual void OnTabMainFrameRender() OVERRIDE;
 
   // Creates and adds the status area (separate window).
   virtual void InitStatusArea();
@@ -147,7 +147,7 @@ class WebUILoginView : public views::WidgetDelegateView,
   scoped_ptr<LoginHtmlDialog> proxy_settings_dialog_;
 
   // Watches webui_login_'s TabContents rendering.
-  scoped_ptr<TabFirstRenderWatcher> tab_watcher_;
+  scoped_ptr<TabRenderWatcher> tab_watcher_;
 
   // Whether the host window is frozen.
   bool host_window_frozen_;
diff --git a/chrome/browser/tab_first_render_watcher.cc b/chrome/browser/tab_render_watcher.cc
index 69de9cf..4edb5ad 100644
--- a/chrome/browser/tab_first_render_watcher.cc
+++ b/chrome/browser/tab_render_watcher.cc
@@ -1,8 +1,8 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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/tab_first_render_watcher.h"
+#include "chrome/browser/tab_render_watcher.h"
 
 #include "content/browser/renderer_host/render_widget_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
@@ -13,9 +13,8 @@
 
 using content::WebContents;
 
-TabFirstRenderWatcher::TabFirstRenderWatcher(WebContents* tab,
-                                             Delegate* delegate)
-    : state_(NONE),
+TabRenderWatcher::TabRenderWatcher(WebContents* tab, Delegate* delegate)
+    : loaded_(false),
       web_contents_(tab),
       delegate_(delegate) {
   registrar_.Add(this,
@@ -26,9 +25,9 @@ TabFirstRenderWatcher::TabFirstRenderWatcher(WebContents* tab,
       content::Source<WebContents>(web_contents_));
 }
 
-void TabFirstRenderWatcher::Observe(int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
+void TabRenderWatcher::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();
@@ -40,18 +39,16 @@ void TabFirstRenderWatcher::Observe(int type,
       break;
     }
     case content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME:
-      if (state_ == NONE) {
-        state_ = LOADED;
+      if (!loaded_) {
+        loaded_ = true;
         delegate_->OnTabMainFrameLoaded();
       }
       break;
     case content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT:
-      if (state_ == LOADED) {
-        state_ = FIRST_PAINT;
-        delegate_->OnTabMainFrameFirstRender();
-      }
+      if (loaded_)
+        delegate_->OnTabMainFrameRender();
       break;
     default:
-      NOTREACHED() << "unknown type" << type;
+      NOTREACHED() << "Unknown notification " << type;
   }
 }
diff --git a/chrome/browser/tab_first_render_watcher.h b/chrome/browser/tab_render_watcher.h
index f73c4fb..72b99ca 100644
--- a/chrome/browser/tab_first_render_watcher.h
+++ b/chrome/browser/tab_render_watcher.h
@@ -1,9 +1,9 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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_TAB_FIRST_RENDER_WATCHER_H_
-#define CHROME_BROWSER_TAB_FIRST_RENDER_WATCHER_H_
+#ifndef CHROME_BROWSER_TAB_RENDER_WATCHER_H_
+#define CHROME_BROWSER_TAB_RENDER_WATCHER_H_
 #pragma once
 
 #include "base/compiler_specific.h"
@@ -17,16 +17,16 @@ class WebContents;
 }
 
 // This class watches given TabContent's loading and rendering state change.
-class TabFirstRenderWatcher : public content::NotificationObserver {
+class TabRenderWatcher : public content::NotificationObserver {
  public:
   class Delegate {
    public:
     virtual void OnRenderHostCreated(RenderViewHost* host) = 0;
     virtual void OnTabMainFrameLoaded() = 0;
-    virtual void OnTabMainFrameFirstRender() = 0;
+    virtual void OnTabMainFrameRender() = 0;
   };
 
-  TabFirstRenderWatcher(content::WebContents* tab, Delegate* delegate);
+  TabRenderWatcher(content::WebContents* tab, Delegate* delegate);
 
  private:
   // Overridden from content::NotificationObserver
@@ -34,12 +34,8 @@ class TabFirstRenderWatcher : public content::NotificationObserver {
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  enum State {
-    NONE,
-    LOADED,        // Renderer loaded the page.
-    FIRST_PAINT,   // 1st paint event after the page is loaded.
-  };
-  State state_;
+  // Has the renderer loaded the page yet?
+  bool loaded_;
 
   // WebContents that this class watches.
   content::WebContents* web_contents_;
@@ -49,7 +45,7 @@ class TabFirstRenderWatcher : public content::NotificationObserver {
 
   content::NotificationRegistrar registrar_;
 
-  DISALLOW_COPY_AND_ASSIGN(TabFirstRenderWatcher);
+  DISALLOW_COPY_AND_ASSIGN(TabRenderWatcher);
 };
 
-#endif  // CHROME_BROWSER_TAB_FIRST_RENDER_WATCHER_H_
+#endif  // CHROME_BROWSER_TAB_RENDER_WATCHER_H_
diff --git a/chrome/browser/tab_first_render_watcher_browsertest.cc b/chrome/browser/tab_render_watcher_browsertest.cc
index 9172500..b16c7b4 100644
--- a/chrome/browser/tab_first_render_watcher_browsertest.cc
+++ b/chrome/browser/tab_render_watcher_browsertest.cc
@@ -1,9 +1,9 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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/message_loop.h"
-#include "chrome/browser/tab_first_render_watcher.h"
+#include "chrome/browser/tab_render_watcher.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/dom_view.h"
 #include "chrome/common/url_constants.h"
@@ -26,16 +26,16 @@ views::Widget* CreateWindowForContents(views::View* contents) {
 
 }  // namespace
 
-class TabFirstRenderWatcherTest : public InProcessBrowserTest,
-                                  public TabFirstRenderWatcher::Delegate {
+class TabRenderWatcherTest : public InProcessBrowserTest,
+                             public TabRenderWatcher::Delegate {
  public:
-  TabFirstRenderWatcherTest()
+  TabRenderWatcherTest()
       : host_created_(false),
         main_frame_loaded_(false),
         main_frame_rendered_(false) {
   }
 
-  // TabFirstRenderWatcher::Delegate implementation.
+  // TabRenderWatcher::Delegate implementation.
   virtual void OnRenderHostCreated(RenderViewHost* host) OVERRIDE {
     host_created_ = true;
   }
@@ -45,7 +45,7 @@ class TabFirstRenderWatcherTest : public InProcessBrowserTest,
     MessageLoop::current()->Quit();
   }
 
-  virtual void OnTabMainFrameFirstRender() OVERRIDE {
+  virtual void OnTabMainFrameRender() OVERRIDE {
     main_frame_rendered_ = true;
     MessageLoop::current()->Quit();
   }
@@ -58,16 +58,14 @@ class TabFirstRenderWatcherTest : public InProcessBrowserTest,
 
 // Migrated from HtmlDialogBrowserTest.TestStateTransition, which times out
 // about 5~10% of runs. See crbug.com/86059.
-IN_PROC_BROWSER_TEST_F(TabFirstRenderWatcherTest,
-                       DISABLED_TestStateTransition) {
+IN_PROC_BROWSER_TEST_F(TabRenderWatcherTest, DISABLED_TestStateTransition) {
   DOMView* dom_view = new DOMView;
   dom_view->Init(browser()->profile(), NULL);
   CreateWindowForContents(dom_view);
   dom_view->GetWidget()->Show();
 
-  scoped_ptr<TabFirstRenderWatcher> watcher(
-      new TabFirstRenderWatcher(dom_view->dom_contents()->web_contents(),
-                                this));
+  scoped_ptr<TabRenderWatcher> watcher(
+      new TabRenderWatcher(dom_view->dom_contents()->web_contents(), this));
 
   EXPECT_FALSE(host_created_);
   EXPECT_FALSE(main_frame_loaded_);
@@ -80,7 +78,7 @@ IN_PROC_BROWSER_TEST_F(TabFirstRenderWatcherTest,
   MessageLoopForUI::current()->Run();
   EXPECT_TRUE(main_frame_loaded_);
 
-  // OnTabMainFrameFirstRender() will Quit().
+  // OnTabMainFrameRender() will Quit().
   MessageLoopForUI::current()->Run();
   EXPECT_TRUE(main_frame_rendered_);
 
diff --git a/chrome/browser/ui/views/html_dialog_view.cc b/chrome/browser/ui/views/html_dialog_view.cc
index c67deee..ea11280 100644
--- a/chrome/browser/ui/views/html_dialog_view.cc
+++ b/chrome/browser/ui/views/html_dialog_view.cc
@@ -300,7 +300,7 @@ void HtmlDialogView::InitDialog() {
   // the comment above HtmlDialogUI in its header file for why.
   HtmlDialogUI::GetPropertyAccessor().SetProperty(
       web_contents->GetPropertyBag(), this);
-  tab_watcher_.reset(new TabFirstRenderWatcher(web_contents, this));
+  tab_watcher_.reset(new TabRenderWatcher(web_contents, this));
 
   DOMView::LoadURL(GetDialogContentURL());
 }
@@ -316,7 +316,8 @@ void HtmlDialogView::OnRenderHostCreated(RenderViewHost* host) {
 void HtmlDialogView::OnTabMainFrameLoaded() {
 }
 
-void HtmlDialogView::OnTabMainFrameFirstRender() {
+void HtmlDialogView::OnTabMainFrameRender() {
+  tab_watcher_.reset();
 #if defined(OS_CHROMEOS) && defined(TOOLKIT_USES_GTK)
   if (initialized_) {
     views::NativeWidgetGtk::UpdateFreezeUpdatesProperty(
diff --git a/chrome/browser/ui/views/html_dialog_view.h b/chrome/browser/ui/views/html_dialog_view.h
index 4a74769..bea8c94 100644
--- a/chrome/browser/ui/views/html_dialog_view.h
+++ b/chrome/browser/ui/views/html_dialog_view.h
@@ -11,7 +11,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/tab_first_render_watcher.h"
+#include "chrome/browser/tab_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"
@@ -40,7 +40,7 @@ class HtmlDialogView
       public HtmlDialogTabContentsDelegate,
       public HtmlDialogUIDelegate,
       public views::WidgetDelegate,
-      public TabFirstRenderWatcher::Delegate {
+      public TabRenderWatcher::Delegate {
  public:
   HtmlDialogView(Profile* profile,
                  Browser* browser,
@@ -103,10 +103,10 @@ class HtmlDialogView
   // Register accelerators for this dialog.
   virtual void RegisterDialogAccelerators();
 
-  // TabFirstRenderWatcher::Delegate implementation.
+  // TabRenderWatcher::Delegate implementation.
   virtual void OnRenderHostCreated(RenderViewHost* host) OVERRIDE;
   virtual void OnTabMainFrameLoaded() OVERRIDE;
-  virtual void OnTabMainFrameFirstRender() OVERRIDE;
+  virtual void OnTabMainFrameRender() OVERRIDE;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(HtmlDialogBrowserTest, WebContentRendered);
@@ -117,7 +117,7 @@ class HtmlDialogView
   bool initialized_;
 
   // Watches for TabContents rendering.
-  scoped_ptr<TabFirstRenderWatcher> tab_watcher_;
+  scoped_ptr<TabRenderWatcher> 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
diff --git a/chrome/browser/ui/views/html_dialog_view_browsertest.cc b/chrome/browser/ui/views/html_dialog_view_browsertest.cc
index 04a6bc4..8e52edc 100644
--- a/chrome/browser/ui/views/html_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/html_dialog_view_browsertest.cc
@@ -69,8 +69,8 @@ class TestHtmlDialogView: public HtmlDialogView {
     HtmlDialogView::OnDialogClosed(json_retval);
   }
 
-  virtual void OnTabMainFrameFirstRender() OVERRIDE {
-    HtmlDialogView::OnTabMainFrameFirstRender();
+  virtual void OnTabMainFrameRender() OVERRIDE {
+    HtmlDialogView::OnTabMainFrameRender();
     painted_ = true;
     MessageLoop::current()->Quit();
   }
@@ -200,7 +200,7 @@ IN_PROC_BROWSER_TEST_F(HtmlDialogBrowserTest, DISABLED_WebContentRendered) {
   html_view->InitDialog();
   html_view->GetWidget()->Show();
 
-  // TestHtmlDialogView::OnTabMainFrameFirstRender() will Quit().
+  // TestHtmlDialogView::OnTabMainFrameRender() will Quit().
   MessageLoopForUI::current()->Run();
 
   EXPECT_TRUE(html_view->painted());