Add bugs, update, and remove TODOs that are no longer valid

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/577021

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

17 files changed, 20 insertions, 77 deletions

diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm
index e5a20c9..e573f2d 100644
--- a/chrome/browser/cocoa/browser_window_cocoa.mm
+++ b/chrome/browser/cocoa/browser_window_cocoa.mm
@@ -165,8 +165,6 @@ void BrowserWindowCocoa::SetStarredState(bool is_starred) {
 }
 
 gfx::Rect BrowserWindowCocoa::GetRestoredBounds() const {
-  // TODO(pinkerton): not sure if we can get the non-zoomed bounds, or if it
-  // really matters. We may want to let Cocoa handle all this for us.
   // Flip coordinates based on the primary screen.
   NSScreen* screen = [[NSScreen screens] objectAtIndex:0];
   NSRect frame = [window() frame];
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index 39d28dc..ae59e2f 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -2169,7 +2169,6 @@ willPositionSheet:(NSWindow*)sheet
 + (GTMTheme*)themeWithBrowserThemeProvider:(BrowserThemeProvider*)provider
                             isOffTheRecord:(BOOL)isOffTheRecord {
   // First check if it's in the cache.
-  // TODO(pinkerton): This might be a good candidate for a singleton.
   typedef std::pair<std::string, BOOL> ThemeKey;
   static std::map<ThemeKey, GTMTheme*> cache;
   ThemeKey key(provider->GetThemeID(), isOffTheRecord);
diff --git a/chrome/browser/cocoa/browser_window_factory.mm b/chrome/browser/cocoa/browser_window_factory.mm
index 1dcfd6c0..ef83956 100644
--- a/chrome/browser/cocoa/browser_window_factory.mm
+++ b/chrome/browser/cocoa/browser_window_factory.mm
@@ -13,10 +13,6 @@
 // window from the nib. The controller takes ownership of |browser|.
 // static
 BrowserWindow* BrowserWindow::CreateBrowserWindow(Browser* browser) {
-  // TODO(pinkerton): figure out ownership model. If BrowserList keeps track
-  // of the browser windows, it will probably tell us when it needs to go
-  // away, and it seems we need to feed back to that when we get a
-  // performClose: from the UI.
   BrowserWindowController* controller =
       [[BrowserWindowController alloc] initWithBrowser:browser];
   return [controller browserWindow];
diff --git a/chrome/browser/cocoa/custom_home_pages_model.mm b/chrome/browser/cocoa/custom_home_pages_model.mm
index 4232e4d..f6f5d71 100644
--- a/chrome/browser/cocoa/custom_home_pages_model.mm
+++ b/chrome/browser/cocoa/custom_home_pages_model.mm
@@ -114,7 +114,7 @@ NSString* const kHomepageEntryChangedNotification =
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kHomepageEntryChangedNotification object:nil];
 
-  // TODO(pinkerton): fetch favicon, convert to NSImage
+  // TODO(pinkerton): fetch favicon, convert to NSImage http://crbug.com/34642
 }
 
 - (NSString*)URL {
diff --git a/chrome/browser/cocoa/menu_controller.h b/chrome/browser/cocoa/menu_controller.h
index d0a085f..0389bff 100644
--- a/chrome/browser/cocoa/menu_controller.h
+++ b/chrome/browser/cocoa/menu_controller.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved. Use of this
+// Copyright (c) 2010 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.
 
@@ -20,8 +20,6 @@ class MenuModel;
 // allow for hierarchical menus). The tag is the index into that model for
 // that particular item. It is important that the model outlives this object
 // as it only maintains weak references.
-// TODO(pinkerton): Handle changes to the model. SimpleMenuModel doesn't yet
-// notify when changes are made.
 @interface MenuController : NSObject {
  @private
   scoped_nsobject<NSMenu> menu_;
diff --git a/chrome/browser/cocoa/menu_controller.mm b/chrome/browser/cocoa/menu_controller.mm
index f4e69a6..83a47ed 100644
--- a/chrome/browser/cocoa/menu_controller.mm
+++ b/chrome/browser/cocoa/menu_controller.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved. Use of this
+// Copyright (c) 2010 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.
 
@@ -107,7 +107,8 @@
 }
 
 // Called before the menu is to be displayed to update the state (enabled,
-// radio, etc) of each item in the menu.
+// radio, etc) of each item in the menu. Also will update the title if
+// the item is marked as "dynamic".
 - (BOOL)validateUserInterfaceItem:(id<NSValidatedUserInterfaceItem>)item {
   SEL action = [item action];
   if (action != @selector(itemSelected:))
diff --git a/chrome/browser/cocoa/preferences_window_controller.mm b/chrome/browser/cocoa/preferences_window_controller.mm
index 582cb6f..3ec7520 100644
--- a/chrome/browser/cocoa/preferences_window_controller.mm
+++ b/chrome/browser/cocoa/preferences_window_controller.mm
@@ -652,6 +652,7 @@ class PrefObserverBridge : public NotificationObserver,
   [self switchToPage:initialPage_ animate:NO];
 
   // TODO(pinkerton): save/restore position based on prefs.
+  // http://crbug.com/34644
   [[self window] center];
 }
 
@@ -687,7 +688,6 @@ class PrefObserverBridge : public NotificationObserver,
   showHomeButton_.Init(prefs::kShowHomeButton, prefs_, observer_.get());
   showPageOptionButtons_.Init(prefs::kShowPageOptionsButtons, prefs_,
                               observer_.get());
-  // TODO(pinkerton): Register Default search.
 
   // Personal Stuff panel
   askSavePasswords_.Init(prefs::kPasswordManagerEnabled,
@@ -727,10 +727,7 @@ class PrefObserverBridge : public NotificationObserver,
   // Basics
   prefs_->RemovePrefObserver(prefs::kURLsToRestoreOnStartup, observer_.get());
 
-  // User Data panel
-  // Nothing to do here.
-
-  // TODO(pinkerton): do other panels...
+  // Nothing to do for other panels...
 }
 
 // Called when a key we're observing via KVO changes.
@@ -1455,7 +1452,7 @@ const int kDisabledIndex = 1;
   }
 
   // TODO(pinkerton): windows shows a dialog here telling the user they need to
-  // restart for this to take effect. Is that really necessary?
+  // restart for this to take effect. http://crbug.com/34653
   metricsRecording_.SetValue(enabled);
 }
 
