Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixing problem with a tabless browser windows showing up after a task…

… manager was used

I traced it down to the creation of the task manager which grabs an existing (or new) browser. It requires the browser only for two things: determining the system it is running in and for giving the created web dialog a context (a window which is used (by Aura only) to position the window within the hierarchy.

As I can see it there are no other dependencies to the browser and it can be disposed of after the task manager was created (and there are no tabs in the tabbed browser).

BUG=176670
TEST=visual


Review URL: https://chromiumcodereview.appspot.com/12712018

git-svn-id: http://src.chromium.org/svn/trunk/src/chrome/browser@189579 4ff67af0-8c30-449e-8e8b-ad334ec8d88c
  • Loading branch information...
commit a20cce21acdaf38ebda1effeac2b1040b9f6fab9 1 parent 3d6dcd7
authored March 21, 2013
3  extensions/api/processes/processes_apitest.cc
@@ -8,6 +8,7 @@
8 8
 #include "chrome/browser/task_manager/task_manager.h"
9 9
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
10 10
 #include "chrome/browser/ui/browser.h"
  11
+#include "chrome/browser/ui/browser_dialogs.h"
11 12
 #include "chrome/browser/ui/browser_window.h"
12 13
 #include "chrome/common/chrome_switches.h"
13 14
 
@@ -49,7 +50,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ProcessesVsTaskManager) {
49 50
   EXPECT_EQ(TaskManagerModel::TASK_PENDING, model->update_state_);
50 51
 
51 52
   // Now show the task manager and wait for it to be ready
52  
-  TaskManagerBrowserTestUtil::ShowTaskManagerAndWaitForReady(browser());
  53
+  chrome::ShowTaskManager(browser(), false);
53 54
 
54 55
   EXPECT_EQ(2, model->update_requests_);
55 56
   EXPECT_EQ(TaskManagerModel::TASK_PENDING, model->update_state_);
2  prerender/prerender_browsertest.cc
@@ -1813,7 +1813,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
1813 1813
 // See crbug.com/131836.
1814 1814
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, DISABLED_PrerenderTaskManager) {
1815 1815
   // Show the task manager. This populates the model.
1816  
-  current_browser()->window()->ShowTaskManager();
  1816
+  chrome::OpenTaskManager(current_browser(), false);
1817 1817
   // Wait for the model of task manager to start.
1818 1818
   TaskManagerBrowserTestUtil::WaitForWebResourceChange(1);
1819 1819
 
3  task_manager/task_manager_browsertest.cc
@@ -22,6 +22,7 @@
22 22
 #include "chrome/browser/profiles/profile.h"
23 23
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
24 24
 #include "chrome/browser/ui/browser.h"
  25
+#include "chrome/browser/ui/browser_dialogs.h"
25 26
 #include "chrome/browser/ui/browser_navigator.h"
26 27
 #include "chrome/browser/ui/browser_window.h"
27 28
 #include "chrome/browser/ui/panels/panel.h"
@@ -79,7 +80,7 @@ class TaskManagerBrowserTest : public ExtensionBrowserTest {
79 80
 
80 81
     // Show the task manager. This populates the model, and helps with debugging
81 82
     // (you see the task manager).
82  
-    browser()->window()->ShowTaskManager();
  83
+    chrome::ShowTaskManager(browser(), false);
83 84
 
84 85
     // New Tab Page.
85 86
     TaskManagerBrowserTestUtil::WaitForWebResourceChange(1);
8  task_manager/task_manager_browsertest_util.cc
@@ -10,6 +10,7 @@
10 10
 #include "chrome/browser/task_manager/task_manager.h"
11 11
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
12 12
 #include "chrome/browser/ui/browser.h"
  13
+#include "chrome/browser/ui/browser_dialogs.h"
13 14
 #include "chrome/browser/ui/browser_window.h"
14 15
 #include "chrome/common/chrome_notification_types.h"
15 16
 #include "chrome/test/base/ui_test_utils.h"
@@ -92,10 +93,3 @@ void TaskManagerBrowserTestUtil::WaitForWebResourceChange(int target_count) {
92 93
   content::RunMessageLoop();
93 94
   model->RemoveObserver(&observer);
94 95
 }
95  
-
96  
-// static
97  
-void TaskManagerBrowserTestUtil::ShowTaskManagerAndWaitForReady(
98  
-    Browser* browser) {
99  
-  browser->window()->ShowTaskManager();
100  
-}
101  
-
1  task_manager/task_manager_browsertest_util.h
@@ -11,7 +11,6 @@ class TaskManagerBrowserTestUtil {
11 11
  public:
12 12
   static void WaitForWebResourceChange(int target_count);
13 13
 
14  
-  static void ShowTaskManagerAndWaitForReady(Browser* browser);
15 14
 };
16 15
 
17 16
 #endif  // CHROME_BROWSER_TASK_MANAGER_TASK_MANAGER_BROWSERTEST_UTIL_H_
3  task_manager/task_manager_notification_browsertest.cc
@@ -13,6 +13,7 @@
13 13
 #include "chrome/browser/notifications/notification_ui_manager.h"
14 14
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
15 15
 #include "chrome/browser/ui/browser.h"
  16
+#include "chrome/browser/ui/browser_dialogs.h"
16 17
 #include "chrome/browser/ui/browser_window.h"
17 18
 #include "chrome/test/base/in_process_browser_test.h"
18 19
 #include "chrome/test/base/ui_test_utils.h"
@@ -50,7 +51,7 @@ IN_PROC_BROWSER_TEST_F(TaskManagerNotificationBrowserTest,
50 51
   EXPECT_EQ(0, model()->ResourceCount());
51 52
 
52 53
   // Show the task manager.
53  
-  browser()->window()->ShowTaskManager();
  54
+  chrome::ShowTaskManager(browser(), false);
54 55
   // Expect to see the browser and the New Tab Page renderer.
55 56
   TaskManagerBrowserTestUtil::WaitForWebResourceChange(1);
56 57
 
5  ui/ash/chrome_shell_delegate.cc
@@ -165,10 +165,7 @@ bool ChromeShellDelegate::RotatePaneFocus(ash::Shell::Direction direction) {
165 165
 }
166 166
 
167 167
 void ChromeShellDelegate::ShowTaskManager() {
168  
-  Browser* browser = chrome::FindOrCreateTabbedBrowser(
169  
-      ProfileManager::GetDefaultProfileOrOffTheRecord(),
170  
-      chrome::HOST_DESKTOP_TYPE_ASH);
171  
-  chrome::OpenTaskManager(browser, false);
  168
+  chrome::OpenTaskManager(NULL, false);
172 169
 }
173 170
 
174 171
 content::BrowserContext* ChromeShellDelegate::GetCurrentBrowserContext() {
6  ui/browser_commands.cc
@@ -38,6 +38,7 @@
38 38
 #include "chrome/browser/ui/bookmarks/bookmark_utils.h"
39 39
 #include "chrome/browser/ui/browser.h"
40 40
 #include "chrome/browser/ui/browser_command_controller.h"
  41
+#include "chrome/browser/ui/browser_dialogs.h"
41 42
 #include "chrome/browser/ui/browser_finder.h"
42 43
 #include "chrome/browser/ui/browser_instant_controller.h"
43 44
 #include "chrome/browser/ui/browser_tab_restore_service_delegate.h"
@@ -886,10 +887,7 @@ bool CanOpenTaskManager() {
886 887
 
887 888
 void OpenTaskManager(Browser* browser, bool highlight_background_resources) {
888 889
   content::RecordAction(UserMetricsAction("TaskManager"));
889  
-  if (highlight_background_resources)
890  
-    browser->window()->ShowBackgroundPages();
891  
-  else
892  
-    browser->window()->ShowTaskManager();
  890
+  chrome::ShowTaskManager(browser, highlight_background_resources);
893 891
 }
894 892
 
895 893
 void OpenFeedbackDialog(Browser* browser) {
4  ui/browser_dialogs.h
@@ -63,6 +63,10 @@ void ShowExtensionInstalledBubble(const extensions::Extension* extension,
63 63
 void ShowHungRendererDialog(content::WebContents* contents);
64 64
 void HideHungRendererDialog(content::WebContents* contents);
65 65
 
  66
+// Shows the Task Manager. If |highlight_background_resources| is set, the
  67
+// backgroundpages will be shown. |browser| can be NULL when called from ASH.
  68
+void ShowTaskManager(Browser* browser, bool highlight_background_resources);
  69
+
66 70
 #if !defined(OS_MACOSX)
67 71
 // Shows the create web app shortcut dialog box.
68 72
 void ShowCreateWebAppShortcutsDialog(gfx::NativeWindow parent_window,
6  ui/browser_window.h
@@ -204,12 +204,6 @@ class BrowserWindow : public BaseWindow {
204 204
   // Shows the Update Recommended dialog box.
205 205
   virtual void ShowUpdateChromeDialog() = 0;
206 206
 
207  
-  // Shows the Task manager.
208  
-  virtual void ShowTaskManager() = 0;
209  
-
210  
-  // Shows task information related to background pages.
211  
-  virtual void ShowBackgroundPages() = 0;
212  
-
213 207
   // Shows the Bookmark bubble. |url| is the URL being bookmarked,
214 208
   // |already_bookmarked| is true if the url is already bookmarked.
215 209
   virtual void ShowBookmarkBubble(const GURL& url, bool already_bookmarked) = 0;
2  ui/cocoa/browser_window_cocoa.h
@@ -94,8 +94,6 @@ class BrowserWindowCocoa :
94 94
                                         Profile* profile) OVERRIDE;
95 95
   virtual void ToggleBookmarkBar() OVERRIDE;
96 96
   virtual void ShowUpdateChromeDialog() OVERRIDE;
97  
-  virtual void ShowTaskManager() OVERRIDE;
98  
-  virtual void ShowBackgroundPages() OVERRIDE;
99 97
   virtual void ShowBookmarkBubble(const GURL& url,
100 98
                                   bool already_bookmarked) OVERRIDE;
101 99
   virtual void ShowChromeToMobileBubble() OVERRIDE;
8  ui/cocoa/browser_window_cocoa.mm
@@ -470,14 +470,6 @@ void CreateShortcuts(const ShellIntegration::ShortcutInfo& shortcut_info) {
470 470
   restart_browser::RequestRestart(window());
471 471
 }
472 472
 
473  
-void BrowserWindowCocoa::ShowTaskManager() {
474  
-  TaskManagerMac::Show(false);
475  
-}
476  
-
477  
-void BrowserWindowCocoa::ShowBackgroundPages() {
478  
-  TaskManagerMac::Show(true);
479  
-}
480  
-
481 473
 void BrowserWindowCocoa::ShowBookmarkBubble(const GURL& url,
482 474
                                             bool already_bookmarked) {
483 475
   [controller_ showBookmarkBubbleForURL:url
10  ui/cocoa/task_manager_mac.mm
@@ -585,3 +585,13 @@ - (void)           tableView:(NSTableView*)tableView
585 585
                                  highlight_background_resources);
586 586
   instance_->model_->StartUpdating();
587 587
 }
  588
+
  589
+namespace chrome {
  590
+
  591
+// Declared in browser_dialogs.h.
  592
+void ShowTaskManager(Browser* browser, bool highlight_background_resources) {
  593
+  TaskManagerMac::Show(highlight_background_resources);
  594
+}
  595
+
  596
+}  // namespace chrome
  597
+
8  ui/gtk/browser_window_gtk.cc
@@ -963,14 +963,6 @@ void BrowserWindowGtk::ShowUpdateChromeDialog() {
963 963
   UpdateRecommendedDialog::Show(window_);
964 964
 }
965 965
 
966  
-void BrowserWindowGtk::ShowTaskManager() {
967  
-  TaskManagerGtk::Show(false);
968  
-}
969  
-
970  
-void BrowserWindowGtk::ShowBackgroundPages() {
971  
-  TaskManagerGtk::Show(true);
972  
-}
973  
-
974 966
 void BrowserWindowGtk::ShowBookmarkBubble(const GURL& url,
975 967
                                           bool already_bookmarked) {
976 968
   toolbar_->GetLocationBarView()->ShowStarBubble(url, !already_bookmarked);
2  ui/gtk/browser_window_gtk.h
@@ -128,8 +128,6 @@ class BrowserWindowGtk
128 128
                                         Profile* profile) OVERRIDE;
129 129
   virtual void ToggleBookmarkBar() OVERRIDE;
130 130
   virtual void ShowUpdateChromeDialog() OVERRIDE;
131  
-  virtual void ShowTaskManager() OVERRIDE;
132  
-  virtual void ShowBackgroundPages() OVERRIDE;
133 131
   virtual void ShowBookmarkBubble(const GURL& url,
134 132
                                   bool already_bookmarked) OVERRIDE;
135 133
   virtual void ShowChromeToMobileBubble() OVERRIDE;
9  ui/gtk/task_manager_gtk.cc
@@ -965,3 +965,12 @@ gboolean TaskManagerGtk::OnGtkAccelerator(GtkAccelGroup* accel_group,
965 965
 
966 966
   return TRUE;
967 967
 }
  968
+
  969
+namespace chrome {
  970
+
  971
+// Declared in browser_dialogs.h.
  972
+void ShowTaskManager(Browser* browser, bool highlight_background_resources) {
  973
+  TaskManagerGtk::Show(highlight_background_resources);
  974
+}
  975
+
  976
+}  // namespace chrome
6  ui/views/browser_dialogs.h
@@ -50,12 +50,6 @@ bool IsChromeToMobileBubbleViewShowing();
50 50
 // Creates and returns a find bar for the given browser window. See FindBarWin.
51 51
 FindBar* CreateFindBar(BrowserView* browser_view);
52 52
 
53  
-// Shows the Task Manager.
54  
-void ShowTaskManager(Browser* browser);
55  
-
56  
-// Shows the Task Manager, highlighting the background pages.
57  
-void ShowBackgroundPages(Browser* browser);
58  
-
59 53
 // Shows a dialog box that allows a search engine to be edited. |template_url|
60 54
 // is the search engine being edited. If it is NULL, then the dialog will add a
61 55
 // new search engine with the data the user supplies. |delegate| is an object
8  ui/views/frame/browser_view.cc
@@ -1209,14 +1209,6 @@ void BrowserView::ShowUpdateChromeDialog() {
1209 1209
   UpdateRecommendedMessageBox::Show(GetWidget()->GetNativeWindow());
1210 1210
 }
1211 1211
 
1212  
-void BrowserView::ShowTaskManager() {
1213  
-  chrome::ShowTaskManager(browser());
1214  
-}
1215  
-
1216  
-void BrowserView::ShowBackgroundPages() {
1217  
-  chrome::ShowBackgroundPages(browser());
1218  
-}
1219  
-
1220 1212
 void BrowserView::ShowBookmarkBubble(const GURL& url, bool already_bookmarked) {
1221 1213
   chrome::ShowBookmarkBubbleView(GetToolbarView()->GetBookmarkBubbleAnchor(),
1222 1214
                                  bookmark_bar_view_.get(), browser_->profile(),
2  ui/views/frame/browser_view.h
@@ -312,8 +312,6 @@ class BrowserView : public BrowserWindow,
312 312
                                         Profile* profile) OVERRIDE;
313 313
   virtual void ToggleBookmarkBar() OVERRIDE;
314 314
   virtual void ShowUpdateChromeDialog() OVERRIDE;
315  
-  virtual void ShowTaskManager() OVERRIDE;
316  
-  virtual void ShowBackgroundPages() OVERRIDE;
317 315
   virtual void ShowBookmarkBubble(const GURL& url,
318 316
                                   bool already_bookmarked) OVERRIDE;
319 317
   virtual void ShowBookmarkPrompt() OVERRIDE;
26  ui/views/task_manager_view.cc
@@ -44,6 +44,10 @@
44 44
 #include "ui/views/widget/widget.h"
45 45
 #include "ui/views/window/dialog_delegate.h"
46 46
 
  47
+#if defined(USE_ASH)
  48
+#include "ash/wm/window_util.h"
  49
+#endif
  50
+
47 51
 #if defined(OS_WIN)
48 52
 #include "win8/util/win8_util.h"
49 53
 #endif
@@ -531,7 +535,10 @@ void TaskManagerView::Show(bool highlight_background_resources,
531 535
   // In Windows Metro it's not good to open this native window.
532 536
   DCHECK(!win8::IsSingleWindowMetroMode());
533 537
 #endif
534  
-  const chrome::HostDesktopType desktop_type = browser->host_desktop_type();
  538
+  // In ash we can come here through the ChromeShellDelegate. If there is no
  539
+  // browser window at that time of the call, browser could be passed as NULL.
  540
+  const chrome::HostDesktopType desktop_type =
  541
+      browser ? browser->host_desktop_type() : chrome::HOST_DESKTOP_TYPE_ASH;
535 542
 
536 543
   if (instance_) {
537 544
     if (instance_->highlight_background_resources_ !=
@@ -545,8 +552,13 @@ void TaskManagerView::Show(bool highlight_background_resources,
545 552
     }
546 553
   }
547 554
   instance_ = new TaskManagerView(highlight_background_resources, desktop_type);
548  
-  DialogDelegateView::CreateDialogWidget(instance_,
549  
-      browser->window()->GetNativeWindow(), NULL);
  555
+  gfx::NativeWindow window =
  556
+      browser ? browser->window()->GetNativeWindow() : NULL;
  557
+#if defined(USE_ASH)
  558
+  if (!window)
  559
+    window = ash::wm::GetActiveWindow();
  560
+#endif
  561
+  DialogDelegateView::CreateDialogWidget(instance_, window, NULL);
550 562
   instance_->InitAlwaysOnTopState();
551 563
   instance_->model_->StartUpdating();
552 564
   instance_->GetWidget()->Show();
@@ -763,12 +775,8 @@ bool TaskManagerView::GetSavedAlwaysOnTopState(bool* always_on_top) const {
763 775
 namespace chrome {
764 776
 
765 777
 // Declared in browser_dialogs.h so others don't need to depend on our header.
766  
-void ShowTaskManager(Browser* browser) {
767  
-  TaskManagerView::Show(false, browser);
768  
-}
769  
-
770  
-void ShowBackgroundPages(Browser* browser) {
771  
-  TaskManagerView::Show(true, browser);
  778
+void ShowTaskManager(Browser* browser, bool highlight_background_resources) {
  779
+  TaskManagerView::Show(highlight_background_resources, browser);
772 780
 }
773 781
 
774 782
 }  // namespace chrome

0 notes on commit a20cce2

Please sign in to comment.
Something went wrong with that request. Please try again.