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

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)

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_;

chrome/browser/tab_first_render_watcher.cc → chrome/browser/tab_render_watcher.cc

-// 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,7 +25,7 @@ TabFirstRenderWatcher::TabFirstRenderWatcher(WebContents* tab,
       content::Source<WebContents>(web_contents_));
 }
 
-void TabFirstRenderWatcher::Observe(int type,
+void TabRenderWatcher::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
   switch (type) {
@@ -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;
   }
 }

chrome/browser/tab_first_render_watcher.h → chrome/browser/tab_render_watcher.h

-// 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_

chrome/browser/tab_first_render_watcher_browsertest.cc → chrome/browser/tab_render_watcher_browsertest.cc

-// 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_);
 

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(

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

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());

chrome/chrome_browser.gypi

@@ -2503,8 +2503,6 @@
         'browser/sync/user_selectable_sync_type.h',
         'browser/tab_closeable_state_watcher.cc',
         'browser/tab_closeable_state_watcher.h',
-        'browser/tab_first_render_watcher.cc',
-        'browser/tab_first_render_watcher.h',
         'browser/tab_contents/background_contents.cc',
         'browser/tab_contents/background_contents.h',
         'browser/tab_contents/chrome_web_contents_view_gtk_delegate.cc',
@@ -2549,6 +2547,8 @@
         'browser/tab_contents/web_drag_bookmark_handler_mac.mm',
         'browser/tab_contents/web_drag_bookmark_handler_win.cc',
         'browser/tab_contents/web_drag_bookmark_handler_win.h',
+        'browser/tab_render_watcher.cc',
+        'browser/tab_render_watcher.h',
         'browser/tabs/default_tab_handler.cc',
         'browser/tabs/default_tab_handler.h',
         'browser/tabs/pinned_tab_codec.cc',

chrome/chrome_tests.gypi

@@ -2749,7 +2749,7 @@
         'browser/ssl/ssl_browser_tests.cc',
         'browser/tab_contents/render_view_context_menu_browsertest.cc',
         'browser/tab_contents/spelling_menu_observer_browsertest.cc',
-        'browser/tab_first_render_watcher_browsertest.cc',
+        'browser/tab_render_watcher_browsertest.cc',
         'browser/task_manager/task_manager_browsertest.cc',
         'browser/task_manager/task_manager_browsertest_util.cc',
         'browser/task_manager/task_manager_browsertest_util.h',
@@ -3155,7 +3155,7 @@
           'sources!': [
             'browser/extensions/browser_action_test_util_views.cc',
             'browser/extensions/extension_input_apitest.cc',
-            'browser/tab_first_render_watcher_browsertest.cc',
+            'browser/tab_render_watcher_browsertest.cc',
             'browser/ui/panels/panel_browser_view_browsertest.cc',
             'browser/ui/views/browser_actions_container_browsertest.cc',
             'browser/ui/views/crypto_module_password_dialog_view_unittest.cc',