The Mac version of the download shelf from the original CL by thakis:

http://codereview.chromium.org/150216

Original description:
Move download item to its own view and a xib, paving the way for a custom
download item view. I didn't change the look of the download items yet. The
context menu is now in the download item xib as well.

BUG=14659,15098,14660
TEST=Download something. Everything should look like before (except for the
     smaller icon), but the context menu items should be disabled/enabled and
     checked/unchecked correctly.

Review URL: http://codereview.chromium.org/149276

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

chrome/browser/cocoa/download_item_controller.h

+// Copyright (c) 2009 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.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/scoped_ptr.h"
+
+class BaseDownloadItemModel;
+class DownloadItemMac;
+class DownloadShelfContextMenuMac;
+
+// A controller class that manages one download item.
+
+@interface DownloadItemController : NSViewController {
+ @private
+  IBOutlet NSPopUpButton* popupButton_;
+
+  IBOutlet NSMenu* activeDownloadMenu_;
+  IBOutlet NSMenu* completeDownloadMenu_;
+
+  scoped_ptr<DownloadItemMac> bridge_;
+  scoped_ptr<DownloadShelfContextMenuMac> menuBridge_;
+};
+
+// Takes ownership of |downloadModel|.
+- (id)initWithFrame:(NSRect)frameRect
+              model:(BaseDownloadItemModel*)downloadModel;
+
+// Updates the UI and menu state from |downloadModel|.
+- (void)setStateFromDownload:(BaseDownloadItemModel*)downloadModel;
+
+// Context menu handlers.
+- (IBAction)handleOpen:(id)sender;
+- (IBAction)handleAlwaysOpen:(id)sender;
+- (IBAction)handleReveal:(id)sender;
+- (IBAction)handleCancel:(id)sender;
+
+@end
+

chrome/browser/cocoa/download_item_controller.mm

