diff options
author | stevenjb@chromium.org <stevenjb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-19 18:10:49 +0000 |
---|---|---|
committer | stevenjb@chromium.org <stevenjb@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-19 18:10:49 +0000 |
commit | c2a7429976443bbca712e9e23012f32416a980de (patch) | |
tree | dd3bd8c6bef622f300638470439ecf8e7959a209 /ui | |
parent | b645bd4b8f85f9a059b42bd4f85f37ddd987bc02 (diff) | |
download | chromium_src-c2a7429976443bbca712e9e23012f32416a980de.zip chromium_src-c2a7429976443bbca712e9e23012f32416a980de.tar.gz chromium_src-c2a7429976443bbca712e9e23012f32416a980de.tar.bz2 |
Introduce AppListSyncableService, owns AppListModel and builder
This introduces AppListSyncableService, but does not implement any
syncing yet. The purpose is to move ownership of AppListModel so that
we can maintain and sync the model state independent of the view. It also
made sense to move ExtensionAppModelBuilder ownership to
AppListSyncableService.
This required a fair bit of (smallish) re-factoring. Here is a simplified
diagram that may help:
https://docs.google.com/a/google.com/drawings/d/1NgaQzdqddTh1W7altZ6P6t4dNxZDRO0e_HgJqa8V36A/edit
BUG=315887
For minor ash/shell/ changes:
R=jennyz@chromium.org, tapted@chromium.org, xiyuan@chromium.org
TBR=jamescook@chromium.org
Review URL: https://codereview.chromium.org/66023003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236020 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ui')
-rw-r--r-- | ui/app_list/app_list_view_delegate.h | 8 | ||||
-rw-r--r-- | ui/app_list/cocoa/app_list_view_controller.h | 3 | ||||
-rw-r--r-- | ui/app_list/cocoa/app_list_view_controller.mm | 13 | ||||
-rw-r--r-- | ui/app_list/cocoa/app_list_view_controller_unittest.mm | 26 | ||||
-rw-r--r-- | ui/app_list/cocoa/apps_grid_controller.h | 3 | ||||
-rw-r--r-- | ui/app_list/cocoa/apps_grid_controller.mm | 58 | ||||
-rw-r--r-- | ui/app_list/cocoa/apps_grid_controller_unittest.mm | 10 | ||||
-rw-r--r-- | ui/app_list/cocoa/test/apps_grid_controller_test_helper.h | 11 | ||||
-rw-r--r-- | ui/app_list/cocoa/test/apps_grid_controller_test_helper.mm | 17 | ||||
-rw-r--r-- | ui/app_list/test/app_list_test_model.cc | 2 | ||||
-rw-r--r-- | ui/app_list/test/app_list_test_view_delegate.cc | 14 | ||||
-rw-r--r-- | ui/app_list/test/app_list_test_view_delegate.h | 14 | ||||
-rw-r--r-- | ui/app_list/views/app_list_main_view.cc | 25 | ||||
-rw-r--r-- | ui/app_list/views/app_list_main_view.h | 5 | ||||
-rw-r--r-- | ui/app_list/views/app_list_view.cc | 30 | ||||
-rw-r--r-- | ui/app_list/views/app_list_view.h | 4 | ||||
-rw-r--r-- | ui/ui_unittests.gyp | 4 |
17 files changed, 116 insertions, 131 deletions
diff --git a/ui/app_list/app_list_view_delegate.h b/ui/app_list/app_list_view_delegate.h index 563afc4..f2634a4 100644 --- a/ui/app_list/app_list_view_delegate.h +++ b/ui/app_list/app_list_view_delegate.h @@ -64,11 +64,9 @@ class APP_LIST_EXPORT AppListViewDelegate { // only used by non-Ash Windows. virtual void SetProfileByPath(const base::FilePath& profile_path) = 0; - // Invoked to initialize the model that AppListView uses. This binds the given - // model to this AppListViewDelegate and makes the AppListViewDelegate - // responsible for updating the model. - // Note that AppListView owns the model. - virtual void InitModel(AppListModel* model) = 0; + // Gets the model associated with the view delegate. The model may be owned + // by the delegate, or owned elsewhere (e.g. a profile keyed service). + virtual AppListModel* GetModel() = 0; // Gets the SigninDelegate for the app list. Owned by the AppListViewDelegate. virtual SigninDelegate* GetSigninDelegate() = 0; diff --git a/ui/app_list/cocoa/app_list_view_controller.h b/ui/app_list/cocoa/app_list_view_controller.h index b993a67..3573ab1 100644 --- a/ui/app_list/cocoa/app_list_view_controller.h +++ b/ui/app_list/cocoa/app_list_view_controller.h @@ -63,9 +63,6 @@ APP_LIST_EXPORT - (NSSegmentedControl*)pagerControl; - (NSView*)backgroundView; -- (void)setDelegate:(scoped_ptr<app_list::AppListViewDelegate>)newDelegate - withTestModel:(scoped_ptr<app_list::AppListModel>)newModel; - @end #endif // UI_APP_LIST_COCOA_APP_LIST_VIEW_CONTROLLER_H_ diff --git a/ui/app_list/cocoa/app_list_view_controller.mm b/ui/app_list/cocoa/app_list_view_controller.mm index d9b4f54..51a71cf 100644 --- a/ui/app_list/cocoa/app_list_view_controller.mm +++ b/ui/app_list/cocoa/app_list_view_controller.mm @@ -150,18 +150,18 @@ void AppListModelObserverBridge::OnAppListModelSigninStatusChanged() { return delegate_.get(); } -- (void)setDelegate:(scoped_ptr<app_list::AppListViewDelegate>)newDelegate - withTestModel:(scoped_ptr<app_list::AppListModel>)newModel { +- (void)setDelegate:(scoped_ptr<app_list::AppListViewDelegate>)newDelegate { if (delegate_) { // First clean up, in reverse order. app_list_model_observer_bridge_.reset(); [appsSearchResultsController_ setDelegate:nil]; [appsSearchBoxController_ setDelegate:nil]; + [appsGridController_ setDelegate:nil]; } delegate_.reset(newDelegate.release()); + if (!delegate_) + return; [appsGridController_ setDelegate:delegate_.get()]; - if (newModel.get()) - [appsGridController_ setModel:newModel.Pass()]; [appsSearchBoxController_ setDelegate:self]; [appsSearchResultsController_ setDelegate:self]; app_list_model_observer_bridge_.reset( @@ -169,11 +169,6 @@ void AppListModelObserverBridge::OnAppListModelSigninStatusChanged() { [self onSigninStatusChanged]; } -- (void)setDelegate:(scoped_ptr<app_list::AppListViewDelegate>)newDelegate { - [self setDelegate:newDelegate.Pass() - withTestModel:scoped_ptr<app_list::AppListModel>()]; -} - -(void)loadAndSetView { pagerControl_.reset([[AppListPagerView alloc] init]); [pagerControl_ setTarget:appsGridController_]; diff --git a/ui/app_list/cocoa/app_list_view_controller_unittest.mm b/ui/app_list/cocoa/app_list_view_controller_unittest.mm index 2836ceb..2f1ea39 100644 --- a/ui/app_list/cocoa/app_list_view_controller_unittest.mm +++ b/ui/app_list/cocoa/app_list_view_controller_unittest.mm @@ -21,6 +21,10 @@ class AppListViewControllerTest : public AppsGridControllerTestHelper, virtual void SetUp() OVERRIDE { app_list_view_controller_.reset([[AppListViewController alloc] init]); + scoped_ptr<AppListTestViewDelegate> delegate(new AppListTestViewDelegate); + delegate->set_test_signin_delegate(this); + [app_list_view_controller_ + setDelegate:delegate.PassAs<app_list::AppListViewDelegate>()]; SetUpWithGridController([app_list_view_controller_ appsGridController]); [[test_window() contentView] addSubview:[app_list_view_controller_ view]]; } @@ -32,12 +36,13 @@ class AppListViewControllerTest : public AppsGridControllerTestHelper, AppsGridControllerTestHelper::TearDown(); } - virtual void ResetModel(scoped_ptr<AppListModel> new_model) OVERRIDE { + void ReplaceTestModel(int item_count) { + [app_list_view_controller_ + setDelegate:scoped_ptr<app_list::AppListViewDelegate>()]; scoped_ptr<AppListTestViewDelegate> delegate(new AppListTestViewDelegate); - delegate->set_test_signin_delegate(this); + delegate->ReplaceTestModel(item_count); [app_list_view_controller_ - setDelegate:delegate.PassAs<AppListViewDelegate>() - withTestModel:new_model.Pass()]; + setDelegate:delegate.PassAs<app_list::AppListViewDelegate>()]; } // SigninDelegate overrides: @@ -60,6 +65,15 @@ class AppListViewControllerTest : public AppsGridControllerTestHelper, return base::string16(); } + AppListTestViewDelegate* delegate() { + return static_cast<AppListTestViewDelegate*>( + [app_list_view_controller_ delegate]); + } + + AppListTestModel* model() { + return delegate()->GetTestModel(); + } + protected: base::scoped_nsobject<AppListViewController> app_list_view_controller_; @@ -122,9 +136,7 @@ TEST_F(AppListViewControllerTest, SignedIn) { // Test the view when signin is required. TEST_F(AppListViewControllerTest, NeedsSignin) { // Begin the test with a signed out app list. - scoped_ptr<AppListModel> new_model(new AppListModel); - new_model->SetSignedIn(false); - ResetModel(new_model.Pass()); + model()->SetSignedIn(false); EXPECT_EQ(2u, [[[app_list_view_controller_ view] subviews] count]); EXPECT_TRUE([[app_list_view_controller_ backgroundView] isHidden]); diff --git a/ui/app_list/cocoa/apps_grid_controller.h b/ui/app_list/cocoa/apps_grid_controller.h index b53c814..e565331 100644 --- a/ui/app_list/cocoa/apps_grid_controller.h +++ b/ui/app_list/cocoa/apps_grid_controller.h @@ -29,7 +29,6 @@ APP_LIST_EXPORT AppListPagerDelegate, NSCollectionViewDelegate> { @private - scoped_ptr<app_list::AppListModel> model_; app_list::AppListViewDelegate* delegate_; // Weak. Owned by view controller. scoped_ptr<app_list::AppsGridDelegateBridge> bridge_; @@ -66,8 +65,6 @@ APP_LIST_EXPORT - (app_list::AppListModel*)model; -- (void)setModel:(scoped_ptr<app_list::AppListModel>)newModel; - - (void)setDelegate:(app_list::AppListViewDelegate*)newDelegate; - (size_t)visiblePage; diff --git a/ui/app_list/cocoa/apps_grid_controller.mm b/ui/app_list/cocoa/apps_grid_controller.mm index d450ef4..dc5145c 100644 --- a/ui/app_list/cocoa/apps_grid_controller.mm +++ b/ui/app_list/cocoa/apps_grid_controller.mm @@ -164,7 +164,6 @@ class AppsGridDelegateBridge : public AppListItemListObserver { - (void)dealloc { [[NSNotificationCenter defaultCenter] removeObserver:self]; - [self setModel:scoped_ptr<app_list::AppListModel>()]; [super dealloc]; } @@ -181,45 +180,43 @@ class AppsGridDelegateBridge : public AppListItemListObserver { } - (app_list::AppListModel*)model { - return model_.get(); + return delegate_ ? delegate_->GetModel() : NULL; } -- (void)setModel:(scoped_ptr<app_list::AppListModel>)newModel { - if (model_) { - model_->item_list()->RemoveObserver(bridge_.get()); +- (void)setDelegate:(app_list::AppListViewDelegate*)newDelegate { + if (delegate_) { + app_list::AppListModel* oldModel = delegate_->GetModel(); + if (oldModel) + oldModel->item_list()->RemoveObserver(bridge_.get()); + } - // Since the model is about to be deleted, and the AppKit objects might be - // sitting in an NSAutoreleasePool, ensure there are no references to the - // model. - for (size_t i = 0; i < [items_ count]; ++i) - [[self itemAtIndex:i] setModel:NULL]; + // Since the old model may be getting deleted, and the AppKit objects might + // be sitting in an NSAutoreleasePool, ensure there are no references to + // the model. + for (size_t i = 0; i < [items_ count]; ++i) + [[self itemAtIndex:i] setModel:NULL]; - [items_ removeAllObjects]; - [self updatePages:0]; - [self scrollToPage:0]; - } + [items_ removeAllObjects]; + [self updatePages:0]; + [self scrollToPage:0]; + + delegate_ = newDelegate; + if (!delegate_) + return; - model_.reset(newModel.release()); - if (!model_) + app_list::AppListModel* newModel = delegate_->GetModel(); + if (!newModel) return; - model_->item_list()->AddObserver(bridge_.get()); - for (size_t i = 0; i < model_->item_list()->item_count(); ++i) { - app_list::AppListItemModel* itemModel = model_->item_list()->item_at(i); + newModel->item_list()->AddObserver(bridge_.get()); + for (size_t i = 0; i < newModel->item_list()->item_count(); ++i) { + app_list::AppListItemModel* itemModel = newModel->item_list()->item_at(i); [items_ insertObject:[NSValue valueWithPointer:itemModel] atIndex:i]; } [self updatePages:0]; } -- (void)setDelegate:(app_list::AppListViewDelegate*)newDelegate { - scoped_ptr<app_list::AppListModel> newModel(new app_list::AppListModel); - delegate_ = newDelegate; - if (delegate_) - delegate_->InitModel(newModel.get()); // Populates items. - [self setModel:newModel.Pass()]; -} - - (size_t)visiblePage { return visiblePage_; } @@ -524,9 +521,10 @@ class AppsGridDelegateBridge : public AppListItemListObserver { if (itemIndex == modelIndex) return; - model_->item_list()->RemoveObserver(bridge_.get()); - model_->item_list()->MoveItem(itemIndex, modelIndex); - model_->item_list()->AddObserver(bridge_.get()); + app_list::AppListItemList* itemList = [self model]->item_list(); + itemList->RemoveObserver(bridge_.get()); + itemList->MoveItem(itemIndex, modelIndex); + itemList->AddObserver(bridge_.get()); } - (AppsCollectionViewDragManager*)dragManager { diff --git a/ui/app_list/cocoa/apps_grid_controller_unittest.mm b/ui/app_list/cocoa/apps_grid_controller_unittest.mm index 5692574..b43ab44 100644 --- a/ui/app_list/cocoa/apps_grid_controller_unittest.mm +++ b/ui/app_list/cocoa/apps_grid_controller_unittest.mm @@ -113,6 +113,16 @@ class AppsGridControllerTest : public AppsGridControllerTestHelper { AppsGridControllerTestHelper::TearDown(); } + void ReplaceTestModel(int item_count) { + // Clear the delegate before reseting and destroying the model. + [owned_apps_grid_controller_ setDelegate:NULL]; + + owned_delegate_->ReplaceTestModel(item_count); + [owned_apps_grid_controller_ setDelegate:owned_delegate_.get()]; + } + + AppListTestModel* model() { return owned_delegate_->GetTestModel(); } + private: base::scoped_nsobject<AppsGridController> owned_apps_grid_controller_; scoped_ptr<AppListTestViewDelegate> owned_delegate_; diff --git a/ui/app_list/cocoa/test/apps_grid_controller_test_helper.h b/ui/app_list/cocoa/test/apps_grid_controller_test_helper.h index 1e2eb27..64c9e15 100644 --- a/ui/app_list/cocoa/test/apps_grid_controller_test_helper.h +++ b/ui/app_list/cocoa/test/apps_grid_controller_test_helper.h @@ -13,12 +13,8 @@ namespace app_list { -class AppListModel; - namespace test { -class AppListTestModel; - class AppsGridControllerTestHelper : public ui::CocoaTest { public: static const size_t kItemsPerPage; @@ -38,9 +34,6 @@ class AppsGridControllerTestHelper : public ui::CocoaTest { void SimulateMouseEnterItemAt(size_t index); void SimulateMouseExitItemAt(size_t index); - // Do a bulk replacement of the items in the grid. - void ReplaceTestModel(int item_count); - // Get a string representation of the items as they are currently ordered in // the view. Each page will start and end with a | character. std::string GetViewContent() const; @@ -57,10 +50,6 @@ class AppsGridControllerTestHelper : public ui::CocoaTest { NSCollectionView* GetPageAt(size_t index); NSView* GetSelectedView(); - AppListTestModel* model(); - - virtual void ResetModel(scoped_ptr<AppListModel> model); - AppsGridController* apps_grid_controller_; private: diff --git a/ui/app_list/cocoa/test/apps_grid_controller_test_helper.mm b/ui/app_list/cocoa/test/apps_grid_controller_test_helper.mm index 9b971a82..f24b7d4 100644 --- a/ui/app_list/cocoa/test/apps_grid_controller_test_helper.mm +++ b/ui/app_list/cocoa/test/apps_grid_controller_test_helper.mm @@ -9,7 +9,6 @@ #include "ui/app_list/app_list_item_model.h" #import "ui/app_list/cocoa/apps_grid_controller.h" #import "ui/app_list/cocoa/apps_grid_view_item.h" -#include "ui/app_list/test/app_list_test_model.h" #import "ui/base/test/cocoa_test_event_utils.h" namespace app_list { @@ -28,7 +27,6 @@ void AppsGridControllerTestHelper::SetUpWithGridController( AppsGridController* grid_controller) { ui::CocoaTest::SetUp(); apps_grid_controller_ = grid_controller; - ReplaceTestModel(0); } void AppsGridControllerTestHelper::SimulateClick(NSView* view) { @@ -52,17 +50,6 @@ void AppsGridControllerTestHelper::SimulateMouseExitItemAt(size_t index) { cocoa_test_event_utils::EnterExitEventWithType(NSMouseExited)]; } -void AppsGridControllerTestHelper::ReplaceTestModel(int item_count) { - scoped_ptr<AppListTestModel> new_model(new AppListTestModel); - new_model->PopulateApps(item_count); - ResetModel(new_model.PassAs<AppListModel>()); -} - -void AppsGridControllerTestHelper::ResetModel( - scoped_ptr<AppListModel> new_model) { - [apps_grid_controller_ setModel:new_model.Pass()]; -} - std::string AppsGridControllerTestHelper::GetViewContent() const { std::string s; for (size_t page_index = 0; page_index < [apps_grid_controller_ pageCount]; @@ -128,9 +115,5 @@ NSView* AppsGridControllerTestHelper::GetSelectedView() { return GetItemViewAt([apps_grid_controller_ selectedItemIndex]); } -AppListTestModel* AppsGridControllerTestHelper::model() { - return static_cast<AppListTestModel*>([apps_grid_controller_ model]); -} - } // namespace test } // namespace app_list diff --git a/ui/app_list/test/app_list_test_model.cc b/ui/app_list/test/app_list_test_model.cc index 4fd1acc..c9ca1c1 100644 --- a/ui/app_list/test/app_list_test_model.cc +++ b/ui/app_list/test/app_list_test_model.cc @@ -45,7 +45,7 @@ std::string AppListTestModel::GetItemName(int id) { } void AppListTestModel::PopulateApps(int n) { - int start_index = item_list()->item_count(); + int start_index = static_cast<int>(item_list()->item_count()); for (int i = 0; i < n; ++i) CreateAndAddItem(GetItemName(start_index + i)); } diff --git a/ui/app_list/test/app_list_test_view_delegate.cc b/ui/app_list/test/app_list_test_view_delegate.cc index d04ae4e..aba8d93 100644 --- a/ui/app_list/test/app_list_test_view_delegate.cc +++ b/ui/app_list/test/app_list_test_view_delegate.cc @@ -6,6 +6,8 @@ #include "base/callback.h" #include "base/files/file_path.h" +#include "ui/app_list/app_list_model.h" +#include "ui/app_list/test/app_list_test_model.h" #include "ui/gfx/image/image_skia.h" namespace app_list { @@ -13,7 +15,8 @@ namespace test { AppListTestViewDelegate::AppListTestViewDelegate() : dismiss_count_(0), - test_signin_delegate_(NULL) { + test_signin_delegate_(NULL), + model_(new AppListTestModel) { } AppListTestViewDelegate::~AppListTestViewDelegate() {} @@ -22,6 +25,10 @@ bool AppListTestViewDelegate::ForceNativeDesktop() const { return false; } +AppListModel* AppListTestViewDelegate::GetModel() { + return model_.get(); +} + SigninDelegate* AppListTestViewDelegate::GetSigninDelegate() { return test_signin_delegate_; } @@ -48,5 +55,10 @@ const AppListViewDelegate::Users& AppListTestViewDelegate::GetUsers() const { return users_; } +void AppListTestViewDelegate::ReplaceTestModel(int item_count) { + model_.reset(new AppListTestModel); + model_->PopulateApps(item_count); +} + } // namespace test } // namespace app_list diff --git a/ui/app_list/test/app_list_test_view_delegate.h b/ui/app_list/test/app_list_test_view_delegate.h index 9f1bacd..bbe3447 100644 --- a/ui/app_list/test/app_list_test_view_delegate.h +++ b/ui/app_list/test/app_list_test_view_delegate.h @@ -7,13 +7,16 @@ #include "base/callback_forward.h" #include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" #include "ui/app_list/app_list_view_delegate.h" namespace app_list { namespace test { +class AppListTestModel; + // A concrete AppListViewDelegate for unit tests. -class AppListTestViewDelegate : public AppListViewDelegate { +class AppListTestViewDelegate : public AppListViewDelegate { public: AppListTestViewDelegate(); virtual ~AppListTestViewDelegate(); @@ -30,7 +33,7 @@ class AppListTestViewDelegate : public AppListViewDelegate { // AppListViewDelegate overrides: virtual bool ForceNativeDesktop() const OVERRIDE; virtual void SetProfileByPath(const base::FilePath& profile_path) OVERRIDE {} - virtual void InitModel(AppListModel* model) OVERRIDE {} + virtual AppListModel* GetModel() OVERRIDE; virtual SigninDelegate* GetSigninDelegate() OVERRIDE; virtual void GetShortcutPathForApp( const std::string& app_id, @@ -54,11 +57,16 @@ class AppListTestViewDelegate : public AppListViewDelegate { virtual content::WebContents* GetStartPageContents() OVERRIDE; virtual const Users& GetUsers() const OVERRIDE; + // Do a bulk replacement of the items in the model. + void ReplaceTestModel(int item_count); + + AppListTestModel* GetTestModel() { return model_.get(); } + private: int dismiss_count_; Users users_; SigninDelegate* test_signin_delegate_; // Weak. Owned by test. - + scoped_ptr<AppListTestModel> model_; }; } // namespace test diff --git a/ui/app_list/views/app_list_main_view.cc b/ui/app_list/views/app_list_main_view.cc index dd1a30c..a4cd7a9 100644 --- a/ui/app_list/views/app_list_main_view.cc +++ b/ui/app_list/views/app_list_main_view.cc @@ -78,11 +78,10 @@ class AppListMainView::IconLoader : public AppListItemModelObserver { // AppListMainView: AppListMainView::AppListMainView(AppListViewDelegate* delegate, - AppListModel* model, PaginationModel* pagination_model, gfx::NativeView parent) : delegate_(delegate), - model_(model), + model_(delegate->GetModel()), search_box_view_(NULL), contents_view_(NULL), weak_ptr_factory_(this) { @@ -201,11 +200,7 @@ void AppListMainView::ActivateApp(AppListItemModel* item, int event_flags) { void AppListMainView::GetShortcutPathForApp( const std::string& app_id, const base::Callback<void(const base::FilePath&)>& callback) { - if (delegate_) { - delegate_->GetShortcutPathForApp(app_id, callback); - return; - } - callback.Run(base::FilePath()); + delegate_->GetShortcutPathForApp(app_id, callback); } void AppListMainView::QueryChanged(SearchBoxView* sender) { @@ -214,24 +209,20 @@ void AppListMainView::QueryChanged(SearchBoxView* sender) { bool should_show_search = !query.empty(); contents_view_->ShowSearchResults(should_show_search); - if (delegate_) { - if (should_show_search) - delegate_->StartSearch(); - else - delegate_->StopSearch(); - } + if (should_show_search) + delegate_->StartSearch(); + else + delegate_->StopSearch(); } void AppListMainView::OpenResult(SearchResult* result, int event_flags) { - if (delegate_) - delegate_->OpenSearchResult(result, event_flags); + delegate_->OpenSearchResult(result, event_flags); } void AppListMainView::InvokeResultAction(SearchResult* result, int action_index, int event_flags) { - if (delegate_) - delegate_->InvokeSearchResultAction(result, action_index, event_flags); + delegate_->InvokeSearchResultAction(result, action_index, event_flags); } void AppListMainView::OnResultInstalled(SearchResult* result) { diff --git a/ui/app_list/views/app_list_main_view.h b/ui/app_list/views/app_list_main_view.h index b9a5cdc..c0de4a0 100644 --- a/ui/app_list/views/app_list_main_view.h +++ b/ui/app_list/views/app_list_main_view.h @@ -38,7 +38,6 @@ class AppListMainView : public views::View, public: // Takes ownership of |delegate|. explicit AppListMainView(AppListViewDelegate* delegate, - AppListModel* model, PaginationModel* pagination_model, gfx::NativeView parent); virtual ~AppListMainView(); @@ -90,8 +89,8 @@ class AppListMainView : public views::View, virtual void OnResultInstalled(SearchResult* result) OVERRIDE; virtual void OnResultUninstalled(SearchResult* result) OVERRIDE; - AppListViewDelegate* delegate_; - AppListModel* model_; + AppListViewDelegate* delegate_; // Owned by parent (AppListView) + AppListModel* model_; // Unowned; ownership is handled by |delegate_|. SearchBoxView* search_box_view_; // Owned by views hierarchy. ContentsView* contents_view_; // Owned by views hierarchy. diff --git a/ui/app_list/views/app_list_view.cc b/ui/app_list/views/app_list_view.cc index 3d7ece3..d6e9dfa 100644 --- a/ui/app_list/views/app_list_view.cc +++ b/ui/app_list/views/app_list_view.cc @@ -67,18 +67,16 @@ bool SupportsShadow() { // AppListView: AppListView::AppListView(AppListViewDelegate* delegate) - : model_(new AppListModel), - delegate_(delegate), + : delegate_(delegate), app_list_main_view_(NULL), signin_view_(NULL) { - if (delegate_) - delegate_->InitModel(model_.get()); - model_->AddObserver(this); + CHECK(delegate); + delegate_->GetModel()->AddObserver(this); } AppListView::~AppListView() { - model_->RemoveObserver(this); - // Models are going away, ensure their references are cleared. + delegate_->GetModel()->RemoveObserver(this); + // Remove child views first to ensure no remaining dependencies on delegate_. RemoveAllChildViews(true); } @@ -128,10 +126,7 @@ void AppListView::ShowWhenReady() { void AppListView::Close() { app_list_main_view_->Close(); - if (delegate_) - delegate_->Dismiss(); - else - GetWidget()->Close(); + delegate_->Dismiss(); } void AppListView::UpdateBounds() { @@ -159,8 +154,9 @@ void AppListView::Prerender() { } void AppListView::OnSigninStatusChanged() { - signin_view_->SetVisible(!model_->signed_in()); - app_list_main_view_->SetVisible(model_->signed_in()); + AppListModel* model = delegate_->GetModel(); + signin_view_->SetVisible(!model->signed_in()); + app_list_main_view_->SetVisible(model->signed_in()); app_list_main_view_->search_box_view()->InvalidateMenu(); } @@ -199,7 +195,6 @@ void AppListView::InitAsBubbleInternal(gfx::NativeView parent, bool border_accepts_events, const gfx::Vector2d& anchor_offset) { app_list_main_view_ = new AppListMainView(delegate_.get(), - model_.get(), pagination_model, parent); AddChildView(app_list_main_view_); @@ -209,10 +204,9 @@ void AppListView::InitAsBubbleInternal(gfx::NativeView parent, app_list_main_view_->layer()->SetMasksToBounds(true); #endif - signin_view_ = new SigninView( - delegate_ ? delegate_->GetSigninDelegate() - : NULL, - app_list_main_view_->GetPreferredSize().width()); + signin_view_ = + new SigninView(delegate_->GetSigninDelegate(), + app_list_main_view_->GetPreferredSize().width()); AddChildView(signin_view_); OnSigninStatusChanged(); diff --git a/ui/app_list/views/app_list_view.h b/ui/app_list/views/app_list_view.h index 46c3784..2d273ce 100644 --- a/ui/app_list/views/app_list_view.h +++ b/ui/app_list/views/app_list_view.h @@ -102,7 +102,6 @@ class APP_LIST_EXPORT AppListView : public views::BubbleDelegateView, HWND GetHWND() const; #endif - AppListModel* model() { return model_.get(); } AppListMainView* app_list_main_view() { return app_list_main_view_; } private: @@ -139,10 +138,9 @@ class APP_LIST_EXPORT AppListView : public views::BubbleDelegateView, SigninDelegate* GetSigninDelegate(); - scoped_ptr<AppListModel> model_; scoped_ptr<AppListViewDelegate> delegate_; - AppListMainView* app_list_main_view_; + AppListMainView* app_list_main_view_; SigninView* signin_view_; ObserverList<Observer> observers_; diff --git a/ui/ui_unittests.gyp b/ui/ui_unittests.gyp index d18cd41..3d58e92 100644 --- a/ui/ui_unittests.gyp +++ b/ui/ui_unittests.gyp @@ -16,6 +16,10 @@ 'gfx/gfx.gyp:gfx', ], 'sources': [ + 'app_list/test/app_list_test_model.cc', + 'app_list/test/app_list_test_model.h', + 'app_list/test/app_list_test_view_delegate.cc', + 'app_list/test/app_list_test_view_delegate.h', 'base/test/cocoa_test_event_utils.h', 'base/test/cocoa_test_event_utils.mm', 'base/test/ui_cocoa_test_helper.h', |