Synchronously update the loading state when a load starts so that the UI will

be more responsive and the loading state won't be out-of-sync with anything
else. This also removes the synchronous update to the throbber when we load a
URL because this happens before the loda actually starts, giving a flash of the
default favicon. The load start call made by RenderViewHost will cover this
case after the load actually starts.

BUG=11051
Review URL: http://codereview.chromium.org/100033

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

11 files changed, 51 insertions, 38 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 8bdbc81..ace2369 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1737,10 +1737,13 @@ void Browser::OpenURLFromTab(TabContents* source,
     if (GetStatusBubble())
       GetStatusBubble()->Hide();
 
-    // Update the location bar and load state. These are both synchronous
-    // updates inside of ScheduleUIUpdate.
-    ScheduleUIUpdate(current_tab, TabContents::INVALIDATE_URL |
-                                  TabContents::INVALIDATE_LOAD);
+    // Update the location bar. This is synchronous. We specfically don't update
+    // the load state since the load hasn't started yet and updating it will put
+    // it out of sync with the actual state like whether we're displaying a
+    // favicon, which controls the throbber. If we updated it here, the throbber
+    // will show the default favicon for a split second when navigating away
+    // from the new tab page.
+    ScheduleUIUpdate(current_tab, TabContents::INVALIDATE_URL);
   } else if (disposition == OFF_THE_RECORD) {
     OpenURLOffTheRecord(profile_, url);
     return;
diff --git a/chrome/browser/extensions/extension_view.cc b/chrome/browser/extensions/extension_view.cc
index 2332886..b394af7 100755
--- a/chrome/browser/extensions/extension_view.cc
+++ b/chrome/browser/extensions/extension_view.cc
@@ -60,8 +60,7 @@ void ExtensionView::SetBackground(const SkBitmap& background) {
   ShowIfCompletelyLoaded();
 }
 
-void ExtensionView::DidStopLoading(RenderViewHost* render_view_host,
-      int32 page_id) {
+void ExtensionView::DidStopLoading(RenderViewHost* render_view_host) {
   render_view_host->WasResized();
   did_stop_loading_ = true;
   ShowIfCompletelyLoaded();
@@ -118,8 +117,7 @@ void ExtensionView::RunJavaScriptMessage(
   render_view_host()->JavaScriptMessageBoxClosed(reply_msg, true, L"");
 }
 
-void ExtensionView::DidStartLoading(RenderViewHost* render_view_host,
-                                    int32 page_id) {
+void ExtensionView::DidStartLoading(RenderViewHost* render_view_host) {
   static const StringPiece toolstrip_css(
       ResourceBundle::GetSharedInstance().GetRawDataResource(
           IDR_EXTENSIONS_TOOLSTRIP_CSS));
diff --git a/chrome/browser/extensions/extension_view.h b/chrome/browser/extensions/extension_view.h
index 429f89f..38e093d 100755
--- a/chrome/browser/extensions/extension_view.h
+++ b/chrome/browser/extensions/extension_view.h
@@ -53,8 +53,7 @@ class ExtensionView : public HWNDHtmlView,
       const std::string& extension_id);
   virtual void RenderViewCreated(RenderViewHost* render_view_host);
   virtual void DidContentsPreferredWidthChange(const int pref_width);
-  virtual void DidStopLoading(RenderViewHost* render_view_host,
-      int32 page_id);
+  virtual void DidStopLoading(RenderViewHost* render_view_host);
   virtual WebPreferences GetWebkitPrefs();
   virtual void RunJavaScriptMessage(
       const std::wstring& message,
@@ -63,8 +62,7 @@ class ExtensionView : public HWNDHtmlView,
       const int flags,
       IPC::Message* reply_msg,
       bool* did_suppress_message);
-  virtual void DidStartLoading(RenderViewHost* render_view_host,
-                               int32 page_id);
+  virtual void DidStartLoading(RenderViewHost* render_view_host);
   virtual RenderViewHostDelegate::View* GetViewDelegate() const;
 
   // RenderViewHostDelegate::View
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index bd4f092..d778250 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -226,6 +226,16 @@ void RenderViewHost::DoNavigate(ViewMsg_Navigate* nav_message) {
     suspended_nav_message_.reset(nav_message);
   } else {
     Send(nav_message);
+
+    // Force the throbber to start. We do this because WebKit's "started
+    // loading" message will be received asynchronously from the UI of the
+    // browser. But we want to keep the throbber in sync with what's happening
+    // in the UI. For example, we want to start throbbing immediately when the
+    // user naivgates even if the renderer is delayed. There is also an issue
+    // with the throbber starting because the DOMUI (which controls whether the
+    // favicon is displayed) happens synchronously. If the start loading
+    // messages was asynchronous, then the default favicon would flash in.
+    delegate_->DidStartLoading(this);
   }
 }
 
@@ -947,12 +957,12 @@ void RenderViewHost::OnMsgDidRedirectProvisionalLoad(int32 page_id,
   delegate_->DidRedirectProvisionalLoad(page_id, source_url, target_url);
 }
 
-void RenderViewHost::OnMsgDidStartLoading(int32 page_id) {
-  delegate_->DidStartLoading(this, page_id);
+void RenderViewHost::OnMsgDidStartLoading() {
+  delegate_->DidStartLoading(this);
 }
 
-void RenderViewHost::OnMsgDidStopLoading(int32 page_id) {
-  delegate_->DidStopLoading(this, page_id);
+void RenderViewHost::OnMsgDidStopLoading() {
+  delegate_->DidStopLoading(this);
 }
 
 void RenderViewHost::OnMsgDidLoadResourceFromMemoryCache(
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index f15131c..0125b50 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -450,8 +450,8 @@ class RenderViewHost : public RenderWidgetHost {
   void OnMsgDidRedirectProvisionalLoad(int32 page_id,
                                        const GURL& source_url,
                                        const GURL& target_url);
-  void OnMsgDidStartLoading(int32 page_id);
-  void OnMsgDidStopLoading(int32 page_id);
+  void OnMsgDidStartLoading();
+  void OnMsgDidStopLoading();
   void OnMsgDidLoadResourceFromMemoryCache(const GURL& url,
                                            const std::string& frame_origin,
                                            const std::string& main_frame_origin,
diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h
index 18b1f38..fe49c4d 100644
--- a/chrome/browser/renderer_host/render_view_host_delegate.h
+++ b/chrome/browser/renderer_host/render_view_host_delegate.h
@@ -209,13 +209,13 @@ class RenderViewHostDelegate {
   // The page is trying to move the RenderView's representation in the client.
   virtual void RequestMove(const gfx::Rect& new_bounds) { }
 
-  // The RenderView began loading a new page.
-  virtual void DidStartLoading(RenderViewHost* render_view_host,
-                               int32 page_id) { }
+  // The RenderView began loading a new page. This corresponds to WebKit's
+  // notion of the throbber starting.
+  virtual void DidStartLoading(RenderViewHost* render_view_host) { }
 
-  // The RenderView stopped loading a page.
-  virtual void DidStopLoading(RenderViewHost* render_view_host,
-                              int32 page_id) { }
+  // The RenderView stopped loading a page. This corresponds to WebKit's
+  // notion of the throbber stopping.
+  virtual void DidStopLoading(RenderViewHost* render_view_host) { }
 
   // The RenderView is starting a provisional load.
   virtual void DidStartProvisionalLoadForFrame(RenderViewHost* render_view_host,
diff --git a/chrome/browser/tab_contents/render_view_host_manager.h b/chrome/browser/tab_contents/render_view_host_manager.h
index 15ee081..a91d488 100644
--- a/chrome/browser/tab_contents/render_view_host_manager.h
+++ b/chrome/browser/tab_contents/render_view_host_manager.h
@@ -42,7 +42,7 @@ class RenderViewHostManager : public NotificationObserver {
     virtual void BeforeUnloadFiredFromRenderManager(
         bool proceed, bool* proceed_to_fire_unload) = 0;
     virtual void DidStartLoadingFromRenderManager(
-        RenderViewHost* render_view_host, int32 page_id) = 0;
+        RenderViewHost* render_view_host) = 0;
     virtual void RenderViewGoneFromRenderManager(
         RenderViewHost* render_view_host) = 0;
     virtual void UpdateRenderViewSizeForRenderManager() = 0;
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index f11a3c9..a74373d 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1155,6 +1155,7 @@ void TabContents::SetIsLoading(bool is_loading,
 
   if (delegate_)
     delegate_->LoadingStateChanged(this);
+  NotifyNavigationStateChanged(INVALIDATE_LOAD);
 
   NotificationType type = is_loading ? NotificationType::LOAD_START :
       NotificationType::LOAD_STOP;
@@ -1781,11 +1782,11 @@ void TabContents::RequestMove(const gfx::Rect& new_bounds) {
     delegate()->MoveContents(this, new_bounds);
 }
 
-void TabContents::DidStartLoading(RenderViewHost* rvh, int32 page_id) {
+void TabContents::DidStartLoading(RenderViewHost* rvh) {
   SetIsLoading(true, NULL);
 }
 
-void TabContents::DidStopLoading(RenderViewHost* rvh, int32 page_id) {
+void TabContents::DidStopLoading(RenderViewHost* rvh) {
   scoped_ptr<LoadNotificationDetails> details;
 
   NavigationEntry* entry = controller_.GetActiveEntry();
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 8630e42..6a8b83f 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -751,8 +751,8 @@ class TabContents : public PageNavigator,
                                const ThumbnailScore& score);
   virtual void Close(RenderViewHost* render_view_host);
   virtual void RequestMove(const gfx::Rect& new_bounds);
-  virtual void DidStartLoading(RenderViewHost* render_view_host, int32 page_id);
-  virtual void DidStopLoading(RenderViewHost* render_view_host, int32 page_id);
+  virtual void DidStartLoading(RenderViewHost* render_view_host);
+  virtual void DidStopLoading(RenderViewHost* render_view_host);
   virtual void DidStartProvisionalLoadForFrame(RenderViewHost* render_view_host,
                                                bool is_main_frame,
                                                const GURL& url);
@@ -853,8 +853,8 @@ class TabContents : public PageNavigator,
       bool proceed,
       bool* proceed_to_fire_unload);
   virtual void DidStartLoadingFromRenderManager(
-      RenderViewHost* render_view_host, int32 page_id) {
-    DidStartLoading(render_view_host, page_id);
+      RenderViewHost* render_view_host) {
+    DidStartLoading(render_view_host);
   }
   virtual void RenderViewGoneFromRenderManager(
       RenderViewHost* render_view_host) {
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 478bed6..41f7a6b 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -667,11 +667,14 @@ IPC_BEGIN_MESSAGES(ViewHost)
   // action (e.g. when the user is hovering over a link).
   IPC_MESSAGE_ROUTED2(ViewHostMsg_UpdateTargetURL, int32, GURL)
 
-  // Sent when the renderer is loading a frame
-  IPC_MESSAGE_ROUTED1(ViewHostMsg_DidStartLoading, int32)
-
-  // Sent when the renderer is done loading a frame
-  IPC_MESSAGE_ROUTED1(ViewHostMsg_DidStopLoading, int32)
+  // Sent when the renderer starts loading the page. This corresponds to
+  // WebKit's notion of the throbber starting. Note that sometimes you may get
+  // duplicates of these during a single load.
+  IPC_MESSAGE_ROUTED0(ViewHostMsg_DidStartLoading)
+
+  // Sent when the renderer is done loading a page. This corresponds to WebKit's
+  // noption of the throbber stopping.
+  IPC_MESSAGE_ROUTED0(ViewHostMsg_DidStopLoading)
 
   // Sent when the renderer loads a resource from its memory cache.
   // The security info is non empty if the resource was originally loaded over
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 11fea1d..0f5b80e 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -1157,7 +1157,7 @@ void RenderView::DidStartLoading(WebView* webview) {
   // plugin on a page.
   first_default_plugin_ = NULL;
 
-  Send(new ViewHostMsg_DidStartLoading(routing_id_, page_id_));
+  Send(new ViewHostMsg_DidStartLoading(routing_id_));
 }
 
 void RenderView::DidStopLoading(WebView* webview) {
@@ -1180,7 +1180,7 @@ void RenderView::DidStopLoading(WebView* webview) {
   AddGURLSearchProvider(webview->GetMainFrame()->GetOSDDURL(),
                         true);  // autodetected
 
-  Send(new ViewHostMsg_DidStopLoading(routing_id_, page_id_));
+  Send(new ViewHostMsg_DidStopLoading(routing_id_));
 
   MessageLoop::current()->PostDelayedTask(FROM_HERE,
       method_factory_.NewRunnableMethod(&RenderView::CapturePageInfo, page_id_,