+// Copyright (c) 2009 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.
+
+#import "chrome/browser/cocoa/download_item_controller.h"
+
+#include "app/l10n_util.h"
+#include "base/mac_util.h"
+#include "base/sys_string_conversions.h"
+#include "chrome/browser/cocoa/download_item_mac.h"
+#include "chrome/browser/download/download_item_model.h"
+#include "chrome/browser/download/download_shelf.h"
+
+// A class for the chromium-side part of the download shelf context menu.
+
+class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
+ public:
+  DownloadShelfContextMenuMac(BaseDownloadItemModel* model)
+      : DownloadShelfContextMenu(model) { }
+
+  using DownloadShelfContextMenu::ExecuteItemCommand;
+  using DownloadShelfContextMenu::ItemIsChecked;
+  using DownloadShelfContextMenu::IsItemCommandEnabled;
+
+  using DownloadShelfContextMenu::SHOW_IN_FOLDER;
+  using DownloadShelfContextMenu::OPEN_WHEN_COMPLETE;
+  using DownloadShelfContextMenu::ALWAYS_OPEN_TYPE;
+  using DownloadShelfContextMenu::CANCEL;
+};
+
+// Implementation of DownloadItemController
+
+@implementation DownloadItemController
+
+- (id)initWithFrame:(NSRect)frameRect
+              model:(BaseDownloadItemModel*)downloadModel {
+  if ((self = [super initWithNibName:@"DownloadItem"
+                              bundle:mac_util::MainAppBundle()])) {
+    // Must be called before [self view], so that bridge_ is set in awakeFromNib
+    bridge_.reset(new DownloadItemMac(downloadModel, self));
+    menuBridge_.reset(new DownloadShelfContextMenuMac(downloadModel));
+
+    [[self view] setFrame:frameRect];
+  }
+  return self;
+}
+
+- (void)awakeFromNib {
+  [self setStateFromDownload:bridge_->download_model()];
+}
+
+- (void)setStateFromDownload:(BaseDownloadItemModel*)downloadModel {
+  // TODO(thakis): The windows version of this does all kinds of things
+  // (gratituous use of animation, special handling of dangerous downloads)
+  // that we don't currently do.
+
+  // Set correct popup menu.
+  if (downloadModel->download()->state() == DownloadItem::COMPLETE)
+    [popupButton_ setMenu:completeDownloadMenu_];
+  else
+    [popupButton_ setMenu:activeDownloadMenu_];
+
+  // Set name and icon of download.
+  FilePath downloadPath = downloadModel->download()->GetFileName();
+
+  // TODO(thakis): use filename eliding like gtk/windows versions.
+  NSString* titleString = base::SysWideToNSString(downloadPath.ToWStringHack());
+  [[popupButton_ itemAtIndex:0] setTitle:titleString];
+
+  // TODO(paulg): Use IconManager for loading icons on the file thread
+  // (crbug.com/16226).
+  NSString* extension = base::SysUTF8ToNSString(downloadPath.Extension());
+  [[popupButton_ itemAtIndex:0] setImage:
+      [[NSWorkspace sharedWorkspace] iconForFileType:extension]];
+
+  // Set status text.
+  std::wstring statusText = downloadModel->GetStatusText();
+  // Remove the status text label.
+  if (statusText.empty()) {
+    // TODO(thakis): Once there is a status label, hide it here.
+    return;
+  }
+
+  // TODO(thakis): Set status_text as status label.
+}
+
+// Sets the enabled and checked state of a particular menu item for this
+// download. We translate the NSMenuItem selection to menu selections understood
+// by the non platform specific download context menu.
+- (BOOL)validateMenuItem:(NSMenuItem *)item {
+  SEL action = [item action];
+
+  int actionId = 0;
+  if (action == @selector(handleOpen:)) {
+    actionId = DownloadShelfContextMenuMac::OPEN_WHEN_COMPLETE;
+  } else if (action == @selector(handleAlwaysOpen:)) {
+    actionId = DownloadShelfContextMenuMac::ALWAYS_OPEN_TYPE;
+  } else if (action == @selector(handleReveal:)) {
+    actionId = DownloadShelfContextMenuMac::SHOW_IN_FOLDER;
+  } else if (action == @selector(handleCancel:)) {
+    actionId = DownloadShelfContextMenuMac::CANCEL;
+  } else {
+    NOTREACHED();
+    return YES;
+  }
+
+  if (menuBridge_->ItemIsChecked(actionId))
+    [item setState:NSOnState];
+  else
+    [item setState:NSOffState];
+
+  return menuBridge_->IsItemCommandEnabled(actionId) ? YES : NO;
+}
+
+- (IBAction)handleOpen:(id)sender {
+  menuBridge_->ExecuteItemCommand(
+      DownloadShelfContextMenuMac::OPEN_WHEN_COMPLETE);
+}
+
+- (IBAction)handleAlwaysOpen:(id)sender {
+  menuBridge_->ExecuteItemCommand(
+      DownloadShelfContextMenuMac::ALWAYS_OPEN_TYPE);
+}
+
+- (IBAction)handleReveal:(id)sender {
+  menuBridge_->ExecuteItemCommand(DownloadShelfContextMenuMac::SHOW_IN_FOLDER);
+}
+
+- (IBAction)handleCancel:(id)sender {
+  menuBridge_->ExecuteItemCommand(DownloadShelfContextMenuMac::CANCEL);
+}
+
+@end

chrome/browser/cocoa/download_item_mac.h

@@ -12,19 +12,16 @@
 #include "chrome/browser/download/download_manager.h"
 
 class BaseDownloadItemModel;
