Rewire the throbber so that the timer for updating lives on BrowserView, not TabStrip, so that app window/popup throbber updating doesn't need to be plumbed through the tabstrip and the browser object!

http://crbug.com/3297
Review URL: http://codereview.chromium.org/10761

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

10 files changed, 69 insertions, 90 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 2b2a381..19ecce9 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -394,6 +394,10 @@ std::wstring Browser::GetCurrentPageTitle() const {
   return l10n_util::GetStringF(IDS_BROWSER_WINDOW_TITLE_FORMAT, title);
 }
 
+bool Browser::IsCurrentPageLoading() const {
+  return GetSelectedTabContents()->is_loading();
+}
+
 // static
 void Browser::FormatTitleForDisplay(std::wstring* title) {
   size_t current_index = 0;
@@ -1333,12 +1337,6 @@ void Browser::DuplicateContentsAt(int index) {
   }
 }
 
-void Browser::ValidateLoadingAnimations() {
-  // TODO(beng): Remove this, per http://crbug.com/3297
-  if (window_)
-    window_->ValidateThrobber();
-}
-
 void Browser::CloseFrameAfterDragSession() {
   // This is scheduled to run after we return to the message loop because
   // otherwise the frame will think the drag session is still active and ignore
@@ -1684,8 +1682,7 @@ void Browser::ActivateContents(TabContents* contents) {
 }
 
 void Browser::LoadingStateChanged(TabContents* source) {
-  tabstrip_model_.UpdateTabContentsLoadingAnimations();
-
+  window_->UpdateLoadingAnimations(tabstrip_model_.TabsAreLoading());
   window_->UpdateTitleBar();
 
   // Let the go button know that it should change appearance if possible.
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 353ab4b..a1ff84e 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -108,6 +108,9 @@ class Browser : public TabStripModelDelegate,
   // Gets the title of the page in the selected tab.
   std::wstring GetCurrentPageTitle() const;
 
+  // Returns true if the current page is loading.
+  bool IsCurrentPageLoading() const;
+
   // Prepares a title string for display (removes embedded newlines, etc).
   static void FormatTitleForDisplay(std::wstring* title);
 
@@ -304,7 +307,6 @@ class Browser : public TabStripModelDelegate,
       SiteInstance* instance) const;
   virtual bool CanDuplicateContentsAt(int index);
   virtual void DuplicateContentsAt(int index);
-  virtual void ValidateLoadingAnimations();
   virtual void CloseFrameAfterDragSession();
 
   // Overridden from TabStripModelObserver:
diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index f500c42..2cd1648 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -71,11 +71,10 @@ class BrowserWindow {
   // TODO(beng): make this pure virtual after XPFrame/VistaFrame retire.
   virtual void UpdateTitleBar() = 0;
 
-  // Updates internal state specifying whether the throbber is to be shown.
-  // If the throbber was shown, and should still be shown, the frame of the
-  // throbber is advanced.
-  // If necessary, the appropriate painting is scheduled.
-  virtual void ValidateThrobber() { }
+  // Update any loading animations running in the window. |should_animate| is
+  // true if there are tabs loading and the animations should continue, false
+  // if there are no active loads and the animations should end.
+  virtual void UpdateLoadingAnimations(bool should_animate) = 0;
 
   // TODO(beng): RENAME (GetRestoredBounds)
   // Returns the nonmaximized bounds of the frame (even if the frame is
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index ac5b2dc..9c29452 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -217,11 +217,6 @@ void TabStripModel::UpdateTabContentsStateAt(int index) {
       TabChangedAt(GetContentsAt(index), index));
 }
 
-void TabStripModel::UpdateTabContentsLoadingAnimations() {
-  FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
-      TabValidateAnimations());
-}
-
 void TabStripModel::CloseAllTabs() {
   // Set state so that observers can adjust their behavior to suit this
   // specific condition when CloseTabContentsAt causes a flurry of
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index ebdec91..386599c 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -69,12 +69,6 @@ class TabStripModelObserver {
                         int to_index) { }
   // The specified TabContents at |index| changed in some way.
   virtual void TabChangedAt(TabContents* contents, int index) { }
-  // Loading progress representations for tabs should be validated/updated.
-  // TODO(beng): this wiring is cracktarded. consider revising. The loading
-  //             animation timer should live in BrowserView2, and from there
-  //             notify both the tabstrip and the window icon.
-  //             clean this up once XPFrame and VistaFrame have retired.
-  virtual void TabValidateAnimations() { }
   // The TabStripModel now no longer has any "significant" (user created or
   // user manipulated) tabs. The implementer may use this as a trigger to try
   // and close the window containing the TabStripModel, for example...
@@ -131,11 +125,6 @@ class TabStripModelDelegate {
   // window.
   virtual void DuplicateContentsAt(int index) = 0;
 
-  // Called every time the the throbber needs to be updated. We have this to
-  // give the browser/frame a chance to implement some loading animation. This
-  // is used by simple web application frames.
-  virtual void ValidateLoadingAnimations() = 0;
-
   // Called when a drag session has completed and the frame that initiated the
   // the session should be closed.
   virtual void CloseFrameAfterDragSession() = 0;
@@ -280,11 +269,6 @@ class TabStripModel : public NotificationObserver {
   // changed in some way.
   void UpdateTabContentsStateAt(int index);
 
-  // Notify any observers that Loading progress for TabContents should be
-  // validated.
-  // TODO(beng): (Cleanup) This should definitely be moved to the View.
-  void UpdateTabContentsLoadingAnimations();
-
   // Make sure there is an auto-generated New Tab tab in the TabStripModel.
   // If |force_create| is true, the New Tab will be created even if the
   // preference is set to false (used by startup).
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 3b8ea61..24c9b3c 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -1009,7 +1009,6 @@ class TabStripDummyDelegate : public TabStripModelDelegate {
   }
   virtual bool CanDuplicateContentsAt(int index) { return false; }
   virtual void DuplicateContentsAt(int index) {}
-  virtual void ValidateLoadingAnimations() {}
   virtual void CloseFrameAfterDragSession() {}
 
  private:
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 8e5cff1..873d38a 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -7,6 +7,7 @@
 #include "chrome/browser/views/frame/browser_view.h"
 
 #include "base/file_version_info.h"
+#include "base/time.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/app/theme/theme_resources.h"
 #include "chrome/browser/app_modal_dialog_queue.h"
@@ -47,6 +48,8 @@
 #include "chromium_strings.h"
 #include "generated_resources.h"
 
+using base::TimeDelta;
+
 // static
 SkBitmap BrowserView::default_favicon_;
 SkBitmap BrowserView::otr_avatar_;
@@ -73,6 +76,8 @@ static const int kWindowTilePixels = 10;
 static const int kDefaultHungPluginDetectFrequency = 2000;
 // How long do we wait before we consider a window hung (in ms).
 static const int kDefaultPluginMessageResponseTimeout = 30000;
+// The number of milliseconds between loading animation frames.
+static const int kLoadingAnimationFrameTimeMs = 30;
 
 static const struct { bool separator; int command; int label; } kMenuLayout[] = {
   { true, 0, 0 },
@@ -416,10 +421,20 @@ void BrowserView::UpdateTitleBar() {
     frame_->GetWindow()->UpdateWindowIcon();
 }
 
-void BrowserView::ValidateThrobber() {
-  if (ShouldShowWindowIcon()) {
-    TabContents* tab_contents = browser_->GetSelectedTabContents();
-    frame_->UpdateThrobber(tab_contents ? tab_contents->is_loading() : false);
+void BrowserView::UpdateLoadingAnimations(bool should_animate) {
+  if (should_animate) {
+    if (!loading_animation_timer_.IsRunning()) {
+      // Loads are happening, and the timer isn't running, so start it.
+      loading_animation_timer_.Start(
+          TimeDelta::FromMilliseconds(kLoadingAnimationFrameTimeMs), this,
+          &BrowserView::LoadingAnimationCallback);
+    }
+  } else {
+    if (loading_animation_timer_.IsRunning()) {
+      loading_animation_timer_.Stop();
+      // Loads are now complete, update the state if a task was scheduled.
+      LoadingAnimationCallback();
+    }
   }
 }
 
@@ -1274,6 +1289,16 @@ int BrowserView::GetCommandIDForAppCommandID(int app_command_id) const {
   return -1;
 }
 
+void BrowserView::LoadingAnimationCallback() {
+  if (SupportsWindowFeature(FEATURE_TABSTRIP)) {
+    // Loading animations are shown in the tab for tabbed windows.
+    tabstrip_->UpdateLoadingAnimations();
+  } else if (ShouldShowWindowIcon()) {
+    // ... or in the window icon area for popups and app windows.
+    frame_->UpdateThrobber(browser_->IsCurrentPageLoading());
+  }
+}
+
 void BrowserView::InitHangMonitor() {
   PrefService* pref_service = g_browser_process->local_state();
   int plugin_message_response_timeout =
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index fde7d12..bf6a41b 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -151,7 +151,7 @@ class BrowserView : public BrowserWindow,
   virtual StatusBubble* GetStatusBubble();
   virtual void SelectedTabToolbarSizeChanged(bool is_animating);
   virtual void UpdateTitleBar();
-  virtual void ValidateThrobber();
+  virtual void UpdateLoadingAnimations(bool should_animate);
   virtual gfx::Rect GetNormalBounds() const;
   virtual bool IsMaximized();
   virtual ToolbarStarToggle* GetStarButton() const;
@@ -308,6 +308,9 @@ class BrowserView : public BrowserWindow,
   // Retrieves the command id for the specified Windows app command.
   int GetCommandIDForAppCommandID(int app_command_id) const;
 
+  // Callback for the loading animation(s) associated with this view.
+  void LoadingAnimationCallback();
+
   // Initialize the hung plugin detector.
   void InitHangMonitor();
 
@@ -385,6 +388,9 @@ class BrowserView : public BrowserWindow,
   // plugin window.
   HungPluginAction hung_plugin_action_;
 
+  // The timer used to update frames for the Loading Animation.
+  base::RepeatingTimer<BrowserView> loading_animation_timer_;
+
   // P13N stuff
 #ifdef CHROME_PERSONALIZATION
   FramePersonalization personalization_;
diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc
index 882d955..8a3db08 100644
--- a/chrome/browser/views/tabs/tab_strip.cc
+++ b/chrome/browser/views/tabs/tab_strip.cc
@@ -32,14 +32,12 @@
 #undef min
 #undef max
 
-using base::TimeDelta;
 using views::DropTargetEvent;
 
 static const int kDefaultAnimationDurationMs = 100;
 static const int kResizeLayoutAnimationDurationMs = 166;
 static const int kReorderAnimationDurationMs = 166;
 
-static const int kLoadingAnimationFrameTimeMs = 30;
 static const int kNewTabButtonHOffset = -5;
 static const int kNewTabButtonVOffset = 5;
 static const int kResizeTabsTimeMs = 300;
@@ -590,6 +588,24 @@ gfx::Rect TabStrip::GetIdealBounds(int index) {
   return tab_data_.at(index).ideal_bounds;
 }
 
+void TabStrip::UpdateLoadingAnimations() {
+  for (int i = 0, index = 0; i < GetTabCount(); ++i, ++index) {
+    Tab* current_tab = GetTabAt(i);
+    if (current_tab->closing()) {
+      --index;
+    } else {
+      TabContents* contents = model_->GetTabContentsAt(index);
+      if (!contents || !contents->is_loading()) {
+        current_tab->ValidateLoadingAnimation(Tab::ANIMATION_NONE);
+      } else if (contents->waiting_for_response()) {
+        current_tab->ValidateLoadingAnimation(Tab::ANIMATION_WAITING);
+      } else {
+        current_tab->ValidateLoadingAnimation(Tab::ANIMATION_LOADING);
+      }
+    }
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // TabStrip, views::View overrides:
 
@@ -881,23 +897,6 @@ void TabStrip::TabChangedAt(TabContents* contents, int index) {
   tab->UpdateFromModel();
 }
 
-void TabStrip::TabValidateAnimations() {
-  if (model_->TabsAreLoading()) {
-    if (!loading_animation_timer_.IsRunning()) {
-      // Loads are happening, and the timer isn't running, so start it.
-      loading_animation_timer_.Start(
-          TimeDelta::FromMilliseconds(kLoadingAnimationFrameTimeMs), this,
-          &TabStrip::LoadingAnimationCallback);
-    }
-  } else {
-    if (loading_animation_timer_.IsRunning()) {
-      loading_animation_timer_.Stop();
-      // Loads are now complete, update the state if a task was scheduled.
-      LoadingAnimationCallback();
-    }
-  }
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // TabStrip, Tab::Delegate implementation:
 
@@ -1253,29 +1252,6 @@ void TabStrip::RemoveMessageLoopObserver() {
   }
 }
 
-void TabStrip::LoadingAnimationCallback() {
-  for (int i = 0, index = 0; i < GetTabCount(); ++i, ++index) {
-    Tab* current_tab = GetTabAt(i);
-    if (current_tab->closing()) {
-      --index;
-    } else {
-      TabContents* contents = model_->GetTabContentsAt(index);
-      if (!contents || !contents->is_loading()) {
-        current_tab->ValidateLoadingAnimation(Tab::ANIMATION_NONE);
-      } else if (contents->waiting_for_response()) {
-        current_tab->ValidateLoadingAnimation(Tab::ANIMATION_WAITING);
-      } else {
-        current_tab->ValidateLoadingAnimation(Tab::ANIMATION_LOADING);
-      }
-    }
-  }
-
-  // Make sure the model delegates updates the animation as well.
-  TabStripModelDelegate* delegate;
-  if (model_ && (delegate = model_->delegate()))
-    delegate->ValidateLoadingAnimations();
-}
-
 gfx::Rect TabStrip::GetDropBounds(int drop_index,
                                   bool drop_before,
                                   bool* is_beneath) {
diff --git a/chrome/browser/views/tabs/tab_strip.h b/chrome/browser/views/tabs/tab_strip.h
index ed752d0..360ab41 100644
--- a/chrome/browser/views/tabs/tab_strip.h
+++ b/chrome/browser/views/tabs/tab_strip.h
@@ -92,6 +92,9 @@ class TabStrip : public views::View,
   // Retrieve the ideal bounds for the Tab at the specified index.
   gfx::Rect GetIdealBounds(int index);
 
+  // Updates loading animations for the TabStrip.
+  void UpdateLoadingAnimations();
+
   // views::View overrides:
   virtual void PaintChildren(ChromeCanvas* canvas);
   virtual views::View* GetViewByID(int id) const;
@@ -122,7 +125,6 @@ class TabStrip : public views::View,
                              bool user_gesture);
   virtual void TabMoved(TabContents* contents, int from_index, int to_index);
   virtual void TabChangedAt(TabContents* contents, int index);
-  virtual void TabValidateAnimations();
 
   // Tab::Delegate implementation:
   virtual bool IsTabSelected(const Tab* tab) const;
@@ -204,9 +206,6 @@ class TabStrip : public views::View,
   void AddMessageLoopObserver();
   void RemoveMessageLoopObserver();
 
-  // Called to update the frame of the Loading animations.
-  void LoadingAnimationCallback();
-
   // -- Link Drag & Drop ------------------------------------------------------
 
   // Returns the bounds to render the drop at, in screen coordinates. Sets
@@ -290,9 +289,6 @@ class TabStrip : public views::View,
   // TODO(beng): (Cleanup) this would be better named "needs_resize_layout_".
   bool resize_layout_scheduled_;
 
-  // The timer used to update frames for the Loading Animation.
-  base::RepeatingTimer<TabStrip> loading_animation_timer_;
-
   // The "New Tab" button.
   views::Button* newtab_button_;
   gfx::Size newtab_button_size_;