summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc18
-rw-r--r--chrome/browser/ui/app_list/app_list_service_mac.h5
-rw-r--r--chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm43
-rw-r--r--chrome/browser/ui/app_list/app_list_view_delegate.cc3
-rw-r--r--ui/app_list/app_list_item_list.h2
-rw-r--r--ui/app_list/app_list_model.h2
-rw-r--r--ui/app_list/app_list_view_delegate_observer.h5
-rw-r--r--ui/app_list/cocoa/app_list_view_controller.mm5
-rw-r--r--ui/app_list/views/app_list_view.cc5
-rw-r--r--ui/app_list/views/app_list_view.h1
10 files changed, 77 insertions, 12 deletions
diff --git a/chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc b/chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc
index 4ee32b3..e354cb9 100644
--- a/chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc
+++ b/chrome/browser/ui/app_list/app_list_service_interactive_uitest.cc
@@ -46,24 +46,30 @@ class AppListServiceInteractiveTest : public InProcessBrowserTest {
// ChromeOS does not support ShowForProfile(), or profile switching within the
// app list. Profile switching on CrOS goes through a different code path.
-// The tests are flaky on Mac, so disable there too. See http://crbug.com/415264
-#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
+#if defined(OS_CHROMEOS)
#define MAYBE_ShowAndDismiss DISABLED_ShowAndDismiss
#define MAYBE_SwitchAppListProfiles DISABLED_SwitchAppListProfiles
-#define MAYBE_SwitchAppListProfilesDuringSearch \
- DISABLED_SwitchAppListProfilesDuringSearch
#define MAYBE_ShowAppListNonDefaultProfile \
DISABLED_ShowAppListNonDefaultProfile
#define MAYBE_DeleteShowingAppList DISABLED_DeleteShowingAppList
#else
#define MAYBE_ShowAndDismiss ShowAndDismiss
#define MAYBE_SwitchAppListProfiles SwitchAppListProfiles
-#define MAYBE_SwitchAppListProfilesDuringSearch \
- SwitchAppListProfilesDuringSearch
#define MAYBE_ShowAppListNonDefaultProfile ShowAppListNonDefaultProfile
#define MAYBE_DeleteShowingAppList DeleteShowingAppList
#endif
+// SwitchAppListProfilesDuringSearch disabled on ChromeOS for reasons above.
+// Disabled on Mac due to an AppKit bug which makes it flaky in rare cases.
+// http://crbug.com/417148.
+#if defined(OS_CHROMEOS) || defined(OS_MACOSX)
+#define MAYBE_SwitchAppListProfilesDuringSearch \
+ DISABLED_SwitchAppListProfilesDuringSearch
+#else
+#define MAYBE_SwitchAppListProfilesDuringSearch \
+ SwitchAppListProfilesDuringSearch
+#endif
+
// Show the app list, then dismiss it.
IN_PROC_BROWSER_TEST_F(AppListServiceInteractiveTest, MAYBE_ShowAndDismiss) {
AppListService* service = test::GetAppListService();
diff --git a/chrome/browser/ui/app_list/app_list_service_mac.h b/chrome/browser/ui/app_list/app_list_service_mac.h
index b9aa2c8..e18b744 100644
--- a/chrome/browser/ui/app_list/app_list_service_mac.h
+++ b/chrome/browser/ui/app_list/app_list_service_mac.h
@@ -23,6 +23,10 @@ class Display;
class Point;
}
+namespace test {
+class AppListServiceMacTestApi;
+}
+
// AppListServiceMac manages global resources needed for the app list to
// operate, and controls when the app list is opened and closed.
class AppListServiceMac : public AppListServiceImpl,
@@ -80,6 +84,7 @@ class AppListServiceMac : public AppListServiceImpl,
virtual void OnShimQuit(apps::AppShimHandler::Host* host) OVERRIDE;
private:
+ friend class test::AppListServiceMacTestApi;
friend struct DefaultSingletonTraits<AppListServiceMac>;
AppListServiceMac();
diff --git a/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm b/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm
index 959271c..17f5b4e 100644
--- a/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm
+++ b/chrome/browser/ui/app_list/app_list_service_mac_interactive_uitest.mm
@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "chrome/browser/ui/app_list/app_list_service_mac.h"
+#import "chrome/browser/ui/app_list/app_list_service_mac.h"
#include <vector>
@@ -12,9 +12,21 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/mac/app_mode_common.h"
#include "chrome/test/base/in_process_browser_test.h"
+#import "ui/app_list/cocoa/app_list_window_controller.h"
using apps::AppShimHandler;
+namespace test {
+
+class AppListServiceMacTestApi {
+ public:
+ static AppListWindowController* window_controller() {
+ return AppListServiceMac::GetInstance()->window_controller_;
+ }
+};
+
+} // namespace test
+
namespace {
// Browser test for mac-specific AppListService functionality.
@@ -32,6 +44,23 @@ class AppListServiceMacInteractiveTest : public InProcessBrowserTest,
std::vector<base::FilePath>());
}
+ AppListViewController* GetViewController() {
+ return [test::AppListServiceMacTestApi::window_controller()
+ appListViewController];
+ }
+
+ // testing::Test overrides:
+ virtual void TearDown() OVERRIDE {
+ // At tear-down, NOTIFICATION_APP_TERMINATING should have been sent for the
+ // browser shutdown. References to browser-owned objects must be removed
+ // from the app list UI.
+ AppListViewController* view_controller = GetViewController();
+ // Note this first check will fail if the test doesn't ever show the
+ // app list, but currently all tests in this file do.
+ EXPECT_TRUE(view_controller);
+ EXPECT_FALSE([view_controller delegate]);
+ }
+
// AppShimHandler::Host overrides:
virtual void OnAppLaunchComplete(apps::AppShimLaunchResult result) OVERRIDE {
// AppList shims are always given APP_SHIM_LAUNCH_DUPLICATE_HOST, indicating
@@ -61,9 +90,7 @@ class AppListServiceMacInteractiveTest : public InProcessBrowserTest,
} // namespace
-// Flaky on Mac. See https://crbug.com/415264
-IN_PROC_BROWSER_TEST_F(AppListServiceMacInteractiveTest,
- DISABLED_ShowAppListUsingShim) {
+IN_PROC_BROWSER_TEST_F(AppListServiceMacInteractiveTest, ShowAppListUsingShim) {
// Check that AppListService has registered as a shim handler for "app_list".
EXPECT_TRUE(AppShimHandler::GetForAppMode(app_mode::kAppListModeId));
@@ -71,6 +98,9 @@ IN_PROC_BROWSER_TEST_F(AppListServiceMacInteractiveTest,
AppListService::Get(chrome::HOST_DESKTOP_TYPE_NATIVE);
EXPECT_FALSE(service->IsAppListVisible());
+ // Creation should be lazy.
+ EXPECT_FALSE(GetViewController());
+
// With no saved profile, the default profile should be chosen and saved.
service->Show();
EXPECT_EQ(browser()->profile(), service->GetCurrentAppListProfile());
@@ -96,4 +126,9 @@ IN_PROC_BROWSER_TEST_F(AppListServiceMacInteractiveTest,
EXPECT_EQ(3, launch_count_);
service->DismissAppList();
EXPECT_FALSE(service->IsAppListVisible());
+
+ // View sticks around until shutdown.
+ AppListViewController* view_controller = GetViewController();
+ EXPECT_TRUE(view_controller);
+ EXPECT_TRUE([view_controller delegate]);
}
diff --git a/chrome/browser/ui/app_list/app_list_view_delegate.cc b/chrome/browser/ui/app_list/app_list_view_delegate.cc
index af012b2..ab45f72 100644
--- a/chrome/browser/ui/app_list/app_list_view_delegate.cc
+++ b/chrome/browser/ui/app_list/app_list_view_delegate.cc
@@ -646,6 +646,9 @@ void AppListViewDelegate::Observe(int type,
const content::NotificationDetails& details) {
switch (type) {
case chrome::NOTIFICATION_APP_TERMINATING:
+ FOR_EACH_OBSERVER(
+ app_list::AppListViewDelegateObserver, observers_, OnShutdown());
+
SetProfile(NULL); // Ensures launcher page web contents are torn down.
// SigninManagerFactory is not a leaky singleton (unlike this class), and
diff --git a/ui/app_list/app_list_item_list.h b/ui/app_list/app_list_item_list.h
index 66b6553..2e67f1c 100644
--- a/ui/app_list/app_list_item_list.h
+++ b/ui/app_list/app_list_item_list.h
@@ -102,7 +102,7 @@ class APP_LIST_EXPORT AppListItemList {
void FixItemPosition(size_t index);
ScopedVector<AppListItem> app_list_items_;
- ObserverList<AppListItemListObserver> observers_;
+ ObserverList<AppListItemListObserver, true> observers_;
DISALLOW_COPY_AND_ASSIGN(AppListItemList);
};
diff --git a/ui/app_list/app_list_model.h b/ui/app_list/app_list_model.h
index 20e6728..07ab33c 100644
--- a/ui/app_list/app_list_model.h
+++ b/ui/app_list/app_list_model.h
@@ -169,7 +169,7 @@ class APP_LIST_EXPORT AppListModel : public AppListItemListObserver {
scoped_ptr<SearchResults> results_;
Status status_;
- ObserverList<AppListModelObserver> observers_;
+ ObserverList<AppListModelObserver, true> observers_;
bool folders_enabled_;
DISALLOW_COPY_AND_ASSIGN(AppListModel);
diff --git a/ui/app_list/app_list_view_delegate_observer.h b/ui/app_list/app_list_view_delegate_observer.h
index a612334..abc4fe4 100644
--- a/ui/app_list/app_list_view_delegate_observer.h
+++ b/ui/app_list/app_list_view_delegate_observer.h
@@ -15,6 +15,11 @@ class APP_LIST_EXPORT AppListViewDelegateObserver {
// profile changes its signin status.
virtual void OnProfilesChanged() = 0;
+ // Invoked on Chrome shutdown. This is only needed on Mac, since reference-
+ // counting in Objective-C means that simply closing the window isn't enough
+ // to guarantee references to Chrome objects are gone.
+ virtual void OnShutdown() = 0;
+
protected:
virtual ~AppListViewDelegateObserver() {}
};
diff --git a/ui/app_list/cocoa/app_list_view_controller.mm b/ui/app_list/cocoa/app_list_view_controller.mm
index 0ca22f4..b831f9a 100644
--- a/ui/app_list/cocoa/app_list_view_controller.mm
+++ b/ui/app_list/cocoa/app_list_view_controller.mm
@@ -88,6 +88,7 @@ class AppListModelObserverBridge : public AppListViewDelegateObserver {
private:
// Overridden from app_list::AppListViewDelegateObserver:
virtual void OnProfilesChanged() OVERRIDE;
+ virtual void OnShutdown() OVERRIDE;
AppListViewController* parent_; // Weak. Owns us.
@@ -108,6 +109,10 @@ void AppListModelObserverBridge::OnProfilesChanged() {
[parent_ onProfilesChanged];
}
+void AppListModelObserverBridge::OnShutdown() {
+ [parent_ setDelegate:nil];
+}
+
} // namespace app_list
@implementation AppListViewController
diff --git a/ui/app_list/views/app_list_view.cc b/ui/app_list/views/app_list_view.cc
index 4d7df42..2a01e82 100644
--- a/ui/app_list/views/app_list_view.cc
+++ b/ui/app_list/views/app_list_view.cc
@@ -282,6 +282,11 @@ void AppListView::OnProfilesChanged() {
app_list_main_view_->search_box_view()->InvalidateMenu();
}
+void AppListView::OnShutdown() {
+ // Nothing to do on views - the widget will soon be closed, which will tear
+ // everything down.
+}
+
void AppListView::SetProfileByPath(const base::FilePath& profile_path) {
delegate_->SetProfileByPath(profile_path);
app_list_main_view_->ModelChanged();
diff --git a/ui/app_list/views/app_list_view.h b/ui/app_list/views/app_list_view.h
index d1ee338..0ee715a 100644
--- a/ui/app_list/views/app_list_view.h
+++ b/ui/app_list/views/app_list_view.h
@@ -100,6 +100,7 @@ class APP_LIST_EXPORT AppListView : public views::BubbleDelegateView,
// Overridden from AppListViewDelegateObserver:
virtual void OnProfilesChanged() OVERRIDE;
+ virtual void OnShutdown() OVERRIDE;
void Prerender();
generated by cgit v1.1 at 2025-02-13 04:46:04 +0000