Ensure dialogs are centered on browser windows.

Also, remove notifications no longer needed after r272421.

BUG=364831
TEST=as in bug

Review URL: https://codereview.chromium.org/420413002

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

chrome/browser/ui/cocoa/browser_window_controller_private.mm

@@ -33,7 +33,6 @@
 #import "chrome/browser/ui/cocoa/profiles/avatar_icon_controller.h"
 #import "chrome/browser/ui/cocoa/status_bubble_mac.h"
 #import "chrome/browser/ui/cocoa/tab_contents/overlayable_contents_controller.h"
-#import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_view.h"
 #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
 #import "chrome/browser/ui/cocoa/version_independent_window.h"
@@ -279,20 +278,6 @@ willPositionSheet:(NSWindow*)sheet
   // Normally, we don't need to tell the toolbar whether or not to show the
   // divider, but things break down during animation.
   [toolbarController_ setDividerOpacity:[self toolbarDividerOpacity]];
-
-  // Update the position of the active constrained window sheet.  We force this
-  // here because the |sheetParentView| may not have been resized (e.g., to
-  // prevent jank during a fullscreen mode transition), but constrained window
-  // sheets also compute their position based on the bookmark bar and toolbar.
-  content::WebContents* const activeWebContents =
-      browser_->tab_strip_model()->GetActiveWebContents();
-  NSView* const sheetParentView = activeWebContents ?
-      GetSheetParentViewForWebContents(activeWebContents) : nil;
-  if (sheetParentView) {
-    [[NSNotificationCenter defaultCenter]
-      postNotificationName:NSViewFrameDidChangeNotification
-                    object:sheetParentView];
-  }
 }
 
 - (CGFloat)floatingBarHeight {

chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm

@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet.h"
 #include "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_info.h"
+#import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 
 namespace {
 
@@ -243,7 +244,7 @@ NSValue* GetKeyForParentWindow(NSWindow* parent_window) {
 }
 
 - (NSRect)overlayWindowFrameForParentView:(NSView*)parentView {
-  NSRect viewFrame = [parentView convertRect:[parentView bounds] toView:nil];
+  NSRect viewFrame = GetSheetParentBoundsForParentView(parentView);
 
   id<NSWindowDelegate> delegate = [[parentView window] delegate];
   if ([delegate respondsToSelector:@selector(window:

chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller_unittest.mm

@@ -75,21 +75,31 @@ class ConstrainedWindowSheetControllerTest : public CocoaTest {
     // Center the window so that the sheet doesn't go offscreen.
     [test_window() center];
 
+    // The real view setup is quite a few levels deep; recreate something
+    // similar.
+    NSRect dummy_rect = NSMakeRect(0, 0, 50, 50);
+    tab_view_parent_ = [test_window() contentView];
+    for (int i = 0; i < 3; ++i) {
+      base::scoped_nsobject<NSView> new_view(
+          [[NSView alloc] initWithFrame:dummy_rect]);
+      [tab_view_parent_ addSubview:new_view.get()];
+      tab_view_parent_ = new_view.get();
+    }
+
     // Create two dummy tabs and make the first one active.
-    NSRect dummyRect = NSMakeRect(0, 0, 50, 50);
     tab_views_.reset([[NSMutableArray alloc] init]);
     for (int i = 0; i < 2; ++i) {
       base::scoped_nsobject<NSView> view(
-          [[NSView alloc] initWithFrame:dummyRect]);
+          [[NSView alloc] initWithFrame:dummy_rect]);
       [tab_views_ addObject:view];
     }
-    tab0_.reset([[tab_views_ objectAtIndex:0] retain]);
-    tab1_.reset([[tab_views_ objectAtIndex:1] retain]);
+    tab0_ = [tab_views_ objectAtIndex:0];
+    tab1_ = [tab_views_ objectAtIndex:1];
     ActivateTabView(tab0_);
 
     // Create a test sheet.
     sheet_window_.reset([[NSWindow alloc]
-        initWithContentRect:dummyRect
+        initWithContentRect:dummy_rect
                   styleMask:NSTitledWindowMask
                     backing:NSBackingStoreBuffered
                       defer:NO]);
@@ -110,11 +120,10 @@ class ConstrainedWindowSheetControllerTest : public CocoaTest {
   }
 
   void ActivateTabView(NSView* tab_view) {
-    for (NSView* view in tab_views_.get()) {
+    for (NSView* view in tab_views_.get())
       [view removeFromSuperview];
-    }
-    [[test_window() contentView] addSubview:tab_view];
-    active_tab_view_.reset([tab_view retain]);
+    [tab_view_parent_ addSubview:tab_view];
+    active_tab_view_ = tab_view;
 
     ConstrainedWindowSheetController* controller =
         [ConstrainedWindowSheetController
@@ -145,9 +154,10 @@ class ConstrainedWindowSheetControllerTest : public CocoaTest {
   base::scoped_nsobject<CustomConstrainedWindowSheet> sheet_;
   base::scoped_nsobject<ConstrainedWindowSheetController> controller_;
   base::scoped_nsobject<NSMutableArray> tab_views_;
-  base::scoped_nsobject<NSView> active_tab_view_;
-  base::scoped_nsobject<NSView> tab0_;
-  base::scoped_nsobject<NSView> tab1_;
+  NSView* tab_view_parent_;
+  NSView* active_tab_view_;
+  NSView* tab0_;
+  NSView* tab1_;
 };
 
 // Test showing then hiding the sheet.
@@ -209,18 +219,6 @@ TEST_F(ConstrainedWindowSheetControllerTest, TwoParentWindows) {
   [parent_window2 close];
 }
 
-// Test that using a top level parent view works.
-TEST_F(ConstrainedWindowSheetControllerTest, TopLevelView) {
-  NSView* parentView = [[test_window() contentView] superview];
-  [controller_ parentViewDidBecomeActive:parentView];
-
-  EXPECT_FALSE([sheet_window_ isVisible]);
-  [controller_ showSheet:sheet_ forParentView:parentView];
-  EXPECT_TRUE([ConstrainedWindowSheetController controllerForSheet:sheet_]);
-  EXPECT_TRUE([sheet_window_ isVisible]);
-  VerifySheetXPosition([sheet_window_ frame], parentView);
-}
-
 // Test that resizing sheet works.
 TEST_F(ConstrainedWindowSheetControllerTest, Resize) {
   [controller_ showSheet:sheet_ forParentView:active_tab_view_];
@@ -269,8 +267,8 @@ TEST_F(ConstrainedWindowSheetControllerTest, ResizeParentWindow) {
   CGFloat insets[] = {-10, 0, 10};
   NSRect old_frame = [test_window() frame];
 
-  for (size_t x = 0; x < sizeof(insets) / sizeof(CGFloat); x++) {
-    for (size_t y = 0; y < sizeof(insets) / sizeof(CGFloat); y++) {
+  for (size_t x = 0; x < arraysize(insets); x++) {
+    for (size_t y = 0; y < arraysize(insets); y++) {
       NSRect resized_frame = NSInsetRect(old_frame, insets[x], insets[y]);
       [test_window() setFrame:resized_frame display:YES];
       NSRect sheet_frame = [sheet_window_ frame];
@@ -292,8 +290,7 @@ TEST_F(ConstrainedWindowSheetControllerTest, SystemSheet) {
   [system_sheet setAlert:alert];
 
   EXPECT_FALSE([[alert window] isVisible]);
-  [controller_ showSheet:system_sheet
-                 forParentView:active_tab_view_];
+  [controller_ showSheet:system_sheet forParentView:active_tab_view_];
   EXPECT_TRUE([[alert window] isVisible]);
 
   [controller_ closeSheet:system_sheet];
@@ -309,8 +306,7 @@ TEST_F(ConstrainedWindowSheetControllerTest, SystemSheetAddToInactiveTab) {
   [system_sheet setAlert:alert];
 
   EXPECT_FALSE([[alert window] isVisible]);
-  [controller_ showSheet:system_sheet
-                 forParentView:tab1_];
+  [controller_ showSheet:system_sheet forParentView:tab1_];
   EXPECT_FALSE([[alert window] isVisible]);
 
   ActivateTabView(tab1_);

chrome/browser/ui/cocoa/tabs/tab_strip_controller.h

@@ -258,4 +258,8 @@ class WebContents;
 // Returns the parent view to use when showing a sheet for a given web contents.
 NSView* GetSheetParentViewForWebContents(content::WebContents* web_contents);
 
+// Returns the bounds to use when showing a sheet for a given parent view. This
+// returns a rect in window coordinates.
+NSRect GetSheetParentBoundsForParentView(NSView* view);
+
 #endif  // CHROME_BROWSER_UI_COCOA_TABS_TAB_STRIP_CONTROLLER_H_

chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm

@@ -19,7 +19,6 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
-#include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
@@ -2222,9 +2221,18 @@ NSView* GetSheetParentViewForWebContents(WebContents* web_contents) {
   // View hierarchy of the contents view:
   // NSView  -- switchView, same for all tabs
   // +- NSView  -- TabContentsController's view
-  //    +- TabContentsViewCocoa
+  //    +- WebContentsViewCocoa
   //
   // Changing it? Do not forget to modify
   // -[TabStripController swapInTabAtIndex:] too.
   return [web_contents->GetNativeView() superview];
 }
+
+NSRect GetSheetParentBoundsForParentView(NSView* view) {
+  // If the devtools view is open, it shrinks the size of the WebContents, so go
+  // up the hierarchy to the devtools container view to avoid that. Note that
+  // the devtools view is always in the hierarchy even if it is not open or it
+  // is detached.
+  NSView* devtools_view = [[[view superview] superview] superview];
+  return [devtools_view convertRect:[devtools_view bounds] toView:nil];
+}