@@ -1467,6 +1464,7 @@ const int kDisabledIndex = 1;
 // Sets the backend pref for whether or not to accept cookies based on |index|.
 - (void)setCookieBehavior:(NSInteger)index {
   // TODO(darin): Remove everything else related to this setter.
+  // http://crbug.com/34656
 }
 
 - (NSURL*)defaultDownloadLocation {
diff --git a/chrome/browser/cocoa/status_bubble_mac_unittest.mm b/chrome/browser/cocoa/status_bubble_mac_unittest.mm
index 7c72a9c..f05b79f 100644
--- a/chrome/browser/cocoa/status_bubble_mac_unittest.mm
+++ b/chrome/browser/cocoa/status_bubble_mac_unittest.mm
@@ -422,11 +422,6 @@ TEST_F(StatusBubbleMacTest, StateTransitions) {
   EXPECT_EQ(StatusBubbleMac::kBubbleHidden, StateAt(0));
 }
 
-TEST_F(StatusBubbleMacTest, MouseMove) {
-  // TODO(pinkerton): Not sure what to do here since it relies on
-  // [NSEvent currentEvent] and the current mouse location.
-}
-
 TEST_F(StatusBubbleMacTest, Delete) {
   NSWindow* window = test_window();
   // Create and delete immediately.
diff --git a/chrome/browser/cocoa/tab_controller_unittest.mm b/chrome/browser/cocoa/tab_controller_unittest.mm
index bab2bef..bf552b8 100644
--- a/chrome/browser/cocoa/tab_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_controller_unittest.mm
@@ -144,11 +144,6 @@ TEST_F(TabControllerTest, Loading) {
 
 // Tests selecting the tab with the mouse click and ensuring the target/action
 // get called.
-// TODO(pinkerton): It's yucky that TabView bakes in the dragging so that we
-// can't test this class w/out lots of extra effort. When cole finishes the
-// rewrite, we should move all that logic out into a separate controller which
-// we can dependency-inject/mock so it has very simple click behavior for unit
-// testing.
 TEST_F(TabControllerTest, UserSelection) {
   NSWindow* window = test_window();
 
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index 876f56a..e5fd1ac 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -226,7 +226,7 @@ private:
 #pragma mark -
 
 // TODO(pinkerton): document tab layout, placeholders, tab dragging on
-// dev.chromium.org
+// dev.chromium.org http://crbug.com/34659
 
 // In general, there is a one-to-one correspondence between TabControllers,
 // TabViews, TabContentsControllers, and the TabContents in the TabStripModel.
@@ -579,6 +579,7 @@ private:
       // Limit the width available for laying out tabs so that tabs are not
       // resized until a later time (when the mouse leaves the tab strip).
       // TODO(pinkerton): re-visit when handling tab overflow.
+      // http://crbug.com/188
       NSView* penultimateTab = [self viewAtIndex:numberOfOpenTabs - 2];
       availableResizeWidth_ = NSMaxX([penultimateTab frame]);
     } else {
@@ -640,11 +641,8 @@ private:
 // the ordering in the TabStripModel. This call isn't that expensive, though
 // it is O(n) in the number of tabs. Tabs will animate to their new position
 // if the window is visible and |animate| is YES.
-// TODO(pinkerton): Handle drag placeholders via proxy objects, perhaps a
-// subclass of TabContentsController with everything stubbed out or by
-// abstracting a base class interface.
 // TODO(pinkerton): Note this doesn't do too well when the number of min-sized
-// tabs would cause an overflow.
+// tabs would cause an overflow. http://crbug.com/188
 - (void)layoutTabsWithAnimation:(BOOL)animate
              regenerateSubviews:(BOOL)doUpdate {
   DCHECK([NSThread isMainThread]);
@@ -887,8 +885,6 @@ private:
   // Set the originating frame to just below the strip so that it animates
   // upwards as it's being initially layed out. Oddly, this works while doing
   // something similar in |-layoutTabs| confuses the window server.
-  // TODO(pinkerton): I'm not happy with this animiation either, but it's
-  // a little better that just sliding over (maybe?).
   [newView setFrame:NSOffsetRect([newView frame],
                                  0, -[[self class] defaultTabHeight])];
 
diff --git a/chrome/browser/cocoa/tab_strip_controller_unittest.mm b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
index d68491c..0967ae4 100644
--- a/chrome/browser/cocoa/tab_strip_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
@@ -131,17 +131,17 @@ TEST_F(TabStripControllerTest, AddRemoveTabs) {
 }
 
 TEST_F(TabStripControllerTest, SelectTab) {
-  // TODO(pinkerton): Implement
+  // TODO(pinkerton): Implement http://crbug.com/10899
 }
 
 TEST_F(TabStripControllerTest, RearrangeTabs) {
-  // TODO(pinkerton): Implement
+  // TODO(pinkerton): Implement http://crbug.com/10899
 }
 
 // Test that changing the number of tabs broadcasts a
 // kTabStripNumberOfTabsChanged notifiction.
 TEST_F(TabStripControllerTest, Notifications) {
-  // TODO(pinkerton): Implement
+  // TODO(pinkerton): Implement http://crbug.com/10899
 }
 
 }  // namespace
diff --git a/chrome/browser/cocoa/tab_view.mm b/chrome/browser/cocoa/tab_view.mm
index 8167071..c5dfd2a 100644
--- a/chrome/browser/cocoa/tab_view.mm
+++ b/chrome/browser/cocoa/tab_view.mm
@@ -180,11 +180,10 @@ const CGFloat kRapidCloseDist = 2.5;
   if (chromeIsVisible_ == shouldBeVisible)
     return;
 
-  // TODO(pinkerton): There appears to be a race-condition in CoreAnimation
-  // where if we use animators to set the alpha values, we can't guarantee
-  // that we cancel them. This has the side effect of sometimes leaving
-  // the dragged window translucent or invisible. We should re-visit this,
-  // but for now, don't animate the alpha change.
+  // There appears to be a race-condition in CoreAnimation where if we use
+  // animators to set the alpha values, we can't guarantee that we cancel them.
+  // This has the side effect of sometimes leaving the dragged window
+  // translucent or invisible. As a result, don't animate the alpha change.
   [[draggedController_ overlayWindow] setAlphaValue:1.0];
   if (targetController_) {
     [dragWindow_ setAlphaValue:0.0];
@@ -200,9 +199,6 @@ const CGFloat kRapidCloseDist = 2.5;
 
 // Handle clicks and drags in this button. We get here because we have
 // overridden acceptsFirstMouse: and the click is within our bounds.
-// TODO(pinkerton/alcor): This routine needs *a lot* of work to marry Cole's
-// ideas of dragging cocoa views between windows and how the Browser and
-// TabStrip models want to manage tabs.
 - (void)mouseDown:(NSEvent*)theEvent {
   if ([self isClosing])
     return;
@@ -378,10 +374,6 @@ const CGFloat kRapidCloseDist = 2.5;
   for (TabWindowController* target in targets) {
     NSRect windowFrame = [[target window] frame];
     if (NSPointInRect(thisPoint, windowFrame)) {
-      // TODO(pinkerton): If bringing the window to the front immediately is too
-      // annoying, use another dwell date. Can't use |targetDwellDate| because
-      // this hasn't yet become the new target until the mouse is in the tab
-      // strip.
       [[target window] orderFront:self];
       NSRect tabStripFrame = [[target tabStripView] frame];
       tabStripFrame.origin = [[target window]
diff --git a/chrome/browser/cocoa/tab_view_unittest.mm b/chrome/browser/cocoa/tab_view_unittest.mm
index 128274b..99ad415 100644
--- a/chrome/browser/cocoa/tab_view_unittest.mm
+++ b/chrome/browser/cocoa/tab_view_unittest.mm
@@ -37,11 +37,6 @@ TEST_F(TabViewTest, Display) {
   }
 }
 
-// Test dragging and mouse tracking.
-TEST_F(TabViewTest, MouseTracking) {
-  // TODO(pinkerton): Test dragging out of window
-}
-
 // Test it doesn't crash when asked for its menu with no TabController set.
 TEST_F(TabViewTest, Menu) {
   EXPECT_FALSE([view_ menu]);
diff --git a/chrome/browser/cocoa/tab_window_controller.h b/chrome/browser/cocoa/tab_window_controller.h
index 8598766..011915a 100644
--- a/chrome/browser/cocoa/tab_window_controller.h
+++ b/chrome/browser/cocoa/tab_window_controller.h
@@ -46,7 +46,6 @@
 // cause the controller to be autoreleased before returning.
 - (void)showOverlay;
 - (void)removeOverlay;
-- (void)removeOverlayAfterDelay:(NSTimeInterval)delay;
 - (NSWindow*)overlayWindow;
 
 // Returns YES if it is ok to constrain the window's frame to fit the screen.
diff --git a/chrome/browser/cocoa/tab_window_controller.mm b/chrome/browser/cocoa/tab_window_controller.mm
index 2cbe4ba..8a638a1 100644
--- a/chrome/browser/cocoa/tab_window_controller.mm
+++ b/chrome/browser/cocoa/tab_window_controller.mm
@@ -49,16 +49,6 @@
   }
 }
 
-// TODO(pinkerton): Nobody calls this, can we remove it?
-- (void)removeOverlayAfterDelay:(NSTimeInterval)delay {
-  [NSObject cancelPreviousPerformRequestsWithTarget:self
-                                           selector:@selector(removeOverlay)
-                                             object:nil];
-  [self performSelector:@selector(removeOverlay)
-             withObject:nil
-             afterDelay:delay];
-}
-
 - (void)showOverlay {
   [self setUseOverlay:YES];
 }
diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm
index 5ce499b..d2cc1d2 100644
--- a/chrome/browser/cocoa/toolbar_controller_unittest.mm
+++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm
@@ -130,13 +130,6 @@ TEST_F(ToolbarControllerTest, UpdateEnabledState) {
   CompareState(updater, [bar_ toolbarViews]);
 }
 
-TEST_F(ToolbarControllerTest, StarredState) {
-  // TODO(pinkerton): I'm not sure how to test this, as the only difference
-  // in internal state is in the image used. I tried using the name of the
-  // image on the button but it doesn't seem to stick to the NSImage, even
-  // when explicitly set.
-}
-
 // Focus the location bar and make sure that it's the first responder.
 TEST_F(ToolbarControllerTest, FocusLocation) {
   NSWindow* window = test_window();
@@ -149,8 +142,6 @@ TEST_F(ToolbarControllerTest, FocusLocation) {
 }
 
 TEST_F(ToolbarControllerTest, LoadingState) {
-  // TODO(pinkerton): Same problem testing this as the starred state above.
-
   // In its initial state, the go button has a tag of IDC_GO. When loading,
   // it should be IDC_STOP.
   NSButton* go = [[bar_ toolbarViews] objectAtIndex:kGoIndex];
diff --git a/chrome/browser/cocoa/web_drop_target.mm b/chrome/browser/cocoa/web_drop_target.mm
index a24c396..fa08c69 100644
--- a/chrome/browser/cocoa/web_drop_target.mm
+++ b/chrome/browser/cocoa/web_drop_target.mm
@@ -246,7 +246,7 @@ using WebKit::WebDragOperationsMask;
     }
   }
 
-  // TODO(pinkerton): Get file contents.
+  // TODO(pinkerton): Get file contents. http://crbug.com/34661
 }
 
 @end