-@class DownloadShelfController;
-@class DownloadShelfContextMenuBridge;
-
+@class DownloadItemController;
 
 // A class that bridges the visible mac download items to chromium's
 // download model.
 
 class DownloadItemMac : DownloadItem::Observer {
  public:
-  // DownloadItemMac takes ownership of |download_item_model|.
-  DownloadItemMac(BaseDownloadItemModel* download_item_model,
-                  NSRect frame,
-                  DownloadShelfController* parent);
+  // DownloadItemMac takes ownership of |download_model|.
+  DownloadItemMac(BaseDownloadItemModel* download_model,
+                  DownloadItemController* controller);
 
   // Destructor.
   ~DownloadItemMac();
@@ -33,15 +30,14 @@ class DownloadItemMac : DownloadItem::Observer {
   virtual void OnDownloadUpdated(DownloadItem* download);
   virtual void OnDownloadOpened(DownloadItem* download) { }
 
+  BaseDownloadItemModel* download_model() { return download_model_.get(); }
+
  private:
   // The download item model we represent.
   scoped_ptr<BaseDownloadItemModel> download_model_;
 
-  // Our parent view
-  DownloadShelfController* parent_;  // weak
-
-  // Context menu
-  scoped_nsobject<DownloadShelfContextMenuBridge> menu_;
+  // The objective-c controller object.
+  DownloadItemController* item_controller_;  // weak, owns us.
 };
 
 #endif  // CHROME_BROWSER_COCOA_DOWNLOAD_ITEM_MAC_H_

chrome/browser/cocoa/download_item_mac.mm

@@ -4,145 +4,15 @@
 
 #include "chrome/browser/cocoa/download_item_mac.h"
 
-#include "base/basictypes.h"
-#include "base/string_util.h"
-#include "base/sys_string_conversions.h"
-#import "chrome/browser/cocoa/download_shelf_controller.h"
-#import "chrome/browser/cocoa/download_shelf_mac.h"
-#import "chrome/browser/cocoa/download_shelf_view.h"
+#import "chrome/browser/cocoa/download_item_controller.h"
 #include "chrome/browser/download/download_item_model.h"
-#include "chrome/browser/download/download_manager.h"
-#include "chrome/browser/download/download_shelf.h"
-
-// A class for the chromium-side part of the download shelf context menu.
-
-class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
- public:
-  DownloadShelfContextMenuMac(BaseDownloadItemModel* model,
-      DownloadShelfContextMenuBridge* bridge)
-      : DownloadShelfContextMenu(model), bridge_(bridge) {
-  }
-
-  NSMenu* GetCocoaMenu();
-
-  using DownloadShelfContextMenu::ExecuteItemCommand;
-
- private:
-  DownloadShelfContextMenuBridge* bridge_;  // weak, owns us
-};
-
-NSMenu* DownloadShelfContextMenuMac::GetCocoaMenu() {
-  // TODO(thakis): win/gtk show slightly different menus when the download is
-  // in progress/done (mainly the first item)
-  // TODO(thakis): this probably wants to be in a xib file or at least use
-  // localized strings
-
-  NSMenuItem* item;
-  NSMenu* menu =
-      [[[NSMenu alloc] initWithTitle:@"DownloadItemPopup"] autorelease];
-  SEL action = @selector(performAction:);
-
-  item = [menu addItemWithTitle:@"Open" action:action keyEquivalent:@""];
-  //  [item addItemWithTitle:@"Open when complete" ...];  // In-progress text.
-  [item setTag:OPEN_WHEN_COMPLETE];
-  [item setTarget:bridge_];
-
-  // TODO(thakis): Set correct checkbox state, make this a checkbox item
-  item = [menu addItemWithTitle:@"Always open type"
-                         action:action
-                  keyEquivalent:@""];
-  [item setTag:ALWAYS_OPEN_TYPE];
-  [item setTarget:bridge_];
-
-  [menu addItem:[NSMenuItem separatorItem]];
-
-  item = [menu addItemWithTitle:@"Reveal in Finder"
-                         action:action
-                  keyEquivalent:@""];
-  [item setTag:SHOW_IN_FOLDER];
-  [item setTarget:bridge_];
-
-  [menu addItem:[NSMenuItem separatorItem]];
-
-  item = [menu addItemWithTitle:@"Cancel" action:action keyEquivalent:@""];
-  [item setTag:CANCEL];
-  [item setTarget:bridge_];
-
-  return menu;
-}
-
-
-// A class for the cocoa side of the download shelf context menu.
-
-@interface DownloadShelfContextMenuBridge : NSObject {
- @private
-  scoped_ptr<DownloadShelfContextMenuMac> contextMenu_;
-}
-
-- (DownloadShelfContextMenuBridge*)initWithModel:(BaseDownloadItemModel*)model;
-@end
-
-@interface DownloadShelfContextMenuBridge(Private)
-- (void)performAction:(id)sender;
-- (NSMenu*)menu;
-@end
-
-@implementation DownloadShelfContextMenuBridge
-
-- (DownloadShelfContextMenuBridge*)initWithModel:(BaseDownloadItemModel*)model {
-  if ((self = [super init]) == nil) {
-    return nil;
-  }
-  contextMenu_.reset(new DownloadShelfContextMenuMac(model, self));
-  return self;
-}
-
-@end
-
-@implementation DownloadShelfContextMenuBridge(Private)
-
-- (void)performAction:(id)sender {
-  contextMenu_->ExecuteItemCommand([sender tag]);
-}
-
-- (NSMenu*)menu {
-  return contextMenu_->GetCocoaMenu();
-}
-
-@end
-
 
 // DownloadItemMac -------------------------------------------------------------
 
 DownloadItemMac::DownloadItemMac(BaseDownloadItemModel* download_model,
-                                 NSRect frame,
-                                 DownloadShelfController* parent)
-    : download_model_(download_model), parent_(parent) {
+                                 DownloadItemController* controller)
+    : download_model_(download_model), item_controller_(controller) {
   download_model_->download()->AddObserver(this);
-
-  // TODO(thakis): The windows version of this does all kinds of things
-  // (gratituous use of animation, special handling of dangerous downloads)
-  // that we don't currently do.
-
-  scoped_nsobject<NSPopUpButton> view(
-      [[NSPopUpButton alloc] initWithFrame:frame pullsDown:YES]);
-  [parent_ addDownloadItem:view.get()];
-
-  FilePath download_path = download_model->download()->GetFileName();
-
-  // TODO(thakis): use filename eliding like gtk/windows versions
-  NSString* titleString = base::SysWideToNSString(
-      download_path.ToWStringHack());
-
-  menu_.reset([[DownloadShelfContextMenuBridge alloc]
-      initWithModel:download_model_.get()]);
-  [view.get() setMenu:[menu_.get() menu]];
-
-  [view.get() insertItemWithTitle:titleString atIndex:0];
-
-  NSString* extension = base::SysUTF8ToNSString(download_path.Extension());
-  [[view.get() itemAtIndex:0] setImage:
-      [[NSWorkspace sharedWorkspace] iconForFileType:extension]];
 }
 
 DownloadItemMac::~DownloadItemMac() {
@@ -152,12 +22,5 @@ DownloadItemMac::~DownloadItemMac() {
 void DownloadItemMac::OnDownloadUpdated(DownloadItem* download) {
   DCHECK_EQ(download, download_model_->download());
 
-  std::wstring status_text = download_model_->GetStatusText();
-  // Remove the status text label.
-  if (status_text.empty()) {
-    // TODO(thakis): Once there is a status label, hide it here
-    return;
-  }
-
-  // TODO(thakis): Set status_text as status label
+  [item_controller_ setStateFromDownload:download_model_.get()];
 }

chrome/browser/cocoa/download_shelf_controller.h

@@ -4,10 +4,13 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/scoped_nsobject.h"
 #include "base/scoped_ptr.h"
 
+class BaseDownloadItemModel;
 class Browser;
 @class BrowserWindowController;
+@class DownloadItemController;
 class DownloadShelf;
 @class DownloadShelfView;
 
@@ -17,19 +20,21 @@ class DownloadShelf;
  @private
   IBOutlet NSTextView* showAllDownloadsLink_;
 
-  // Currently these two are always the same, but they mean slightly
-  // different things.  contentAreaHasOffset_ is an implementation
-  // detail of download shelf visibility.
+  // Currently these two are always the same, but they mean slightly different
+  // things. |contentAreaHasOffset_| is an implementation detail of the download
+  // shelf visibility.
   BOOL contentAreaHasOffset_;
   BOOL barIsVisible_;
 
   scoped_ptr<DownloadShelf> bridge_;
   NSView* contentArea_;
   int shelfHeight_;
+
+  // The download items we have added to our shelf.
+  scoped_nsobject<NSMutableArray> downloadItemControllers_;
 };
 
-- (id)initWithBrowser:(Browser*)browser
-    contentArea:(NSView*)content;
+- (id)initWithBrowser:(Browser*)browser contentArea:(NSView*)content;
 
 - (DownloadShelf*)bridge;
 - (BOOL)isVisible;
@@ -37,9 +42,7 @@ class DownloadShelf;
 - (IBAction)show:(id)sender;
 - (IBAction)hide:(id)sender;
 
-// TODO(thakis): this should internally build an item and get only
-// the model as parameter.
-- (void)addDownloadItem:(NSView*)view;
+- (void)addDownloadItem:(BaseDownloadItemModel*)model;
 
 // Resizes the download shelf based on the state of the content area.
 - (void)resizeDownloadShelf;

chrome/browser/cocoa/download_shelf_controller.mm

@@ -2,17 +2,35 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import "download_shelf_controller.h"
+#import "chrome/browser/cocoa/download_shelf_controller.h"
 
 #include "app/l10n_util.h"
 #include "base/mac_util.h"
 #include "base/sys_string_conversions.h"
 #import "chrome/browser/cocoa/browser_window_controller.h"
 #include "chrome/browser/cocoa/browser_window_cocoa.h"
+#include "chrome/browser/cocoa/download_item_controller.h"
 #include "chrome/browser/cocoa/download_shelf_mac.h"
 #import "chrome/browser/cocoa/download_shelf_view.h"
 #include "grit/generated_resources.h"
 
+namespace {
+
+// TODO(thakis): These are all temporary until there's a download item view.
+
+// Border padding of a download item.
+const int kDownloadItemBorderPadding = 4;
+
+// Width of a download item.
+const int kDownloadItemWidth = 200;
+
+// Height of a download item.
+const int kDownloadItemHeight = 32;
+
+// Horizontal padding between two download items.
+const int kDownloadItemPadding = 10;
+
+}  // namespace
 
 @interface DownloadShelfController(Private)
 - (void)applyContentAreaOffset:(BOOL)apply;
@@ -33,6 +51,8 @@
     [self positionBar];
     [[[contentArea_ window] contentView] addSubview:[self view]];
 
+    downloadItemControllers_.reset([[NSMutableArray alloc] init]);
+
     // This calls show:, so it needs to be last.
     bridge_.reset(new DownloadShelfMac(browser, self));
   }
