Given the results of Rohit's investigation, I think that the times that we're

getting out of the pageload tests right now are highly suspect because we're
not actually getting the bits from each page to the screen.  As such, I'm
much less inclined to treat the "regression" in these tests as a problem.

BUG=15232
TEST=throbber changes between waiting (gray) and loading (blue) at appropriate
     times; animation still disabled
Review URL: http://codereview.chromium.org/147211

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

5 files changed, 49 insertions, 34 deletions

diff --git a/chrome/browser/cocoa/tab_controller.h b/chrome/browser/cocoa/tab_controller.h
index 7795ec3..c2da0d9 100644
--- a/chrome/browser/cocoa/tab_controller.h
+++ b/chrome/browser/cocoa/tab_controller.h
@@ -7,6 +7,17 @@
 
 #import <Cocoa/Cocoa.h>
 
+// The loading/waiting state of the tab.
+// TODO(pinkerton): this really doesn't belong here, but something needs to
+// know the state and another parallel array in TabStripController doesn't seem
+// like the right place either. In a perfect world, this class shouldn't know
+// anything about states that are specific to a browser.
+enum TabLoadingState {
+  kTabDone,
+  kTabLoading,
+  kTabWaiting,
+};
+
 @class TabView;
 @protocol TabControllerTarget;
 
@@ -26,19 +37,12 @@
   IBOutlet NSButton *backgroundButton_;
   IBOutlet NSView* iconView_;
   BOOL selected_;
-  BOOL loading_;
-  BOOL waiting_;
+  TabLoadingState loadingState_;
   id<TabControllerTarget> target_;  // weak, where actions are sent
   SEL action_;  // selector sent when tab is selected by clicking
 }
 
-// The loading/waiting state of the tab.
-// TODO(pinkerton): these really don't belong here, but something needs to
-// know the state and another parallel array in TabStripController doesn't seem
-// like the right place either. In a perfect world, this class shouldn't know
-// anything about states that are specific to a browser.
-@property(assign, nonatomic) BOOL loading;
-@property(assign, nonatomic) BOOL waiting;
+@property(assign, nonatomic) TabLoadingState loadingState;
 
 @property(assign, nonatomic) BOOL selected;
 @property(assign, nonatomic) id target;
diff --git a/chrome/browser/cocoa/tab_controller.mm b/chrome/browser/cocoa/tab_controller.mm
index 94d1ca2..f675b9c 100644
--- a/chrome/browser/cocoa/tab_controller.mm
+++ b/chrome/browser/cocoa/tab_controller.mm
@@ -8,8 +8,7 @@
 
 @implementation TabController
 
-@synthesize loading = loading_;
-@synthesize waiting = waiting_;
+@synthesize loadingState = loadingState_;
 @synthesize target = target_;
 @synthesize action = action_;
 
diff --git a/chrome/browser/cocoa/tab_controller_unittest.mm b/chrome/browser/cocoa/tab_controller_unittest.mm
index c6526be..0761679 100644
--- a/chrome/browser/cocoa/tab_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_controller_unittest.mm
@@ -126,9 +126,13 @@ TEST_F(TabControllerTest, Loading) {
   scoped_nsobject<TabController> controller([[TabController alloc] init]);
   [[window contentView] addSubview:[controller view]];
 
-  EXPECT_FALSE([controller loading]);
-  [controller setLoading:YES];
-  EXPECT_TRUE([controller loading]);
+  EXPECT_EQ(kTabDone, [controller loadingState]);
+  [controller setLoadingState:kTabWaiting];
+  EXPECT_EQ(kTabWaiting, [controller loadingState]);
+  [controller setLoadingState:kTabLoading];
+  EXPECT_EQ(kTabLoading, [controller loadingState]);
+  [controller setLoadingState:kTabDone];
+  EXPECT_EQ(kTabDone, [controller loadingState]);
 
   [[controller view] removeFromSuperview];
 }
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index 1acff657..eae1bbb 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -466,30 +466,38 @@ NSString* const kTabStripNumberOfTabsChanged = @"kTabStripNumberOfTabsChanged";
   // load, so we need to make sure we're not creating the throbber view over and
   // over.
   if (contents) {
-    static NSImage* throbberImage = [[NSImage imageNamed:@"throbber"] retain];
     static NSImage* throbberWaitingImage =
         [[NSImage imageNamed:@"throbber_waiting"] retain];
+    static NSImage* throbberLoadingImage =
+        [[NSImage imageNamed:@"throbber"] retain];
 
     TabController* tabController = [tabArray_ objectAtIndex:index];
-    NSImage* image = nil;
-    if (contents->waiting_for_response() && ![tabController waiting]) {
-      image = throbberWaitingImage;
-      [tabController setWaiting:YES];
-    } else if (contents->is_loading() && ![tabController loading]) {
-      image = throbberImage;
-      [tabController setLoading:YES];
-    }
-    if (image) {
-      NSRect frame = NSMakeRect(0, 0, 16, 16);
-      ThrobberView* throbber =
-          [[[ThrobberView alloc] initWithFrame:frame image:image] autorelease];
-      [tabController setIconView:throbber];
+
+    TabLoadingState oldState = [tabController loadingState];
+
+    TabLoadingState newState = kTabDone;
+    NSImage* throbberImage = nil;
+    if (contents->waiting_for_response()) {
+      newState = kTabWaiting;
+      throbberImage = throbberWaitingImage;
+    } else if (contents->is_loading()) {
+      newState = kTabLoading;
+      throbberImage = throbberLoadingImage;
     }
-    else if (!contents->is_loading()) {
-      // Set everything back to normal, we're done loading.
-      [tabController setIconView:[self favIconImageViewForContents:contents]];
-      [tabController setWaiting:NO];
-      [tabController setLoading:NO];
+
+    if (oldState != newState) {
+      NSView* iconView = nil;
+      if (newState == kTabDone) {
+        iconView = [self favIconImageViewForContents:contents];
+      } else {
+        NSRect frame = NSMakeRect(0, 0, 16, 16);
+        iconView =
+            [[[ThrobberView alloc] initWithFrame:frame
+                                           image:throbberImage] autorelease];
+      }
+
+      [tabController setLoadingState:newState];
+      [tabController setIconView:iconView];
     }
   }
 
diff --git a/chrome/browser/cocoa/throbber_view.mm b/chrome/browser/cocoa/throbber_view.mm
index 2f9b491..81df5c9 100644
--- a/chrome/browser/cocoa/throbber_view.mm
+++ b/chrome/browser/cocoa/throbber_view.mm
@@ -6,7 +6,7 @@
 
 #include "base/logging.h"
 
-const float kAnimationIntervalSeconds = 0.03;  // 30ms, same as windows
+static const float kAnimationIntervalSeconds = 0.03;  // 30ms, same as windows
 
 @interface ThrobberView(PrivateMethods)
 - (void)animate;