diff options
author | asanka@chromium.org <asanka@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-08 18:27:29 +0000 |
---|---|---|
committer | asanka@chromium.org <asanka@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-01-08 18:27:29 +0000 |
commit | 41b0021c13870f41b76884abba7a2656ea661e48 (patch) | |
tree | 01b78efbfdb9b19580ebe4ecbbf776fa1d7ace0f /chrome | |
parent | c39df1a7114290fa7c6f2683f69d751a904f49cb (diff) | |
download | chromium_src-41b0021c13870f41b76884abba7a2656ea661e48.zip chromium_src-41b0021c13870f41b76884abba7a2656ea661e48.tar.gz chromium_src-41b0021c13870f41b76884abba7a2656ea661e48.tar.bz2 |
Don't pass DownloadItemModel ownership.
DownloadItemModel is a thin wrapper around DownloadItem*. It's
easier to instantiate it from a DownloadItem* than try to pass
ownership around.
Doing so was required when BaseDownloadItemModel existed, and the
actual model could be either a DownloadItemModel or a
SavePageModel. However this is no longer the case.
In addition, DownloadShelfContextMenu should track the lifetime
of the corresponding DownloadItem. It is possible for the
DownloadItem to be destroyed while the context menu is showing.
BUG=116551
Review URL: https://chromiumcodereview.appspot.com/11673004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@175558 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
29 files changed, 222 insertions, 204 deletions
diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc index 66f24ea..d6242b1 100644 --- a/chrome/browser/download/download_item_model.cc +++ b/chrome/browser/download/download_item_model.cc @@ -218,10 +218,6 @@ DownloadItemModel::DownloadItemModel(DownloadItem* download) DownloadItemModel::~DownloadItemModel() { } -void DownloadItemModel::CancelTask() { - download_->Cancel(true /* update history service */); -} - string16 DownloadItemModel::GetInterruptReasonText() const { if (download_->GetState() != DownloadItem::INTERRUPTED || download_->GetLastReason() == diff --git a/chrome/browser/download/download_item_model.h b/chrome/browser/download/download_item_model.h index 8806226..6ac4f59 100644 --- a/chrome/browser/download/download_item_model.h +++ b/chrome/browser/download/download_item_model.h @@ -35,9 +35,6 @@ class DownloadItemModel { explicit DownloadItemModel(content::DownloadItem* download); ~DownloadItemModel(); - // Cancel the task corresponding to the item. - void CancelTask(); - // Returns a long descriptive string for a download that's in the INTERRUPTED // state. For other downloads, the returned string will be empty. string16 GetInterruptReasonText() const; diff --git a/chrome/browser/download/download_shelf.cc b/chrome/browser/download/download_shelf.cc index fa4f453..524ff7d 100644 --- a/chrome/browser/download/download_shelf.cc +++ b/chrome/browser/download/download_shelf.cc @@ -8,11 +8,11 @@ DownloadShelf::DownloadShelf() : should_show_on_unhide_(false), is_hidden_(false) {} -void DownloadShelf::AddDownload(DownloadItemModel* download_model) { +void DownloadShelf::AddDownload(content::DownloadItem* download) { if (is_hidden_) Unhide(); Show(); - DoAddDownload(download_model); + DoAddDownload(download); } void DownloadShelf::Show() { diff --git a/chrome/browser/download/download_shelf.h b/chrome/browser/download/download_shelf.h index f77bf02..b04b5c2 100644 --- a/chrome/browser/download/download_shelf.h +++ b/chrome/browser/download/download_shelf.h @@ -5,7 +5,10 @@ #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SHELF_H_ #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SHELF_H_ -class DownloadItemModel; +namespace content { +class DownloadItem; +} + class Browser; // This is an abstract base class for platform specific download shelf @@ -15,9 +18,9 @@ class DownloadShelf { DownloadShelf(); virtual ~DownloadShelf() {} - // A new download has started, so add it to our shelf. This object will - // take ownership of |download_model|. Also make the shelf visible. - void AddDownload(DownloadItemModel* download_model); + // A new download has started, so add it to our shelf. Also make the shelf + // visible. + void AddDownload(content::DownloadItem* download); // The browser view needs to know when we are going away to properly return // the resize corner size to WebKit so that we don't draw on top of it. @@ -47,7 +50,7 @@ class DownloadShelf { bool is_hidden() { return is_hidden_; } protected: - virtual void DoAddDownload(DownloadItemModel* download_model) = 0; + virtual void DoAddDownload(content::DownloadItem* download) = 0; virtual void DoShow() = 0; virtual void DoClose() = 0; diff --git a/chrome/browser/download/download_shelf_context_menu.cc b/chrome/browser/download/download_shelf_context_menu.cc index 64236f7..d940db6 100644 --- a/chrome/browser/download/download_shelf_context_menu.cc +++ b/chrome/browser/download/download_shelf_context_menu.cc @@ -21,23 +21,31 @@ using content::DownloadItem; using extensions::Extension; -DownloadShelfContextMenu::~DownloadShelfContextMenu() {} +DownloadShelfContextMenu::~DownloadShelfContextMenu() { + DetachFromDownloadItem(); +} DownloadShelfContextMenu::DownloadShelfContextMenu( - DownloadItemModel* download_model, + DownloadItem* download_item, content::PageNavigator* navigator) - : download_model_(download_model), - download_item_(download_model->download()), + : download_item_(download_item), navigator_(navigator) { + DCHECK(download_item_); + download_item_->AddObserver(this); } ui::SimpleMenuModel* DownloadShelfContextMenu::GetMenuModel() { ui::SimpleMenuModel* model = NULL; + + if (!download_item_) + return NULL; + + DownloadItemModel download_model(download_item_); // We shouldn't be opening a context menu for a dangerous download, unless it // is a malicious download. - DCHECK(!download_model_->IsDangerous() || download_model_->IsMalicious()); + DCHECK(!download_model.IsDangerous() || download_model.IsMalicious()); - if (download_model_->IsMalicious()) + if (download_model.IsMalicious()) model = GetMaliciousMenuModel(); else if (download_item_->IsComplete()) model = GetFinishedMenuModel(); @@ -49,6 +57,9 @@ ui::SimpleMenuModel* DownloadShelfContextMenu::GetMenuModel() { } bool DownloadShelfContextMenu::IsCommandIdEnabled(int command_id) const { + if (!download_item_) + return false; + switch (static_cast<ContextMenuCommands>(command_id)) { case SHOW_IN_FOLDER: return download_item_->CanShowInFolder(); @@ -75,6 +86,9 @@ bool DownloadShelfContextMenu::IsCommandIdEnabled(int command_id) const { } bool DownloadShelfContextMenu::IsCommandIdChecked(int command_id) const { + if (!download_item_) + return false; + switch (command_id) { case OPEN_WHEN_COMPLETE: return download_item_->GetOpenWhenComplete() || @@ -88,6 +102,9 @@ bool DownloadShelfContextMenu::IsCommandIdChecked(int command_id) const { } void DownloadShelfContextMenu::ExecuteCommand(int command_id) { + if (!download_item_) + return; + switch (static_cast<ContextMenuCommands>(command_id)) { case SHOW_IN_FOLDER: download_item_->ShowDownloadInShell(); @@ -106,7 +123,7 @@ void DownloadShelfContextMenu::ExecuteCommand(int command_id) { break; } case CANCEL: - download_model_->CancelTask(); + download_item_->Cancel(true /* Cancelled by user */); break; case TOGGLE_PAUSE: // It is possible for the download to complete before the user clicks the @@ -169,18 +186,17 @@ string16 DownloadShelfContextMenu::GetLabelForCommandId(int command_id) const { case SHOW_IN_FOLDER: return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_SHOW); case OPEN_WHEN_COMPLETE: - if (download_item_->IsInProgress()) + if (download_item_ && download_item_->IsInProgress()) return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_OPEN_WHEN_COMPLETE); return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_OPEN); case ALWAYS_OPEN_TYPE: return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_ALWAYS_OPEN_TYPE); case CANCEL: return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_CANCEL); - case TOGGLE_PAUSE: { - if (download_item_->IsPaused()) + case TOGGLE_PAUSE: + if (download_item_ && download_item_->IsPaused()) return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_RESUME_ITEM); return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_PAUSE_ITEM); - } case DISCARD: return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_DISCARD); case KEEP: @@ -195,6 +211,19 @@ string16 DownloadShelfContextMenu::GetLabelForCommandId(int command_id) const { return string16(); } +void DownloadShelfContextMenu::DetachFromDownloadItem() { + if (!download_item_) + return; + + download_item_->RemoveObserver(this); + download_item_ = NULL; +} + +void DownloadShelfContextMenu::OnDownloadDestroyed(DownloadItem* download) { + DCHECK(download_item_ == download); + DetachFromDownloadItem(); +} + ui::SimpleMenuModel* DownloadShelfContextMenu::GetInProgressMenuModel() { if (in_progress_download_menu_model_.get()) return in_progress_download_menu_model_.get(); diff --git a/chrome/browser/download/download_shelf_context_menu.h b/chrome/browser/download/download_shelf_context_menu.h index d4f83b9..a4a8cb7 100644 --- a/chrome/browser/download/download_shelf_context_menu.h +++ b/chrome/browser/download/download_shelf_context_menu.h @@ -9,18 +9,20 @@ #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "base/string16.h" +#include "content/public/browser/download_item.h" #include "ui/base/models/simple_menu_model.h" -class DownloadItemModel; - namespace content { -class DownloadItem; class PageNavigator; } // This class is responsible for the download shelf context menu. Platform // specific subclasses are responsible for creating and running the menu. -class DownloadShelfContextMenu : public ui::SimpleMenuModel::Delegate { +// +// The DownloadItem corresponding to the context menu is observed for removal or +// destruction. +class DownloadShelfContextMenu : public ui::SimpleMenuModel::Delegate, + public content::DownloadItem::Observer { public: enum ContextMenuCommands { SHOW_IN_FOLDER = 1, // Open a folder view window with the item selected. @@ -37,14 +39,13 @@ class DownloadShelfContextMenu : public ui::SimpleMenuModel::Delegate { virtual ~DownloadShelfContextMenu(); content::DownloadItem* download_item() const { return download_item_; } - void set_download_item(content::DownloadItem* item) { download_item_ = item; } protected: - DownloadShelfContextMenu(DownloadItemModel* download_model, + DownloadShelfContextMenu(content::DownloadItem* download_item, content::PageNavigator* navigator); - // Returns the correct menu model depending whether the download item is - // completed or not. + // Returns the correct menu model depending on the state of the download item. + // Returns NULL if the download was destroyed. ui::SimpleMenuModel* GetMenuModel(); // ui::SimpleMenuModel::Delegate: @@ -58,6 +59,13 @@ class DownloadShelfContextMenu : public ui::SimpleMenuModel::Delegate { virtual string16 GetLabelForCommandId(int command_id) const OVERRIDE; private: + // Detaches self from |download_item_|. Called when the DownloadItem is + // destroyed or when this object is being destroyed. + void DetachFromDownloadItem(); + + // content::DownloadItem::Observer + virtual void OnDownloadDestroyed(content::DownloadItem* download) OVERRIDE; + ui::SimpleMenuModel* GetInProgressMenuModel(); ui::SimpleMenuModel* GetFinishedMenuModel(); ui::SimpleMenuModel* GetInterruptedMenuModel(); @@ -70,9 +78,6 @@ class DownloadShelfContextMenu : public ui::SimpleMenuModel::Delegate { scoped_ptr<ui::SimpleMenuModel> interrupted_download_menu_model_; scoped_ptr<ui::SimpleMenuModel> malicious_download_menu_model_; - // A model to control the cancel behavior. - DownloadItemModel* download_model_; - // Information source. content::DownloadItem* download_item_; diff --git a/chrome/browser/download/test_download_shelf.cc b/chrome/browser/download/test_download_shelf.cc index 60c050b..9957c64 100644 --- a/chrome/browser/download/test_download_shelf.cc +++ b/chrome/browser/download/test_download_shelf.cc @@ -23,7 +23,7 @@ Browser* TestDownloadShelf::browser() const { return NULL; } -void TestDownloadShelf::DoAddDownload(DownloadItemModel* download_model) { +void TestDownloadShelf::DoAddDownload(content::DownloadItem* download) { } void TestDownloadShelf::DoShow() { diff --git a/chrome/browser/download/test_download_shelf.h b/chrome/browser/download/test_download_shelf.h index 35e5bdf..da7bb57 100644 --- a/chrome/browser/download/test_download_shelf.h +++ b/chrome/browser/download/test_download_shelf.h @@ -22,7 +22,7 @@ class TestDownloadShelf : public DownloadShelf { virtual Browser* browser() const OVERRIDE; protected: - virtual void DoAddDownload(DownloadItemModel* download_model) OVERRIDE; + virtual void DoAddDownload(content::DownloadItem* download) OVERRIDE; virtual void DoShow() OVERRIDE; virtual void DoClose() OVERRIDE; diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 455d3dc..f112448 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -1439,8 +1439,7 @@ int Browser::GetExtraRenderViewHeight() const { void Browser::OnStartDownload(WebContents* source, content::DownloadItem* download) { - scoped_ptr<DownloadItemModel> download_model(new DownloadItemModel(download)); - if (!download_model->ShouldShowInShelf()) + if (!DownloadItemModel(download).ShouldShowInShelf()) return; WebContents* constrained = GetConstrainingWebContents(source); @@ -1455,7 +1454,7 @@ void Browser::OnStartDownload(WebContents* source, // GetDownloadShelf creates the download shelf if it was not yet created. DownloadShelf* shelf = window()->GetDownloadShelf(); - shelf->AddDownload(download_model.release()); + shelf->AddDownload(download); // Don't show the animation for "Save file" downloads. // For non-theme extensions, we don't show the download animation. // Show animation in same window as the download shelf. Download shelf diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.h b/chrome/browser/ui/cocoa/download/download_item_controller.h index 1c5a6c3..b7b25f5 100644 --- a/chrome/browser/ui/cocoa/download/download_item_controller.h +++ b/chrome/browser/ui/cocoa/download/download_item_controller.h @@ -71,10 +71,10 @@ class Font; } state_; }; -// Takes ownership of |downloadModel|. -- (id)initWithModel:(DownloadItemModel*)downloadModel - shelf:(DownloadShelfController*)shelf - navigator:(content::PageNavigator*)navigator; +// Initialize controller for |downloadItem|. +- (id)initWithDownload:(content::DownloadItem*)downloadItem + shelf:(DownloadShelfController*)shelf + navigator:(content::PageNavigator*)navigator; // Updates the UI and menu state from |downloadModel|. - (void)setStateFromDownload:(DownloadItemModel*)downloadModel; diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.mm b/chrome/browser/ui/cocoa/download/download_item_controller.mm index e89a86e..574756e 100644 --- a/chrome/browser/ui/cocoa/download/download_item_controller.mm +++ b/chrome/browser/ui/cocoa/download/download_item_controller.mm @@ -74,9 +74,9 @@ void WidenView(NSView* view, CGFloat widthChange) { class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { public: - DownloadShelfContextMenuMac(DownloadItemModel* model, + DownloadShelfContextMenuMac(DownloadItem* downloadItem, content::PageNavigator* navigator) - : DownloadShelfContextMenu(model, navigator) { } + : DownloadShelfContextMenu(downloadItem, navigator) { } using DownloadShelfContextMenu::ExecuteCommand; using DownloadShelfContextMenu::IsCommandIdChecked; @@ -99,14 +99,14 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { @implementation DownloadItemController -- (id)initWithModel:(DownloadItemModel*)downloadModel - shelf:(DownloadShelfController*)shelf - navigator:(content::PageNavigator*)navigator { +- (id)initWithDownload:(DownloadItem*)downloadItem + shelf:(DownloadShelfController*)shelf + navigator:(content::PageNavigator*)navigator { if ((self = [super initWithNibName:@"DownloadItem" bundle:base::mac::FrameworkBundle()])) { // Must be called before [self view], so that bridge_ is set in awakeFromNib - bridge_.reset(new DownloadItemMac(downloadModel, self)); - menuBridge_.reset(new DownloadShelfContextMenuMac(downloadModel, + bridge_.reset(new DownloadItemMac(downloadItem, self)); + menuBridge_.reset(new DownloadShelfContextMenuMac(downloadItem, navigator)); NSNotificationCenter* defaultCenter = [NSNotificationCenter defaultCenter]; @@ -164,7 +164,7 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { } - (void)setStateFromDownload:(DownloadItemModel*)downloadModel { - DCHECK_EQ(bridge_->download_model(), downloadModel); + DCHECK_EQ([self download], downloadModel->download()); // Handle dangerous downloads. if (downloadModel->IsDangerous()) { diff --git a/chrome/browser/ui/cocoa/download/download_item_mac.h b/chrome/browser/ui/cocoa/download/download_item_mac.h index b82c16d..dbbf90e 100644 --- a/chrome/browser/ui/cocoa/download/download_item_mac.h +++ b/chrome/browser/ui/cocoa/download/download_item_mac.h @@ -9,13 +9,13 @@ #include "base/memory/scoped_nsobject.h" #include "base/memory/scoped_ptr.h" +#include "chrome/browser/download/download_item_model.h" #include "chrome/browser/icon_manager.h" #include "chrome/common/cancelable_task_tracker.h" #include "content/public/browser/download_item.h" #include "content/public/browser/download_manager.h" @class DownloadItemController; -class DownloadItemModel; namespace gfx{ class Image; @@ -27,8 +27,7 @@ class Image; class DownloadItemMac : content::DownloadItem::Observer { public: - // DownloadItemMac takes ownership of |download_model|. - DownloadItemMac(DownloadItemModel* download_model, + DownloadItemMac(content::DownloadItem* download, DownloadItemController* controller); // Destructor. @@ -39,7 +38,7 @@ class DownloadItemMac : content::DownloadItem::Observer { virtual void OnDownloadOpened(content::DownloadItem* download) OVERRIDE; virtual void OnDownloadDestroyed(content::DownloadItem* download) OVERRIDE; - DownloadItemModel* download_model() { return download_model_.get(); } + DownloadItemModel* download_model() { return &download_model_; } // Asynchronous icon loading support. void LoadIcon(); @@ -49,7 +48,7 @@ class DownloadItemMac : content::DownloadItem::Observer { void OnExtractIconComplete(gfx::Image* icon_bitmap); // The download item model we represent. - scoped_ptr<DownloadItemModel> download_model_; + DownloadItemModel download_model_; // The objective-c controller object. DownloadItemController* item_controller_; // weak, owns us. diff --git a/chrome/browser/ui/cocoa/download/download_item_mac.mm b/chrome/browser/ui/cocoa/download/download_item_mac.mm index f0a899e..ea3dd9b 100644 --- a/chrome/browser/ui/cocoa/download/download_item_mac.mm +++ b/chrome/browser/ui/cocoa/download/download_item_mac.mm @@ -16,20 +16,21 @@ using content::DownloadItem; // DownloadItemMac ------------------------------------------------------------- -DownloadItemMac::DownloadItemMac(DownloadItemModel* download_model, +DownloadItemMac::DownloadItemMac(DownloadItem* download, DownloadItemController* controller) - : download_model_(download_model), item_controller_(controller) { - download_model_->download()->AddObserver(this); + : download_model_(download), + item_controller_(controller) { + download_model_.download()->AddObserver(this); } DownloadItemMac::~DownloadItemMac() { - download_model_->download()->RemoveObserver(this); + download_model_.download()->RemoveObserver(this); } void DownloadItemMac::OnDownloadUpdated(content::DownloadItem* download) { - DCHECK_EQ(download, download_model_->download()); + DCHECK_EQ(download, download_model_.download()); - if ([item_controller_ isDangerousMode] && !download_model_->IsDangerous()) { + if ([item_controller_ isDangerousMode] && !download_model_.IsDangerous()) { // We have been approved. [item_controller_ clearDangerousMode]; } @@ -53,11 +54,11 @@ void DownloadItemMac::OnDownloadUpdated(content::DownloadItem* download) { // fall through case DownloadItem::IN_PROGRESS: case DownloadItem::CANCELLED: - [item_controller_ setStateFromDownload:download_model_.get()]; + [item_controller_ setStateFromDownload:&download_model_]; break; case DownloadItem::INTERRUPTED: [item_controller_ updateToolTip]; - [item_controller_ setStateFromDownload:download_model_.get()]; + [item_controller_ setStateFromDownload:&download_model_]; break; default: NOTREACHED(); @@ -69,7 +70,7 @@ void DownloadItemMac::OnDownloadDestroyed(content::DownloadItem* download) { } void DownloadItemMac::OnDownloadOpened(content::DownloadItem* download) { - DCHECK_EQ(download, download_model_->download()); + DCHECK_EQ(download, download_model_.download()); [item_controller_ downloadWasOpened]; } @@ -81,7 +82,7 @@ void DownloadItemMac::LoadIcon() { } // We may already have this particular image cached. - FilePath file = download_model_->download()->GetUserVerifiedFilePath(); + FilePath file = download_model_.download()->GetUserVerifiedFilePath(); gfx::Image* icon = icon_manager->LookupIcon(file, IconLoader::ALL); if (icon) { [item_controller_ setIcon:icon->ToNSImage()]; diff --git a/chrome/browser/ui/cocoa/download/download_shelf_controller.h b/chrome/browser/ui/cocoa/download/download_shelf_controller.h index 158227c..91f5a65 100644 --- a/chrome/browser/ui/cocoa/download/download_shelf_controller.h +++ b/chrome/browser/ui/cocoa/download/download_shelf_controller.h @@ -12,13 +12,13 @@ class Browser; @class BrowserWindowController; @class DownloadItemController; -class DownloadItemModel; class DownloadShelf; @class DownloadShelfView; @class HyperlinkButtonCell; @class HoverButton; namespace content { +class DownloadItem; class PageNavigator; } @@ -90,7 +90,7 @@ class PageNavigator; // Run when the user clicks the close button on the right side of the shelf. - (IBAction)hide:(id)sender; -- (void)addDownloadItem:(DownloadItemModel*)model; +- (void)addDownloadItem:(content::DownloadItem*)downloadItem; // Remove a download, possibly via clearing browser data. - (void)remove:(DownloadItemController*)download; diff --git a/chrome/browser/ui/cocoa/download/download_shelf_controller.mm b/chrome/browser/ui/cocoa/download/download_shelf_controller.mm index 838622b..d6bb3b0 100644 --- a/chrome/browser/ui/cocoa/download/download_shelf_controller.mm +++ b/chrome/browser/ui/cocoa/download/download_shelf_controller.mm @@ -292,15 +292,15 @@ const NSSize kHoverCloseButtonDefaultSize = { 18, 18 }; [self layoutItems:NO]; } -- (void)addDownloadItem:(DownloadItemModel*)model { +- (void)addDownloadItem:(DownloadItem*)downloadItem { DCHECK([NSThread isMainThread]); [self cancelAutoCloseAndRemoveTrackingArea]; // Insert new item at the left. scoped_nsobject<DownloadItemController> controller( - [[DownloadItemController alloc] initWithModel:model - shelf:self - navigator:navigator_]); + [[DownloadItemController alloc] initWithDownload:downloadItem + shelf:self + navigator:navigator_]); // Adding at index 0 in NSMutableArrays is O(1). [downloadItemControllers_ insertObject:controller.get() atIndex:0]; diff --git a/chrome/browser/ui/cocoa/download/download_shelf_mac.h b/chrome/browser/ui/cocoa/download/download_shelf_mac.h index 2423088..422de654 100644 --- a/chrome/browser/ui/cocoa/download/download_shelf_mac.h +++ b/chrome/browser/ui/cocoa/download/download_shelf_mac.h @@ -27,7 +27,7 @@ class DownloadShelfMac : public DownloadShelf { virtual Browser* browser() const OVERRIDE; protected: - virtual void DoAddDownload(DownloadItemModel* download_model) OVERRIDE; + virtual void DoAddDownload(content::DownloadItem* download) OVERRIDE; virtual void DoShow() OVERRIDE; virtual void DoClose() OVERRIDE; diff --git a/chrome/browser/ui/cocoa/download/download_shelf_mac.mm b/chrome/browser/ui/cocoa/download/download_shelf_mac.mm index fe4ed36..c644b51 100644 --- a/chrome/browser/ui/cocoa/download/download_shelf_mac.mm +++ b/chrome/browser/ui/cocoa/download/download_shelf_mac.mm @@ -15,8 +15,8 @@ DownloadShelfMac::DownloadShelfMac(Browser* browser, shelf_controller_(controller) { } -void DownloadShelfMac::DoAddDownload(DownloadItemModel* download_model) { - [shelf_controller_ addDownloadItem:download_model]; +void DownloadShelfMac::DoAddDownload(content::DownloadItem* download) { + [shelf_controller_ addDownloadItem:download]; } bool DownloadShelfMac::IsShowing() const { diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.cc b/chrome/browser/ui/gtk/download/download_item_gtk.cc index 89ca90c..e1da54d 100644 --- a/chrome/browser/ui/gtk/download/download_item_gtk.cc +++ b/chrome/browser/ui/gtk/download/download_item_gtk.cc @@ -104,14 +104,14 @@ NineBox* DownloadItemGtk::dangerous_nine_box_ = NULL; using content::DownloadItem; DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf, - DownloadItemModel* download_model) + DownloadItem* download_item) : parent_shelf_(parent_shelf), arrow_(NULL), menu_showing_(false), theme_service_( GtkThemeService::GetFrom(parent_shelf->browser()->profile())), progress_angle_(download_util::kStartAngleDegrees), - download_model_(download_model), + download_model_(download_item), dangerous_prompt_(NULL), dangerous_label_(NULL), complete_animation_(this), @@ -193,13 +193,13 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf, // Insert as the leftmost item. gtk_box_reorder_child(GTK_BOX(shelf_hbox), hbox_.get(), 0); - get_download()->AddObserver(this); + download()->AddObserver(this); new_item_animation_.reset(new ui::SlideAnimation(this)); new_item_animation_->SetSlideDuration(kNewItemAnimationDurationMs); gtk_widget_show_all(hbox_.get()); - if (download_model_->IsDangerous()) { + if (download_model_.IsDangerous()) { // Hide the download item components for now. gtk_widget_set_no_show_all(body_.get(), TRUE); gtk_widget_set_no_show_all(menu_button_, TRUE); @@ -239,7 +239,7 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf, // Create the ok button. GtkWidget* dangerous_accept = gtk_button_new_with_label( - UTF16ToUTF8(download_model_->GetWarningConfirmButtonText()).c_str()); + UTF16ToUTF8(download_model_.GetWarningConfirmButtonText()).c_str()); g_signal_connect(dangerous_accept, "clicked", G_CALLBACK(OnDangerousAcceptThunk), this); gtk_util::CenterWidgetInHBox(dangerous_hbox_.get(), dangerous_accept, false, @@ -265,7 +265,7 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf, theme_service_->InitThemesFor(this); // Set the initial width of the widget to be animated. - if (download_model_->IsDangerous()) { + if (download_model_.IsDangerous()) { gtk_widget_set_size_request(dangerous_hbox_.get(), dangerous_hbox_start_width_, -1); } else { @@ -278,7 +278,7 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf, complete_animation_.SetSlideDuration(kCompleteAnimationDurationMs); // Update the status text and animation state. - OnDownloadUpdated(get_download()); + OnDownloadUpdated(download()); } DownloadItemGtk::~DownloadItemGtk() { @@ -287,7 +287,7 @@ DownloadItemGtk::~DownloadItemGtk() { menu_.reset(); StopDownloadProgress(); - get_download()->RemoveObserver(this); + download()->RemoveObserver(this); // We may free some shelf space for showing more download items. parent_shelf_->MaybeShowMoreDownloadItems(); @@ -302,10 +302,10 @@ DownloadItemGtk::~DownloadItemGtk() { DCHECK(!status_label_); } -void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download) { - DCHECK_EQ(download, get_download()); +void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download_item) { + DCHECK_EQ(download(), download_item); - if (dangerous_prompt_ != NULL && !download_model_->IsDangerous()) { + if (dangerous_prompt_ != NULL && !download_model_.IsDangerous()) { // We have been approved. gtk_widget_set_no_show_all(body_.get(), FALSE); gtk_widget_set_no_show_all(menu_button_, FALSE); @@ -318,7 +318,7 @@ void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download) { parent_shelf_->MaybeShowMoreDownloadItems(); } - if (download->GetUserVerifiedFilePath() != icon_filepath_) { + if (download()->GetUserVerifiedFilePath() != icon_filepath_) { // Turns out the file path is "Unconfirmed %d.crdownload" for dangerous // downloads. When the download is confirmed, the file is renamed on // another thread, so reload the icon if the download filename changes. @@ -327,7 +327,7 @@ void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download) { UpdateTooltip(); } - switch (download->GetState()) { + switch (download()->GetState()) { case DownloadItem::CANCELLED: StopDownloadProgress(); gtk_widget_queue_draw(progress_area_.get()); @@ -341,7 +341,7 @@ void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download) { case DownloadItem::COMPLETE: // GetAutoOpened() may change after the download's initial transition to // COMPLETE, so we check it before the idemopotency shield below. - if (download->GetAutoOpened()) { + if (download()->GetAutoOpened()) { parent_shelf_->RemoveDownloadItem(this); // This will delete us! return; } @@ -354,24 +354,25 @@ void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download) { StopDownloadProgress(); // Set up the widget as a drag source. - DownloadItemDrag::SetSource(body_.get(), get_download(), icon_large_); + DownloadItemDrag::SetSource(body_.get(), download(), icon_large_); complete_animation_.Show(); download_complete_ = true; break; case DownloadItem::IN_PROGRESS: - get_download()->IsPaused() ? + download()->IsPaused() ? StopDownloadProgress() : StartDownloadProgress(); break; default: NOTREACHED(); } - status_text_ = UTF16ToUTF8(download_model_->GetStatusText()); + status_text_ = UTF16ToUTF8(download_model_.GetStatusText()); UpdateStatusLabel(status_text_); } -void DownloadItemGtk::OnDownloadDestroyed(DownloadItem* download) { +void DownloadItemGtk::OnDownloadDestroyed(DownloadItem* download_item) { + DCHECK_EQ(download(), download_item); parent_shelf_->RemoveDownloadItem(this); // This will delete us! } @@ -381,7 +382,7 @@ void DownloadItemGtk::AnimationProgressed(const ui::Animation* animation) { gtk_widget_queue_draw(progress_area_.get()); } else { DCHECK(animation == new_item_animation_.get()); - if (download_model_->IsDangerous()) { + if (download_model_.IsDangerous()) { int progress = static_cast<int>((dangerous_hbox_full_width_ - dangerous_hbox_start_width_) * animation->GetCurrentValue()); @@ -427,10 +428,6 @@ void DownloadItemGtk::Observe(int type, } } -DownloadItem* DownloadItemGtk::get_download() { - return download_model_->download(); -} - // Download progress animation functions. void DownloadItemGtk::UpdateDownloadProgress() { @@ -461,13 +458,13 @@ void DownloadItemGtk::OnLoadSmallIconComplete(gfx::Image* image) { void DownloadItemGtk::OnLoadLargeIconComplete(gfx::Image* image) { icon_large_ = image; - DownloadItemDrag::SetSource(body_.get(), get_download(), icon_large_); + DownloadItemDrag::SetSource(body_.get(), download(), icon_large_); } void DownloadItemGtk::LoadIcon() { cancelable_task_tracker_.TryCancelAll(); IconManager* im = g_browser_process->icon_manager(); - icon_filepath_ = get_download()->GetUserVerifiedFilePath(); + icon_filepath_ = download()->GetUserVerifiedFilePath(); im->LoadIcon(icon_filepath_, IconLoader::SMALL, base::Bind(&DownloadItemGtk::OnLoadSmallIconComplete, @@ -482,7 +479,7 @@ void DownloadItemGtk::LoadIcon() { void DownloadItemGtk::UpdateTooltip() { string16 tooltip_text = - download_model_->GetTooltipText(gfx::Font(), kTooltipMaxWidth); + download_model_.GetTooltipText(gfx::Font(), kTooltipMaxWidth); gtk_widget_set_tooltip_text(body_.get(), UTF16ToUTF8(tooltip_text).c_str()); } @@ -495,7 +492,7 @@ void DownloadItemGtk::UpdateNameLabel() { string16 filename; if (!disabled_while_opening_) { filename = ui::ElideFilename( - get_download()->GetFileNameToReportUser(), font, kTextWidth); + download()->GetFileNameToReportUser(), font, kTextWidth); } else { // First, Calculate the download status opening string width. string16 status_string = @@ -503,7 +500,7 @@ void DownloadItemGtk::UpdateNameLabel() { int status_string_width = font.GetStringWidth(status_string); // Then, elide the file name. string16 filename_string = - ui::ElideFilename(get_download()->GetFileNameToReportUser(), font, + ui::ElideFilename(download()->GetFileNameToReportUser(), font, kTextWidth - status_string_width); // Last, concat the whole string. filename = l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_OPENING, @@ -582,7 +579,7 @@ void DownloadItemGtk::UpdateDangerWarning() { // We create |dangerous_warning| as a wide string so we can more easily // calculate its length in characters. string16 dangerous_warning = - download_model_->GetWarningText(gfx::Font(), kTextWidth); + download_model_.GetWarningText(gfx::Font(), kTextWidth); if (theme_service_->UsingNativeTheme()) { gtk_util::SetLabelColor(dangerous_label_, NULL); } else { @@ -629,15 +626,15 @@ void DownloadItemGtk::UpdateDangerWarning() { void DownloadItemGtk::UpdateDangerIcon() { if (theme_service_->UsingNativeTheme()) { - const char* stock = download_model_->IsMalicious() ? + const char* stock = download_model_.IsMalicious() ? GTK_STOCK_DIALOG_ERROR : GTK_STOCK_DIALOG_WARNING; gtk_image_set_from_stock( GTK_IMAGE(dangerous_image_), stock, GTK_ICON_SIZE_SMALL_TOOLBAR); } else { // Set the warning icon. ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - int pixbuf_id = download_model_->IsMalicious() ? IDR_SAFEBROWSING_WARNING - : IDR_WARNING; + int pixbuf_id = download_model_.IsMalicious() ? IDR_SAFEBROWSING_WARNING + : IDR_WARNING; gtk_image_set_from_pixbuf(GTK_IMAGE(dangerous_image_), rb.GetNativeImageNamed(pixbuf_id).ToGdkPixbuf()); } @@ -719,7 +716,7 @@ gboolean DownloadItemGtk::OnHboxExpose(GtkWidget* widget, GdkEventExpose* e) { int width = allocation.width - border_width * 2; int height = allocation.height - border_width * 2; - if (download_model_->IsDangerous()) { + if (download_model_.IsDangerous()) { // Draw a simple frame around the area when we're displaying the warning. gtk_paint_shadow(gtk_widget_get_style(widget), gtk_widget_get_window(widget), @@ -830,7 +827,7 @@ void DownloadItemGtk::OnDownloadOpened(DownloadItem* download) { void DownloadItemGtk::OnClick(GtkWidget* widget) { UMA_HISTOGRAM_LONG_TIMES("clickjacking.open_download", base::Time::Now() - creation_time_); - get_download()->OpenDownload(); + download()->OpenDownload(); } gboolean DownloadItemGtk::OnButtonPress(GtkWidget* button, @@ -852,7 +849,7 @@ gboolean DownloadItemGtk::OnProgressAreaExpose(GtkWidget* widget, // Create a transparent canvas. gfx::CanvasSkiaPaint canvas(event, false); if (complete_animation_.is_animating()) { - if (get_download()->IsInterrupted()) { + if (download()->IsInterrupted()) { download_util::PaintDownloadInterrupted(&canvas, allocation.x, allocation.y, complete_animation_.GetCurrentValue(), @@ -863,11 +860,11 @@ gboolean DownloadItemGtk::OnProgressAreaExpose(GtkWidget* widget, complete_animation_.GetCurrentValue(), download_util::SMALL); } - } else if (get_download()->IsInProgress()) { + } else if (download()->IsInProgress()) { download_util::PaintDownloadProgress(&canvas, allocation.x, allocation.y, progress_angle_, - download_model_->PercentComplete(), + download_model_.PercentComplete(), download_util::SMALL); } @@ -901,8 +898,7 @@ void DownloadItemGtk::ShowPopupMenu(GtkWidget* button, complete_animation_.End(); if (!menu_.get()) { - menu_.reset(new DownloadShelfContextMenuGtk(download_model_.get(), - this, + menu_.reset(new DownloadShelfContextMenuGtk(this, parent_shelf_->GetNavigator())); } menu_->Popup(button, event); @@ -921,13 +917,13 @@ gboolean DownloadItemGtk::OnDangerousPromptExpose(GtkWidget* widget, void DownloadItemGtk::OnDangerousAccept(GtkWidget* button) { UMA_HISTOGRAM_LONG_TIMES("clickjacking.save_download", base::Time::Now() - creation_time_); - get_download()->DangerousDownloadValidated(); + download()->DangerousDownloadValidated(); } void DownloadItemGtk::OnDangerousDecline(GtkWidget* button) { UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download", base::Time::Now() - creation_time_); - if (get_download()->IsPartialDownload()) - get_download()->Cancel(true); - get_download()->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); + if (download()->IsPartialDownload()) + download()->Cancel(true); + download()->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); } diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.h b/chrome/browser/ui/gtk/download/download_item_gtk.h index 0efcab4..e659a7d 100644 --- a/chrome/browser/ui/gtk/download/download_item_gtk.h +++ b/chrome/browser/ui/gtk/download/download_item_gtk.h @@ -14,6 +14,7 @@ #include "base/memory/weak_ptr.h" #include "base/time.h" #include "base/timer.h" +#include "chrome/browser/download/download_item_model.h" #include "chrome/browser/icon_manager.h" #include "chrome/common/cancelable_task_tracker.h" #include "content/public/browser/download_item.h" @@ -24,7 +25,6 @@ #include "ui/base/gtk/gtk_signal.h" #include "ui/base/gtk/owned_widget_gtk.h" -class DownloadItemModel; class DownloadShelfContextMenuGtk; class DownloadShelfGtk; class GtkThemeService; @@ -44,7 +44,7 @@ class DownloadItemGtk : public content::DownloadItem::Observer, public: // DownloadItemGtk takes ownership of |download_item_model|. DownloadItemGtk(DownloadShelfGtk* parent_shelf, - DownloadItemModel* download_item_model); + content::DownloadItem* download_item); // Destroys all widgets belonging to this DownloadItemGtk. virtual ~DownloadItemGtk(); @@ -68,7 +68,7 @@ class DownloadItemGtk : public content::DownloadItem::Observer, void OnLoadLargeIconComplete(gfx::Image* image); // Returns the DownloadItem model object belonging to this item. - content::DownloadItem* get_download(); + content::DownloadItem* download() { return download_model_.download(); } private: friend class DownloadShelfContextMenuGtk; @@ -199,7 +199,7 @@ class DownloadItemGtk : public content::DownloadItem::Observer, scoped_ptr<DownloadShelfContextMenuGtk> menu_; // The download item model we represent. - scoped_ptr<DownloadItemModel> download_model_; + DownloadItemModel download_model_; // The dangerous download dialog. This will be null for safe downloads. GtkWidget* dangerous_prompt_; diff --git a/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.cc b/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.cc index 1441a93..75da2df 100644 --- a/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.cc +++ b/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.cc @@ -11,10 +11,9 @@ #include "ui/gfx/point.h" DownloadShelfContextMenuGtk::DownloadShelfContextMenuGtk( - DownloadItemModel* model, DownloadItemGtk* download_item, content::PageNavigator* navigator) - : DownloadShelfContextMenu(model, navigator), + : DownloadShelfContextMenu(download_item->download(), navigator), download_item_gtk_(download_item) { } @@ -22,7 +21,11 @@ DownloadShelfContextMenuGtk::~DownloadShelfContextMenuGtk() {} void DownloadShelfContextMenuGtk::Popup(GtkWidget* widget, GdkEventButton* event) { - menu_.reset(new MenuGtk(this, GetMenuModel())); + ui::SimpleMenuModel* menu_model = GetMenuModel(); + // Popup() should never be called after the DownloadItem is destroyed. + DCHECK(menu_model); + + menu_.reset(new MenuGtk(this, menu_model)); if (widget) menu_->PopupForWidget(widget, event->button, event->time); diff --git a/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.h b/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.h index bc079e3..00c3644 100644 --- a/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.h +++ b/chrome/browser/ui/gtk/download/download_shelf_context_menu_gtk.h @@ -12,13 +12,11 @@ #include "chrome/browser/ui/gtk/menu_gtk.h" class DownloadItemGtk; -class DownloadItemModel; class DownloadShelfContextMenuGtk : public DownloadShelfContextMenu, public MenuGtk::Delegate { public: - DownloadShelfContextMenuGtk(DownloadItemModel* model, - DownloadItemGtk* download_item, + DownloadShelfContextMenuGtk(DownloadItemGtk* download_item, content::PageNavigator* navigator); virtual ~DownloadShelfContextMenuGtk(); diff --git a/chrome/browser/ui/gtk/download/download_shelf_gtk.cc b/chrome/browser/ui/gtk/download/download_shelf_gtk.cc index 18c7563..7ce574c 100644 --- a/chrome/browser/ui/gtk/download/download_shelf_gtk.cc +++ b/chrome/browser/ui/gtk/download/download_shelf_gtk.cc @@ -179,8 +179,8 @@ content::PageNavigator* DownloadShelfGtk::GetNavigator() { return browser_; } -void DownloadShelfGtk::DoAddDownload(DownloadItemModel* download_model) { - download_items_.push_back(new DownloadItemGtk(this, download_model)); +void DownloadShelfGtk::DoAddDownload(DownloadItem* download) { + download_items_.push_back(new DownloadItemGtk(this, download)); } bool DownloadShelfGtk::IsShowing() const { @@ -205,7 +205,7 @@ void DownloadShelfGtk::DoClose() { browser_->UpdateDownloadShelfVisibility(false); int num_in_progress = 0; for (size_t i = 0; i < download_items_.size(); ++i) { - if (download_items_[i]->get_download()->IsInProgress()) + if (download_items_[i]->download()->IsInProgress()) ++num_in_progress; } download_util::RecordShelfClose( @@ -225,7 +225,7 @@ void DownloadShelfGtk::Closed() { // When the close animation is complete, remove all completed downloads. size_t i = 0; while (i < download_items_.size()) { - DownloadItem* download = download_items_[i]->get_download(); + DownloadItem* download = download_items_[i]->download(); bool is_transfer_done = download->IsComplete() || download->IsCancelled() || download->IsInterrupted(); @@ -316,7 +316,7 @@ void DownloadShelfGtk::OnButtonClick(GtkWidget* button) { void DownloadShelfGtk::AutoCloseIfPossible() { for (std::vector<DownloadItemGtk*>::iterator iter = download_items_.begin(); iter != download_items_.end(); ++iter) { - if (!(*iter)->get_download()->GetOpened()) + if (!(*iter)->download()->GetOpened()) return; } diff --git a/chrome/browser/ui/gtk/download/download_shelf_gtk.h b/chrome/browser/ui/gtk/download/download_shelf_gtk.h index 05a98591..ebbee9b 100644 --- a/chrome/browser/ui/gtk/download/download_shelf_gtk.h +++ b/chrome/browser/ui/gtk/download/download_shelf_gtk.h @@ -24,7 +24,6 @@ class Browser; class CustomDrawButton; class DownloadItemGtk; -class DownloadItemModel; class GtkThemeService; namespace content { @@ -69,7 +68,7 @@ class DownloadShelfGtk : public DownloadShelf, protected: // DownloadShelf implementation. - virtual void DoAddDownload(DownloadItemModel* download_model) OVERRIDE; + virtual void DoAddDownload(content::DownloadItem* download) OVERRIDE; virtual void DoShow() OVERRIDE; virtual void DoClose() OVERRIDE; diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc index 7fc4f0a..13bbd761 100644 --- a/chrome/browser/ui/views/download/download_item_view.cc +++ b/chrome/browser/ui/views/download/download_item_view.cc @@ -82,11 +82,9 @@ static const double kDownloadItemLuminanceMod = 0.8; using content::DownloadItem; -DownloadItemView::DownloadItemView(DownloadItem* download, - DownloadShelfView* parent, - DownloadItemModel* model) +DownloadItemView::DownloadItemView(DownloadItem* download_item, + DownloadShelfView* parent) : warning_icon_(NULL), - download_(download), shelf_(parent), status_text_(l10n_util::GetStringUTF16(IDS_DOWNLOAD_STATUS_STARTING)), body_state_(NORMAL), @@ -96,7 +94,7 @@ DownloadItemView::DownloadItemView(DownloadItem* download, drop_down_pressed_(false), dragging_(false), starting_drag_(false), - model_(model), + model_(download_item), save_button_(NULL), discard_button_(NULL), dangerous_download_label_(NULL), @@ -104,8 +102,8 @@ DownloadItemView::DownloadItemView(DownloadItem* download, disabled_while_opening_(false), creation_time_(base::Time::Now()), ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) { - DCHECK(download_); - download_->AddObserver(this); + DCHECK(download()); + download()->AddObserver(this); set_context_menu_controller(this); ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); @@ -204,13 +202,13 @@ DownloadItemView::DownloadItemView(DownloadItem* download, set_accessibility_focusable(true); - OnDownloadUpdated(download_); + OnDownloadUpdated(download()); UpdateDropDownButtonPosition(); } DownloadItemView::~DownloadItemView() { StopDownloadProgress(); - download_->RemoveObserver(this); + download()->RemoveObserver(this); } // Progress animation handlers. @@ -243,22 +241,22 @@ void DownloadItemView::OnExtractIconComplete(gfx::Image* icon_bitmap) { // Update the progress graphic on the icon and our text status label // to reflect our current bytes downloaded, time remaining. -void DownloadItemView::OnDownloadUpdated(DownloadItem* download) { - DCHECK_EQ(download_, download); +void DownloadItemView::OnDownloadUpdated(DownloadItem* download_item) { + DCHECK_EQ(download(), download_item); - if (IsShowingWarningDialog() && !model_->IsDangerous()) { + if (IsShowingWarningDialog() && !model_.IsDangerous()) { // We have been approved. ClearWarningDialog(); - } else if (!IsShowingWarningDialog() && model_->IsDangerous()) { + } else if (!IsShowingWarningDialog() && model_.IsDangerous()) { ShowWarningDialog(); // Force the shelf to layout again as our size has changed. shelf_->Layout(); SchedulePaint(); } else { - string16 status_text = model_->GetStatusText(); - switch (download_->GetState()) { + string16 status_text = model_.GetStatusText(); + switch (download()->GetState()) { case DownloadItem::IN_PROGRESS: - download_->IsPaused() ? + download()->IsPaused() ? StopDownloadProgress() : StartDownloadProgress(); LoadIconIfItemPathChanged(); break; @@ -272,7 +270,7 @@ void DownloadItemView::OnDownloadUpdated(DownloadItem* download) { LoadIcon(); break; case DownloadItem::COMPLETE: - if (download_->GetAutoOpened()) { + if (download()->GetAutoOpened()) { shelf_->RemoveDownloadView(this); // This will delete us! return; } @@ -294,7 +292,7 @@ void DownloadItemView::OnDownloadUpdated(DownloadItem* download) { status_text_ = status_text; } - string16 new_tip = model_->GetTooltipText(font_, kTooltipMaxWidth); + string16 new_tip = model_.GetTooltipText(font_, kTooltipMaxWidth); if (new_tip != tooltip_text_) { tooltip_text_ = new_tip; TooltipTextChanged(); @@ -406,13 +404,13 @@ bool DownloadItemView::OnMouseDragged(const ui::MouseEvent& event) { drag_start_point_ = event.location(); } if (dragging_) { - if (download_->IsComplete()) { + if (download()->IsComplete()) { IconManager* im = g_browser_process->icon_manager(); - gfx::Image* icon = im->LookupIcon(download_->GetUserVerifiedFilePath(), + gfx::Image* icon = im->LookupIcon(download()->GetUserVerifiedFilePath(), IconLoader::SMALL); if (icon) { views::Widget* widget = GetWidget(); - download_util::DragDownload(download_, icon, + download_util::DragDownload(download(), icon, widget ? widget->GetNativeView() : NULL); } } @@ -484,7 +482,7 @@ bool DownloadItemView::GetTooltipText(const gfx::Point& p, void DownloadItemView::GetAccessibleState(ui::AccessibleViewState* state) { state->name = accessible_name_; state->role = ui::AccessibilityTypes::ROLE_PUSHBUTTON; - if (model_->IsDangerous()) { + if (model_.IsDangerous()) { state->state = ui::AccessibilityTypes::STATE_UNAVAILABLE; } else { state->state = ui::AccessibilityTypes::STATE_HASPOPUP; @@ -525,9 +523,9 @@ void DownloadItemView::ButtonPressed( if (sender == discard_button_) { UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download", base::Time::Now() - creation_time_); - if (download_->IsPartialDownload()) - download_->Cancel(true); - download_->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); + if (download()->IsPartialDownload()) + download()->Cancel(true); + download()->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD); // WARNING: we are deleted at this point. Don't access 'this'. } else if (save_button_ && sender == save_button_) { // The user has confirmed a dangerous download. We'd record how quickly the @@ -535,7 +533,7 @@ void DownloadItemView::ButtonPressed( UMA_HISTOGRAM_LONG_TIMES("clickjacking.save_download", base::Time::Now() - creation_time_); // This will change the state and notify us. - download_->DangerousDownloadValidated(); + download()->DangerousDownloadValidated(); } } @@ -742,7 +740,7 @@ void DownloadItemView::OnPaint(gfx::Canvas* canvas) { if (!IsShowingWarningDialog()) { string16 filename; if (!disabled_while_opening_) { - filename = ui::ElideFilename(download_->GetFileNameToReportUser(), + filename = ui::ElideFilename(download()->GetFileNameToReportUser(), font_, kTextWidth); } else { // First, Calculate the download status opening string width. @@ -751,7 +749,7 @@ void DownloadItemView::OnPaint(gfx::Canvas* canvas) { int status_string_width = font_.GetStringWidth(status_string); // Then, elide the file name. string16 filename_string = - ui::ElideFilename(download_->GetFileNameToReportUser(), font_, + ui::ElideFilename(download()->GetFileNameToReportUser(), font_, kTextWidth - status_string_width); // Last, concat the whole string. filename = l10n_util::GetStringFUTF16(IDS_DOWNLOAD_STATUS_OPENING, @@ -775,7 +773,7 @@ void DownloadItemView::OnPaint(gfx::Canvas* canvas) { // Load the icon. IconManager* im = g_browser_process->icon_manager(); - gfx::Image* image = im->LookupIcon(download_->GetUserVerifiedFilePath(), + gfx::Image* image = im->LookupIcon(download()->GetUserVerifiedFilePath(), IconLoader::SMALL); const gfx::ImageSkia* icon = NULL; if (IsShowingWarningDialog()) @@ -790,15 +788,15 @@ void DownloadItemView::OnPaint(gfx::Canvas* canvas) { // triggered only when we think the status might change. if (icon) { if (!IsShowingWarningDialog()) { - if (download_->IsInProgress()) { + if (download()->IsInProgress()) { download_util::PaintDownloadProgress(canvas, this, 0, 0, progress_angle_, - model_->PercentComplete(), + model_.PercentComplete(), download_util::SMALL); - } else if (download_->IsComplete() && + } else if (download()->IsComplete() && complete_animation_.get() && complete_animation_->is_animating()) { - if (download_->IsInterrupted()) { + if (download()->IsInterrupted()) { download_util::PaintDownloadInterrupted(canvas, this, 0, 0, complete_animation_->GetCurrentValue(), download_util::SMALL); @@ -838,13 +836,13 @@ void DownloadItemView::OpenDownload() { // open downloads super quickly, we should be concerned about clickjacking. UMA_HISTOGRAM_LONG_TIMES("clickjacking.open_download", base::Time::Now() - creation_time_); - download_->OpenDownload(); + download()->OpenDownload(); UpdateAccessibleName(); } void DownloadItemView::LoadIcon() { IconManager* im = g_browser_process->icon_manager(); - last_download_item_path_ = download_->GetUserVerifiedFilePath(); + last_download_item_path_ = download()->GetUserVerifiedFilePath(); im->LoadIcon(last_download_item_path_, IconLoader::SMALL, base::Bind(&DownloadItemView::OnExtractIconComplete, @@ -853,7 +851,7 @@ void DownloadItemView::LoadIcon() { } void DownloadItemView::LoadIconIfItemPathChanged() { - FilePath current_download_path = download_->GetUserVerifiedFilePath(); + FilePath current_download_path = download()->GetUserVerifiedFilePath(); if (last_download_item_path_ == current_download_path) return; @@ -902,8 +900,7 @@ void DownloadItemView::ShowContextMenuImpl(const gfx::Point& p, if (!context_menu_.get()) { context_menu_.reset( - new DownloadShelfContextMenuView(model_.get(), - shelf_->GetNavigator())); + new DownloadShelfContextMenuView(download(), shelf_->GetNavigator())); } context_menu_->Run(GetWidget()->GetTopLevelWidget(), gfx::Rect(point, size)); @@ -1000,8 +997,8 @@ void DownloadItemView::SetState(State new_body_state, State new_drop_state) { } void DownloadItemView::ClearWarningDialog() { - DCHECK(download_->GetSafetyState() == DownloadItem::DANGEROUS_BUT_VALIDATED && - (mode_ == DANGEROUS_MODE || mode_ == MALICIOUS_MODE)); + DCHECK(download()->GetSafetyState() == DownloadItem::DANGEROUS_BUT_VALIDATED); + DCHECK(mode_ == DANGEROUS_MODE || mode_ == MALICIOUS_MODE); mode_ = NORMAL_MODE; body_state_ = NORMAL; @@ -1027,7 +1024,7 @@ void DownloadItemView::ClearWarningDialog() { UpdateAccessibleName(); UpdateDropDownButtonPosition(); - // We need to load the icon now that the download_ has the real path. + // We need to load the icon now that the download has the real path. LoadIcon(); // Force the shelf to layout again as our size has changed. @@ -1039,13 +1036,13 @@ void DownloadItemView::ClearWarningDialog() { void DownloadItemView::ShowWarningDialog() { DCHECK(mode_ != DANGEROUS_MODE && mode_ != MALICIOUS_MODE); - mode_ = ((model_->IsMalicious()) ? MALICIOUS_MODE : DANGEROUS_MODE); + mode_ = ((model_.IsMalicious()) ? MALICIOUS_MODE : DANGEROUS_MODE); body_state_ = NORMAL; drop_down_state_ = NORMAL; if (mode_ == DANGEROUS_MODE) { save_button_ = new views::NativeTextButton( - this, model_->GetWarningConfirmButtonText()); + this, model_.GetWarningConfirmButtonText()); save_button_->set_ignore_minimum_size(true); AddChildView(save_button_); } @@ -1063,7 +1060,7 @@ void DownloadItemView::ShowWarningDialog() { // The download file has dangerous file type (e.g.: an executable). warning_icon_ = rb.GetImageSkiaNamed(IDR_WARNING); } - string16 dangerous_label = model_->GetWarningText(font_, kTextWidth); + string16 dangerous_label = model_.GetWarningText(font_, kTextWidth); dangerous_download_label_ = new views::Label(dangerous_label); dangerous_download_label_->SetMultiLine(true); dangerous_download_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT); @@ -1185,7 +1182,7 @@ void DownloadItemView::UpdateAccessibleName() { new_name = dangerous_download_label_->text(); } else { new_name = status_text_ + char16(' ') + - download_->GetFileNameToReportUser().LossyDisplayName(); + download()->GetFileNameToReportUser().LossyDisplayName(); } // If the name has changed, notify assistive technology that the name diff --git a/chrome/browser/ui/views/download/download_item_view.h b/chrome/browser/ui/views/download/download_item_view.h index e454ce7..b14dddc 100644 --- a/chrome/browser/ui/views/download/download_item_view.h +++ b/chrome/browser/ui/views/download/download_item_view.h @@ -25,6 +25,7 @@ #include "base/time.h" #include "base/timer.h" #include "chrome/browser/common/cancelable_request.h" +#include "chrome/browser/download/download_item_model.h" #include "chrome/browser/icon_manager.h" #include "content/public/browser/download_item.h" #include "content/public/browser/download_manager.h" @@ -34,7 +35,6 @@ #include "ui/views/controls/button/button.h" #include "ui/views/view.h" -class DownloadItemModel; class DownloadShelfView; class DownloadShelfContextMenuView; @@ -59,8 +59,7 @@ class DownloadItemView : public views::ButtonListener, public ui::AnimationDelegate { public: DownloadItemView(content::DownloadItem* download, - DownloadShelfView* parent, - DownloadItemModel* model); + DownloadShelfView* parent); virtual ~DownloadItemView(); // Timer callback for handling animations @@ -72,7 +71,7 @@ class DownloadItemView : public views::ButtonListener, void OnExtractIconComplete(gfx::Image* icon); // Returns the DownloadItem model object belonging to this item. - content::DownloadItem* download() const { return download_; } + content::DownloadItem* download() { return model_.download(); } // DownloadItem::Observer methods virtual void OnDownloadUpdated(content::DownloadItem* download) OVERRIDE; @@ -233,9 +232,6 @@ class DownloadItemView : public views::ButtonListener, // The warning icon showns for dangerous downloads. const gfx::ImageSkia* warning_icon_; - // The model we query for display information - content::DownloadItem* download_; - // The download shelf that owns us. DownloadShelfView* shelf_; @@ -283,10 +279,8 @@ class DownloadItemView : public views::ButtonListener, // For canceling an in progress icon request. CancelableTaskTracker cancelable_task_tracker_; - // A model class to control the status text we display and the cancel - // behavior. - // This class owns the pointer. - scoped_ptr<DownloadItemModel> model_; + // A model class to control the status text we display. + DownloadItemModel model_; // Hover animations for our body and drop buttons. scoped_ptr<ui::SlideAnimation> body_hover_animation_; diff --git a/chrome/browser/ui/views/download/download_shelf_context_menu_view.cc b/chrome/browser/ui/views/download/download_shelf_context_menu_view.cc index 8191673..a38fd94 100644 --- a/chrome/browser/ui/views/download/download_shelf_context_menu_view.cc +++ b/chrome/browser/ui/views/download/download_shelf_context_menu_view.cc @@ -15,16 +15,20 @@ #include "ui/views/controls/menu/menu_runner.h" DownloadShelfContextMenuView::DownloadShelfContextMenuView( - DownloadItemModel* model, + content::DownloadItem* download_item, content::PageNavigator* navigator) - : DownloadShelfContextMenu(model, navigator) { + : DownloadShelfContextMenu(download_item, navigator) { } DownloadShelfContextMenuView::~DownloadShelfContextMenuView() {} void DownloadShelfContextMenuView::Run(views::Widget* parent_widget, const gfx::Rect& rect) { - views::MenuModelAdapter menu_model_adapter(GetMenuModel()); + ui::MenuModel* menu_model = GetMenuModel(); + // Run() should not be getting called if the DownloadItem was destroyed. + DCHECK(menu_model); + + views::MenuModelAdapter menu_model_adapter(menu_model); menu_runner_.reset(new views::MenuRunner(menu_model_adapter.CreateMenu())); // The menu's alignment is determined based on the UI layout. diff --git a/chrome/browser/ui/views/download/download_shelf_context_menu_view.h b/chrome/browser/ui/views/download/download_shelf_context_menu_view.h index 7039a11..916dc0a 100644 --- a/chrome/browser/ui/views/download/download_shelf_context_menu_view.h +++ b/chrome/browser/ui/views/download/download_shelf_context_menu_view.h @@ -10,9 +10,8 @@ #include "base/memory/scoped_ptr.h" #include "chrome/browser/download/download_shelf_context_menu.h" -class DownloadItemModel; - namespace content { +class DownloadItem; class PageNavigator; } @@ -27,7 +26,7 @@ class Widget; class DownloadShelfContextMenuView : public DownloadShelfContextMenu { public: - DownloadShelfContextMenuView(DownloadItemModel* model, + DownloadShelfContextMenuView(content::DownloadItem* download_item, content::PageNavigator* navigator); virtual ~DownloadShelfContextMenuView(); diff --git a/chrome/browser/ui/views/download/download_shelf_view.cc b/chrome/browser/ui/views/download/download_shelf_view.cc index 99a060c..22af358 100644 --- a/chrome/browser/ui/views/download/download_shelf_view.cc +++ b/chrome/browser/ui/views/download/download_shelf_view.cc @@ -120,9 +120,8 @@ void DownloadShelfView::AddDownloadView(DownloadItemView* view) { new_item_animation_->Show(); } -void DownloadShelfView::DoAddDownload(DownloadItemModel* download_model) { - DownloadItemView* view = new DownloadItemView( - download_model->download(), this, download_model); +void DownloadShelfView::DoAddDownload(DownloadItem* download) { + DownloadItemView* view = new DownloadItemView(download, this); AddDownloadView(view); } diff --git a/chrome/browser/ui/views/download/download_shelf_view.h b/chrome/browser/ui/views/download/download_shelf_view.h index df75272..b91cf95 100644 --- a/chrome/browser/ui/views/download/download_shelf_view.h +++ b/chrome/browser/ui/views/download/download_shelf_view.h @@ -19,9 +19,9 @@ class Browser; class BrowserView; class DownloadItemView; -class DownloadItemModel; namespace content { +class DownloadItem; class PageNavigator; } @@ -98,7 +98,7 @@ class DownloadShelfView : public views::AccessiblePaneView, protected: // Implementation of DownloadShelf. - virtual void DoAddDownload(DownloadItemModel* download_model) OVERRIDE; + virtual void DoAddDownload(content::DownloadItem* download) OVERRIDE; virtual void DoShow() OVERRIDE; virtual void DoClose() OVERRIDE; |