@@ -70,18 +90,17 @@
 
 // Initializes the download shelf at the bottom edge of |contentArea_|.
 - (void)positionBar {
-  // Set the bar's height to zero and position it at the bottom of the
-  // content area, within the window's content view (as opposed to the
-  // tab strip, which is a sibling). We'll enlarge it and slide the
-  // content area up when we need to show this strip.
+  // Set the bar's height to zero and position it at the bottom of the content
+  // area, within the window's content view (as opposed to the tab strip, which
+  // is a sibling). We'll enlarge it and slide the content area up when we need
+  // to show this strip.
   NSRect contentFrame = [contentArea_ frame];
-  NSRect barFrame = NSMakeRect(0, 0,
-                               contentFrame.size.width, shelfHeight_);
+  NSRect barFrame = NSMakeRect(0, 0, contentFrame.size.width, shelfHeight_);
   [[self view] setFrame:barFrame];
 }
 
-// Called when the contentArea's frame changes.  Enlarge the view to
-// stay with the bottom of the contentArea.
+// Called when the contentArea's frame changes.  Enlarge the view to stay with
+// the bottom of the contentArea.
 - (void)resizeDownloadShelf {
   NSRect barFrame = [[self view] frame];
   barFrame.origin.y = 0;
@@ -102,10 +121,9 @@
   barIsVisible_ = enable;
 }
 
-// Apply a contents box offset to make (or remove) room for the
-// download shelf.  If apply==YES, always make room (the contentView_ is
-// "full size").  If apply==NO we are trying to undo an offset.  If no
-// offset there is nothing to undo.
+// Apply a contents box offset to make (or remove) room for the download shelf.
+// If apply is YES, always make room (the contentView_ is "full size"). If apply
+// is NO, we are trying to undo an offset. If no offset there is nothing to undo.
 - (void)applyContentAreaOffset:(BOOL)apply {
   if (!contentAreaHasOffset_ && apply) {
     // There is no offset to unconditionally apply.
@@ -142,8 +160,23 @@
   [self showDownloadShelf:NO];
 }
 
-- (void)addDownloadItem:(NSView*)view {
-  [[self view] addSubview:view];
+- (void)addDownloadItem:(BaseDownloadItemModel*)model {
+  // TODO(thakis): we need to delete these at some point. There's no explicit
+  // mass delete on windows, figure out where they do it.
+
+  // TODO(thakis): RTL support?
+  // (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+  int startX = kDownloadItemBorderPadding +
+               (kDownloadItemWidth + kDownloadItemPadding) *
+               [downloadItemControllers_ count];
+
+  NSRect position = NSMakeRect(startX, kDownloadItemBorderPadding,
+                               kDownloadItemWidth, kDownloadItemHeight);
+  scoped_nsobject<DownloadItemController> controller(
+      [[DownloadItemController alloc] initWithFrame:position model:model]);
+  [downloadItemControllers_ addObject:controller.get()];
+
+  [[self view] addSubview:[controller.get() view]];
 }
 
 @end

chrome/browser/cocoa/download_shelf_mac.h

@@ -35,9 +35,6 @@ class DownloadShelfMac : public DownloadShelf {
   virtual void Close();
 
  private:
-  // The download items we have added to our shelf.
-  std::vector<DownloadItemMac*> download_items_;
-
   DownloadShelfController* shelf_controller_;  // weak, owns us
 };
 

chrome/browser/cocoa/download_shelf_mac.mm

@@ -8,24 +8,6 @@
 #include "chrome/browser/cocoa/download_item_mac.h"
 #include "chrome/browser/download/download_item_model.h"
 
-namespace {
-
-// TODO(thakis): These are all temporary until there's a download item view
-
-// Border padding of a download item
-const int kDownloadItemBorderPadding = 4;
-
-// Width of a download item
-const int kDownloadItemWidth = 200;
-
-// Height of a download item
-const int kDownloadItemHeight = 32;
-
-// Horizontal padding between two download items
-const int kDownloadItemPadding = 10;
-
-}  // namespace
-
 DownloadShelfMac::DownloadShelfMac(Browser* browser,
                                    DownloadShelfController* controller)
     : DownloadShelf(browser),
@@ -34,20 +16,7 @@ DownloadShelfMac::DownloadShelfMac(Browser* browser,
 }
 
 void DownloadShelfMac::AddDownload(BaseDownloadItemModel* download_model) {
-
-  // TODO(thakis): we need to delete these at some point. There's no explicit
-  // mass delete on windows, figure out where they do it.
-
-  // TODO(thakis): This should just forward to the controller.
-
-  // TODO(thakis): RTL support?
-  int startX = kDownloadItemBorderPadding +
-      (kDownloadItemWidth + kDownloadItemPadding) * download_items_.size();
-  download_items_.push_back(new DownloadItemMac(download_model,
-      NSMakeRect(startX, kDownloadItemBorderPadding,
-                 kDownloadItemWidth, kDownloadItemHeight),
-      shelf_controller_));
-
+  [shelf_controller_ addDownloadItem:download_model];
   Show();
 }
 

chrome/chrome.gyp

@@ -750,6 +750,8 @@
         'browser/cocoa/command_observer_bridge.mm',
         'browser/cocoa/custom_home_pages_model.h',
         'browser/cocoa/custom_home_pages_model.mm',
+        'browser/cocoa/download_item_controller.h',
+        'browser/cocoa/download_item_controller.mm',
         'browser/cocoa/download_item_mac.h',
         'browser/cocoa/download_item_mac.mm',
         'browser/cocoa/download_shelf_controller.h',
@@ -2450,6 +2452,7 @@
         'app/nibs/en.lproj/About.xib',
         'app/nibs/en.lproj/BrowserWindow.xib',
         'app/nibs/en.lproj/ClearBrowsingData.xib',
+        'app/nibs/en.lproj/DownloadItem.xib',
         'app/nibs/en.lproj/DownloadShelf.xib',
         'app/nibs/en.lproj/FindBar.xib',
         'app/nibs/en.lproj/FirstRunDialog.xib',