diff options
author | sail@chromium.org <sail@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-15 23:53:52 +0000 |
---|---|---|
committer | sail@chromium.org <sail@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-10-15 23:53:52 +0000 |
commit | c7486cc49e9bebda3598cb14bab05bd16969d9cb (patch) | |
tree | e69d637024405fac058f03f6c8b4c6e4c8161082 | |
parent | 61d71fe9c9aabcb5b376df7932722777c3d0ca41 (diff) | |
download | chromium_src-c7486cc49e9bebda3598cb14bab05bd16969d9cb.zip chromium_src-c7486cc49e9bebda3598cb14bab05bd16969d9cb.tar.gz chromium_src-c7486cc49e9bebda3598cb14bab05bd16969d9cb.tar.bz2 |
Mac Web Intents: Fix constrained window memory management
Currently ConstrainedWindowMac2 manages its own lifetime. This CL changes things so that ConstrainedWindowMac must explicitly be deleted.
Here's a summary of how the memory management used to work and how it works now:
https://docs.google.com/document/d/1rlZRfHtaBdqDeTj2vNJ3vCPxCwZ3zscHpBmLJtrsVeI/edit
BUG=152010
TBR=ben@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=161987
Review URL: https://codereview.chromium.org/11139010
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@162006 0039d316-1c4b-4281-b951-d872f2087c98
25 files changed, 174 insertions, 230 deletions
diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_alert.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_alert.mm index 9304345..27bff9c 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_alert.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_alert.mm @@ -43,7 +43,6 @@ const CGFloat kButtonMinWidth = 72; if ((self = [super init])) { window_.reset([[ConstrainedWindowCustomWindow alloc] initWithContentRect:ui::kWindowSizeDeterminedLater]); - [window_ setReleasedWhenClosed:NO]; NSView* contentView = [window_ contentView]; informativeTextField_.reset([constrained_window::CreateLabel() retain]); diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h b/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h deleted file mode 100644 index 004e9f2..0000000 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_CONTROLLER_H_ -#define CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_CONTROLLER_H_ - -#import <Cocoa/Cocoa.h> - -#import "base/memory/scoped_nsobject.h" - -namespace content { -class WebContents; -} - -class ConstrainedWindow; - -// A window controller to manage constrained windows. This class creates a -// native window and a ConstrainedWindow object. -// -// The caller can also set a embedded view object that will be managed by this -// class. If the embedded view resizes then the constrained window is resized to -// match. -@interface ConstrainedWindowController : NSWindowController { - @private - // Weak pointer to the constrained window that shows the picker. - // The constrained window manages its own lifetime. - // TODO(sail): Change to a strong pointer. - ConstrainedWindow* constrainedWindow_; - scoped_nsobject<NSView> embeddedView_; -} - -// Creates a new constrained window attached to |parentWebContents|. -// The |embeddedView| is added inside the constrained window. -- (id)initWithParentWebContents:(content::WebContents*)parentWebContents - embeddedView:(NSView*)embeddedView; - -@end - -#endif // CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_CONTROLLER_H_ diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.mm deleted file mode 100644 index b02cead..0000000 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.mm +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h" - -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.h" -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h" -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h" - -@interface ConstrainedWindowController () -- (void)onEmbeddedViewFrameDidChange:(NSNotification*)note; -@end - -@implementation ConstrainedWindowController - -- (id)initWithParentWebContents:(content::WebContents*)parentWebContents - embeddedView:(NSView*)embeddedView { - scoped_nsobject<NSWindow> window([[ConstrainedWindowCustomWindow alloc] - initWithContentRect:[embeddedView bounds]]); - if ((self = [super initWithWindow:window])) { - embeddedView_.reset([embeddedView retain]); - [[window contentView] addSubview:embeddedView]; - - [embeddedView setPostsFrameChangedNotifications:YES]; - [[NSNotificationCenter defaultCenter] - addObserver:self - selector:@selector(onEmbeddedViewFrameDidChange:) - name:NSViewFrameDidChangeNotification - object:embeddedView]; - - constrainedWindow_ = new ConstrainedWindowMac2(parentWebContents, window); - } - return self; -} - -- (void)dealloc { - [[NSNotificationCenter defaultCenter] removeObserver:self]; - [super dealloc]; -} - -- (void)close { - if (constrainedWindow_) { - constrainedWindow_->CloseConstrainedWindow(); - constrainedWindow_ = NULL; - } -} - -- (void)onEmbeddedViewFrameDidChange:(NSNotification*)note { - NSSize newSize = [embeddedView_ frame].size; - [[ConstrainedWindowSheetController controllerForSheet:[self window]] - setSheet:[self window] - windowSize:newSize]; -} - -@end diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller_browsertest.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller_browsertest.mm deleted file mode 100644 index 6943849..0000000 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_controller_browsertest.mm +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h" - -#include "chrome/browser/ui/browser.h" -#include "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h" -#include "chrome/browser/ui/cocoa/run_loop_testing.h" -#include "chrome/browser/ui/tabs/tab_strip_model.h" -#include "chrome/test/base/in_process_browser_test.h" - -typedef InProcessBrowserTest ConstrainedWindowControllerTest; - -IN_PROC_BROWSER_TEST_F(ConstrainedWindowControllerTest, BasicTest) { - content::WebContents* tab = browser()->tab_strip_model()->GetWebContentsAt(0); - NSRect frame = NSMakeRect(0, 0, 100, 50); - scoped_nsobject<NSView> view([[NSView alloc] initWithFrame:frame]); - scoped_nsobject<ConstrainedWindowController> window_controller( - [[ConstrainedWindowController alloc] - initWithParentWebContents:tab - embeddedView:view]); - - // The window should match the view size. - NSWindow* window = [window_controller window]; - ConstrainedWindowSheetController* sheetController = - [ConstrainedWindowSheetController controllerForSheet:window]; - EXPECT_TRUE(sheetController); - EXPECT_TRUE(NSEqualSizes(frame.size, [window frame].size)); - - // Test that resizing the view resizes the window. - NSRect new_frame = NSMakeRect(0, 0, 200, 100); - [view setFrame:new_frame]; - EXPECT_TRUE(NSEqualSizes(new_frame.size, [window frame].size)); - - // Test that closing the window closes the constrained window. - [window_controller close]; - [sheetController endAnimationForSheet:window]; - chrome::testing::NSRunLoopRunAllPending(); - EXPECT_FALSE([ConstrainedWindowSheetController controllerForSheet:window]); -} diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.mm index d056e60..d97fded 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.mm @@ -5,6 +5,7 @@ #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.h" #import "base/memory/scoped_nsobject.h" +#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h" #import "chrome/browser/ui/constrained_window.h" #import "chrome/browser/ui/constrained_window_constants.h" #include "skia/ext/skia_utils_mac.h" @@ -23,6 +24,7 @@ [self setHasShadow:YES]; [self setBackgroundColor:[NSColor clearColor]]; [self setOpaque:NO]; + [self setReleasedWhenClosed:NO]; scoped_nsobject<NSView> contentView( [[ConstrainedWindowCustomWindowContentView alloc] initWithFrame:NSZeroRect]); @@ -35,6 +37,16 @@ return YES; } +- (NSRect)frameRectForContentRect:(NSRect)windowContent { + ConstrainedWindowSheetController* sheetController = + [ConstrainedWindowSheetController controllerForSheet:self]; + NSRect frame; + frame.origin = [sheetController originForSheet:self + withWindowSize:windowContent.size]; + frame.size = windowContent.size; + return frame; +} + @end @implementation ConstrainedWindowCustomWindowContentView diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window_unittest.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window_unittest.mm index c9a86b0..f40ac1a 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window_unittest.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window_unittest.mm @@ -11,8 +11,9 @@ class ConstrainedWindowCustomWindowTest : public CocoaTest { // Simply test creating and drawing the window. TEST_F(ConstrainedWindowCustomWindowTest, Basic) { - ConstrainedWindowCustomWindow* window = [[ConstrainedWindowCustomWindow alloc] - initWithContentRect:NSMakeRect(0, 0, 10, 10)]; + scoped_nsobject<ConstrainedWindowCustomWindow> window( + [[ConstrainedWindowCustomWindow alloc] + initWithContentRect:NSMakeRect(0, 0, 10, 10)]); EXPECT_TRUE([window canBecomeKeyWindow]); EXPECT_FALSE([window canBecomeMainWindow]); diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h index 1645c93..24ef877 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h @@ -13,11 +13,24 @@ namespace content { class WebContents; } +class ConstrainedWindowMac2; + +// A delegate for a constrained window. The delegate is notified when the +// window closes. +class ConstrainedWindowMacDelegate2 { + public: + virtual void OnConstrainedWindowClosed(ConstrainedWindowMac2* window) = 0; +}; // Constrained window implementation for Mac. +// Normally an instance of this class is owned by the delegate. The delegate +// should delete the instance when the window is closed. class ConstrainedWindowMac2 : public ConstrainedWindow { public: - ConstrainedWindowMac2(content::WebContents* web_contents, NSWindow* window); + ConstrainedWindowMac2(ConstrainedWindowMacDelegate2* delegate, + content::WebContents* web_contents, + NSWindow* window); + virtual ~ConstrainedWindowMac2(); // ConstrainedWindow implementation. virtual void ShowConstrainedWindow() OVERRIDE; @@ -28,11 +41,11 @@ class ConstrainedWindowMac2 : public ConstrainedWindow { virtual bool CanShowConstrainedWindow() OVERRIDE; private: - virtual ~ConstrainedWindowMac2(); - // Gets the parent window of the dialog. NSWindow* GetParentWindow() const; + ConstrainedWindowMacDelegate2* delegate_; // weak, owns us. + // The WebContents that owns and constrains this ConstrainedWindow. Weak. content::WebContents* web_contents_; diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.mm index 91e5163..6de8a20 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.mm @@ -14,9 +14,11 @@ #include "content/public/browser/web_contents_view.h" ConstrainedWindowMac2::ConstrainedWindowMac2( + ConstrainedWindowMacDelegate2* delegate, content::WebContents* web_contents, NSWindow* window) - : web_contents_(web_contents), + : delegate_(delegate), + web_contents_(web_contents), window_([window retain]) { DCHECK(web_contents); DCHECK(window_.get()); @@ -27,6 +29,9 @@ ConstrainedWindowMac2::ConstrainedWindowMac2( constrained_window_tab_helper->AddConstrainedDialog(this); } +ConstrainedWindowMac2::~ConstrainedWindowMac2() { +} + void ConstrainedWindowMac2::ShowConstrainedWindow() { NSWindow* parent_window = GetParentWindow(); DCHECK(parent_window); @@ -45,7 +50,8 @@ void ConstrainedWindowMac2::CloseConstrainedWindow() { ConstrainedWindowTabHelper* constrained_window_tab_helper = ConstrainedWindowTabHelper::FromWebContents(web_contents_); constrained_window_tab_helper->WillClose(this); - delete this; + if (delegate_) + delegate_->OnConstrainedWindowClosed(this); } void ConstrainedWindowMac2::PulseConstrainedWindow() { @@ -64,9 +70,6 @@ bool ConstrainedWindowMac2::CanShowConstrainedWindow() { return !browser->window()->IsInstantTabShowing(); } -ConstrainedWindowMac2::~ConstrainedWindowMac2() { -} - NSWindow* ConstrainedWindowMac2::GetParentWindow() const { // Tab contents in a tabbed browser may not be inside a window. For this // reason use a browser window if possible. diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac_browsertest.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac_browsertest.mm index dd050b0..458099b 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac_browsertest.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac_browsertest.mm @@ -12,6 +12,18 @@ #include "chrome/test/base/in_process_browser_test.h" #include "content/public/browser/web_contents.h" #include "googleurl/src/gurl.h" +#include "testing/gmock/include/gmock/gmock.h" + +using ::testing::NiceMock; + +namespace { + +class ConstrainedWindowDelegateMock : public ConstrainedWindowMacDelegate2 { + public: + MOCK_METHOD1(OnConstrainedWindowClosed, void(ConstrainedWindowMac2*)); +}; + +} // namespace class ConstrainedWindowMacTest : public InProcessBrowserTest { public: @@ -58,33 +70,33 @@ class ConstrainedWindowMacTest : public InProcessBrowserTest { // tab is activated. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, ShowInInactiveTab) { // Show dialog in non active tab. - // Dialog will delete itself when closed. - ConstrainedWindowMac2* dialog = new ConstrainedWindowMac2(tab0_, sheet_); + NiceMock<ConstrainedWindowDelegateMock> delegate; + ConstrainedWindowMac2 dialog(&delegate, tab0_, sheet_); EXPECT_EQ(0.0, [sheet_ alphaValue]); // Switch to inactive tab. browser()->tab_strip_model()->ActivateTabAt(0, true); EXPECT_EQ(1.0, [sheet_ alphaValue]); - dialog->CloseConstrainedWindow(); + dialog.CloseConstrainedWindow(); } // Test that adding a sheet disables tab dragging. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, TabDragging) { - // Dialog will delete itself when closed. - ConstrainedWindowMac2* dialog = new ConstrainedWindowMac2(tab1_, sheet_); + NiceMock<ConstrainedWindowDelegateMock> delegate; + ConstrainedWindowMac2 dialog(&delegate, tab1_, sheet_); // Verify that the dialog disables dragging. EXPECT_TRUE([controller_ isTabDraggable:tab_view0_]); EXPECT_FALSE([controller_ isTabDraggable:tab_view1_]); - dialog->CloseConstrainedWindow(); + dialog.CloseConstrainedWindow(); } // Test that closing a browser window with a sheet works. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, BrowserWindowClose) { - // Dialog will delete itself when closed. - new ConstrainedWindowMac2(tab1_, sheet_); + NiceMock<ConstrainedWindowDelegateMock> delegate; + ConstrainedWindowMac2 dialog(&delegate, tab1_, sheet_); EXPECT_EQ(1.0, [sheet_ alphaValue]); // Close the browser window. @@ -97,8 +109,8 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, BrowserWindowClose) { // Test that closing a tab with a sheet works. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, TabClose) { - // Dialog will delete itself when closed. - new ConstrainedWindowMac2(tab1_, sheet_); + NiceMock<ConstrainedWindowDelegateMock> delegate; + ConstrainedWindowMac2 dialog(&delegate, tab1_, sheet_); EXPECT_EQ(1.0, [sheet_ alphaValue]); // Close the tab. diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h index 9616ee6..414d0ae 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.h @@ -35,8 +35,8 @@ - (void)showSheet:(NSWindow*)sheet forParentView:(NSView*)parentView; -// Resize the given sheet. -- (void)setSheet:(NSWindow*)sheet windowSize:(NSSize)size; +// Calculates the position of the sheet for the given window size. +- (NSPoint)originForSheet:(NSWindow*)sheet withWindowSize:(NSSize)size; // Closes the given sheet. If the parent view of the sheet is currently active // then an asynchronous animation will be run and the sheet will be closed diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm index 65d7892..37d1b0a 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm @@ -154,18 +154,13 @@ NSValue* GetKeyForParentWindow(NSWindow* parent_window) { object:parentView]; } -- (void)setSheet:(NSWindow*)sheet - windowSize:(NSSize)size { +- (NSPoint)originForSheet:(NSWindow*)sheet + withWindowSize:(NSSize)size { ConstrainedWindowSheetInfo* info = [self findSheetInfoForSheet:sheet]; DCHECK(info); NSRect containerRect = [self overlayWindowFrameForParentView:[info parentView]]; - - NSRect sheetRect; - sheetRect.size = size; - sheetRect.origin = [self originForSheetSize:size - inContainerRect:containerRect]; - [sheet setFrame:sheetRect display:YES]; + return [self originForSheetSize:size inContainerRect:containerRect]; } - (void)closeSheet:(NSWindow*)sheet { diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller_unittest.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller_unittest.mm index 9007bcc..561f395 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller_unittest.mm @@ -178,14 +178,12 @@ TEST_F(ConstrainedWindowSheetControllerTest, Resize) { [controller showSheet:sheet_ forParentView:active_tab_view_]; NSRect old_frame = [sheet_ frame]; - NSSize desired_size = NSMakeSize(NSWidth(old_frame) + 100, - NSHeight(old_frame) + 50); - [controller setSheet:sheet_ - windowSize:desired_size]; - NSRect sheet_frame = [sheet_ frame]; - EXPECT_EQ(NSWidth(sheet_frame), desired_size.width); - EXPECT_EQ(NSHeight(sheet_frame), desired_size.height); + NSRect sheet_frame; + sheet_frame.size = NSMakeSize(NSWidth(old_frame) + 100, + NSHeight(old_frame) + 50); + sheet_frame.origin = [controller originForSheet:sheet_ + withWindowSize:sheet_frame.size]; // Y pos should not have changed. EXPECT_EQ(NSMaxY(sheet_frame), NSMaxY(old_frame)); diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h index cbc8e18..3707c39 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h +++ b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h @@ -9,7 +9,7 @@ #include "base/memory/scoped_nsobject.h" #include "chrome/browser/extensions/extension_install_prompt.h" -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h" +#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h" namespace content { class PageNavigator; @@ -20,10 +20,11 @@ class WebContents; // Displays an extension install prompt as a tab modal dialog. class ExtensionInstallDialogController : - public ExtensionInstallPrompt::Delegate { + public ExtensionInstallPrompt::Delegate, + public ConstrainedWindowMacDelegate2 { public: ExtensionInstallDialogController( - content::WebContents* webContents, + content::WebContents* web_contents, content::PageNavigator* navigator, ExtensionInstallPrompt::Delegate* delegate, const ExtensionInstallPrompt::Prompt& prompt); @@ -33,18 +34,21 @@ class ExtensionInstallDialogController : virtual void InstallUIProceed() OVERRIDE; virtual void InstallUIAbort(bool user_initiated) OVERRIDE; + // ConstrainedWindowMacDelegate2 implementation. + virtual void OnConstrainedWindowClosed( + ConstrainedWindowMac2* window) OVERRIDE; + + ConstrainedWindowMac2* constrained_window() const { + return constrained_window_.get(); + } ExtensionInstallViewController* view_controller() const { return view_controller_; } - ConstrainedWindowController* window_controller() const { - return window_controller_; - } - private: - ExtensionInstallPrompt::Delegate* const delegate_; + ExtensionInstallPrompt::Delegate* delegate_; scoped_nsobject<ExtensionInstallViewController> view_controller_; - scoped_nsobject<ConstrainedWindowController> window_controller_; + scoped_ptr<ConstrainedWindowMac2> constrained_window_; }; #endif // CHROME_BROWSER_UI_COCOA_EXTENSIONS_EXTENSION_INSTALL_DIALOG_CONTROLLER_H_ diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm index a5a6cdb..ea6b7fc 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm @@ -4,9 +4,10 @@ #import "chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.h" +#include "base/message_loop.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_finder.h" -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h" +#include "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.h" #import "chrome/browser/ui/cocoa/extensions/extension_install_view_controller.h" #include "chrome/browser/ui/tab_contents/tab_contents.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" @@ -36,7 +37,7 @@ void ShowExtensionInstallDialogImpl( } // namespace ExtensionInstallDialogController::ExtensionInstallDialogController( - content::WebContents* webContents, + content::WebContents* web_contents, content::PageNavigator* navigator, ExtensionInstallPrompt::Delegate* delegate, const ExtensionInstallPrompt::Prompt& prompt) : delegate_(delegate) { @@ -44,9 +45,13 @@ ExtensionInstallDialogController::ExtensionInstallDialogController( initWithNavigator:navigator delegate:this prompt:prompt]); - window_controller_.reset([[ConstrainedWindowController alloc] - initWithParentWebContents:webContents - embeddedView:[view_controller_ view]]); + + scoped_nsobject<NSWindow> window([[ConstrainedWindowCustomWindow alloc] + initWithContentRect:[[view_controller_ view] bounds]]); + [[window contentView] addSubview:[view_controller_ view]]; + + constrained_window_.reset(new ConstrainedWindowMac2( + this, web_contents, window)); } ExtensionInstallDialogController::~ExtensionInstallDialogController() { @@ -54,14 +59,21 @@ ExtensionInstallDialogController::~ExtensionInstallDialogController() { void ExtensionInstallDialogController::InstallUIProceed() { delegate_->InstallUIProceed(); - [window_controller_ close]; - delete this; + delegate_ = NULL; + constrained_window_->CloseConstrainedWindow(); } void ExtensionInstallDialogController::InstallUIAbort(bool user_initiated) { delegate_->InstallUIAbort(user_initiated); - [window_controller_ close]; - delete this; + delegate_ = NULL; + constrained_window_->CloseConstrainedWindow(); +} + +void ExtensionInstallDialogController::OnConstrainedWindowClosed( + ConstrainedWindowMac2* window) { + if (delegate_) + delegate_->InstallUIAbort(false); + MessageLoop::current()->DeleteSoon(FROM_HERE, this); } // static diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller_browsertest.mm b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller_browsertest.mm index 120c818..e1ef909 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller_browsertest.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller_browsertest.mm @@ -39,9 +39,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogControllerTest, BasicTest) { &delegate, prompt); - scoped_nsobject<ConstrainedWindowController> window_controller( - [controller->window_controller() retain]); - scoped_nsobject<NSWindow> window([[window_controller window] retain]); + scoped_nsobject<NSWindow> window( + [controller->constrained_window()->GetNativeWindow() retain]); EXPECT_TRUE([window isVisible]); // Press cancel to close the window diff --git a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm index ab553ac..e255132 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm @@ -35,6 +35,7 @@ using extensions::BundleInstaller; children:(NSArray*)children; - (NSDictionary*)buildIssue:(const IssueAdviceInfoEntry&)issue; - (NSArray*)buildWarnings:(const ExtensionInstallPrompt::Prompt&)prompt; +- (void)updateViewFrame:(NSRect)frame; @end namespace { @@ -269,10 +270,8 @@ void DrawBulletInFrame(NSRect frame) { // If necessary, adjust the window size. if (totalOffset) { NSRect currentRect = [[self view] bounds]; - [[self view] setFrame:NSMakeRect(0, - 0, - NSWidth(currentRect), - NSHeight(currentRect) + totalOffset)]; + currentRect.size.height += totalOffset; + [self updateViewFrame:currentRect]; } } @@ -309,11 +308,9 @@ void DrawBulletInFrame(NSRect frame) { CGFloat totalOffset = 0.0; OffsetOutlineViewVerticallyToFitContent(outlineView_, &totalOffset); if (totalOffset) { - NSRect currentRect = [[[self view] window] frame]; - currentRect.origin.y -= totalOffset; + NSRect currentRect = [[self view] bounds]; currentRect.size.height += totalOffset; - [[[self view] window] setFrame:currentRect - display:YES]; + [self updateViewFrame:currentRect]; } } @@ -493,4 +490,10 @@ void DrawBulletInFrame(NSRect frame) { return warnings; } +- (void)updateViewFrame:(NSRect)frame { + NSWindow* window = [[self view] window]; + [window setFrame:[window frameRectForContentRect:frame] display:YES]; + [[self view] setFrame:frame]; +} + @end diff --git a/chrome/browser/ui/cocoa/intents/web_intent_inline_service_view_controller_browsertest.mm b/chrome/browser/ui/cocoa/intents/web_intent_inline_service_view_controller_browsertest.mm index bcf73c8..f193ed3 100644 --- a/chrome/browser/ui/cocoa/intents/web_intent_inline_service_view_controller_browsertest.mm +++ b/chrome/browser/ui/cocoa/intents/web_intent_inline_service_view_controller_browsertest.mm @@ -36,13 +36,13 @@ class WebIntentInlineServiceViewControllerTest : public InProcessBrowserTest { [[WebIntentInlineServiceViewController alloc] initWithPicker:picker_]); view_.reset([[view_controller_ view] retain]); - NSWindow* sheet = [picker_->window_controller() window]; + NSWindow* sheet = picker_->constrained_window()->GetNativeWindow(); [[sheet contentView] addSubview:view_]; } virtual void CleanUpOnMainThread() OVERRIDE { EXPECT_CALL(delegate_, OnClosing()); - NSWindow* sheet = [picker_->window_controller() window]; + NSWindow* sheet = picker_->constrained_window()->GetNativeWindow(); ConstrainedWindowSheetController* sheetController = [ConstrainedWindowSheetController controllerForSheet:sheet]; picker_->Close(); diff --git a/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.h b/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.h index 437b24a..1bcc969 100644 --- a/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.h +++ b/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.h @@ -8,6 +8,7 @@ #import <Cocoa/Cocoa.h> #include "base/memory/scoped_nsobject.h" +#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h" #include "chrome/browser/ui/intents/web_intent_picker.h" #include "chrome/browser/ui/intents/web_intent_picker_model_observer.h" @@ -17,7 +18,8 @@ // the Cocoa WebIntentPickerViewController object. Note, this will eventually // replace WebIntentPickerCocoa, see http://crbug.com/152010. class WebIntentPickerCocoa2 : public WebIntentPicker, - public WebIntentPickerModelObserver { + public WebIntentPickerModelObserver, + public ConstrainedWindowMacDelegate2 { public: WebIntentPickerCocoa2(content::WebContents* web_contents, WebIntentPickerDelegate* delegate, @@ -27,7 +29,9 @@ class WebIntentPickerCocoa2 : public WebIntentPicker, WebIntentPickerDelegate* delegate() const { return delegate_; } WebIntentPickerModel* model() const { return model_; } content::WebContents* web_contents() const { return web_contents_; } - NSWindowController* window_controller() const { return window_controller_; } + ConstrainedWindowMac2* constrained_window() const { + return constrained_window_.get(); + } WebIntentPickerViewController* view_controller() const { return view_controller_; } @@ -59,12 +63,16 @@ class WebIntentPickerCocoa2 : public WebIntentPicker, virtual void OnInlineDisposition(const string16& title, const GURL& url) OVERRIDE; + // ConstrainedWindowMacDelegate2 implementation. + virtual void OnConstrainedWindowClosed( + ConstrainedWindowMac2* window) OVERRIDE; + private: content::WebContents* const web_contents_; WebIntentPickerDelegate* const delegate_; WebIntentPickerModel* model_; scoped_nsobject<WebIntentPickerViewController> view_controller_; - scoped_nsobject<NSWindowController> window_controller_; + scoped_ptr<ConstrainedWindowMac2> constrained_window_; }; #endif // CHROME_BROWSER_UI_COCOA_INTENTS_WEB_INTENT_PICKER_COCOA2_H_ diff --git a/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.mm b/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.mm index 11d87b5..72edc4d 100644 --- a/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.mm +++ b/chrome/browser/ui/cocoa/intents/web_intent_picker_cocoa2.mm @@ -9,9 +9,10 @@ #include "base/mac/foundation_util.h" #include "base/message_loop.h" #import "chrome/browser/ui/cocoa/chrome_event_processing_window.h" -#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_controller.h" +#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.h" #import "chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller.h" #include "chrome/browser/ui/intents/web_intent_picker_delegate.h" +#include "chrome/browser/ui/tab_contents/tab_contents.h" #include "content/public/browser/native_web_keyboard_event.h" WebIntentPickerCocoa2::WebIntentPickerCocoa2(content::WebContents* web_contents, @@ -24,10 +25,14 @@ WebIntentPickerCocoa2::WebIntentPickerCocoa2(content::WebContents* web_contents, view_controller_.reset( [[WebIntentPickerViewController alloc] initWithPicker:this]); - // TODO(sail) Support bubble window as well. - window_controller_.reset([[ConstrainedWindowController alloc] - initWithParentWebContents:web_contents - embeddedView:[view_controller_ view]]); + + scoped_nsobject<NSWindow> window([[ConstrainedWindowCustomWindow alloc] + initWithContentRect:[[view_controller_ view] bounds]]); + [[window contentView] addSubview:[view_controller_ view]]; + + constrained_window_.reset(new ConstrainedWindowMac2( + this, web_contents, window)); + [view_controller_ update]; } @@ -35,13 +40,7 @@ WebIntentPickerCocoa2::~WebIntentPickerCocoa2() { } void WebIntentPickerCocoa2::Close() { - // After the OnClosing call the model may be deleted so unset this reference. - model_->set_observer(NULL); - model_ = NULL; - - [window_controller_ close]; - delegate_->OnClosing(); - MessageLoop::current()->DeleteSoon(FROM_HERE, this); + constrained_window_->CloseConstrainedWindow(); } void WebIntentPickerCocoa2::SetActionString(const string16& action) { @@ -77,7 +76,7 @@ void WebIntentPickerCocoa2::OnInlineDispositionHandleKeyboardEvent( } ChromeEventProcessingWindow* window = base::mac::ObjCCastStrict<ChromeEventProcessingWindow>( - [window_controller_ window]); + constrained_window_->GetNativeWindow()); [window redispatchKeyEvent:event.os_event]; } @@ -113,3 +112,13 @@ void WebIntentPickerCocoa2::OnInlineDisposition(const string16& title, const GURL& url) { [view_controller_ update]; } + +void WebIntentPickerCocoa2::OnConstrainedWindowClosed( + ConstrainedWindowMac2* window) { + // After the OnClosing call the model may be deleted so unset this reference. + model_->set_observer(NULL); + model_ = NULL; + + delegate_->OnClosing(); + MessageLoop::current()->DeleteSoon(FROM_HERE, this); +} diff --git a/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller.mm b/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller.mm index 8002ea9..5b0393a 100644 --- a/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller.mm +++ b/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller.mm @@ -206,6 +206,9 @@ [closeButton_ setFrame:closeFrame]; [[self view] setFrame:bounds]; + + NSWindow* window = [[self view] window]; + [window setFrame:[window frameRectForContentRect:bounds] display:YES]; } - (NSRect)minimumInnerFrame { diff --git a/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller_browsertest.mm b/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller_browsertest.mm index 3f3e85b..672f785 100644 --- a/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller_browsertest.mm +++ b/chrome/browser/ui/cocoa/intents/web_intent_picker_view_controller_browsertest.mm @@ -49,7 +49,7 @@ class WebIntentPickerViewControllerTest : public InProcessBrowserTest { virtual void CleanUpOnMainThread() OVERRIDE { EXPECT_CALL(delegate_, OnClosing()); - NSWindow* sheet = [picker_->window_controller() window]; + NSWindow* sheet = picker_->constrained_window()->GetNativeWindow(); ConstrainedWindowSheetController* sheetController = [ConstrainedWindowSheetController controllerForSheet:sheet]; picker_->Close(); @@ -68,7 +68,7 @@ class WebIntentPickerViewControllerTest : public InProcessBrowserTest { IN_PROC_BROWSER_TEST_F(WebIntentPickerViewControllerTest, View) { EXPECT_TRUE([controller_ view]); EXPECT_TRUE([[controller_ view] superview]); - EXPECT_NSEQ([picker_->window_controller() window], + EXPECT_NSEQ(picker_->constrained_window()->GetNativeWindow(), [[controller_ view] window]); } diff --git a/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.h b/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.h index da34113..6c53bcf 100644 --- a/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.h +++ b/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.h @@ -9,6 +9,7 @@ #include "base/memory/scoped_ptr.h" #include "chrome/browser/ui/cocoa/constrained_window_mac.h" +#include "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac2.h" #include "chrome/browser/ui/tab_modal_confirm_dialog.h" @class ConstrainedWindowAlert; @@ -46,7 +47,8 @@ class TabModalConfirmDialogMac // This class is the same as TabModalConfirmDialogMac except that it uses // the new constrained window look and feel. -class TabModalConfirmDialogMac2 : public TabModalConfirmDialog { +class TabModalConfirmDialogMac2 : public TabModalConfirmDialog, + public ConstrainedWindowMacDelegate2 { public: TabModalConfirmDialogMac2(TabModalConfirmDialogDelegate* delegate, TabContents* tab_contents); @@ -58,6 +60,11 @@ class TabModalConfirmDialogMac2 : public TabModalConfirmDialog { virtual void AcceptTabModalDialog() OVERRIDE; virtual void CancelTabModalDialog() OVERRIDE; + // ConstrainedWindowMacDelegate2: + virtual void OnConstrainedWindowClosed( + ConstrainedWindowMac2* window) OVERRIDE; + + scoped_ptr<ConstrainedWindowMac2> window_; scoped_ptr<TabModalConfirmDialogDelegate> delegate_; scoped_nsobject<ConstrainedWindowAlert> alert_; scoped_nsobject<TabModalConfirmDialogMacBridge2> bridge_; diff --git a/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm b/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm index d6da719..1ef5c5a 100644 --- a/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm +++ b/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm @@ -162,8 +162,9 @@ TabModalConfirmDialogMac2::TabModalConfirmDialogMac2( [[alert_ closeButton] setAction:@selector(onCancelButton:)]; [alert_ layout]; - delegate->set_window( - new ConstrainedWindowMac2(tab_contents->web_contents(), [alert_ window])); + window_.reset(new ConstrainedWindowMac2( + this, tab_contents->web_contents(), [alert_ window])); + delegate->set_window(window_.get()); } TabModalConfirmDialogMac2::~TabModalConfirmDialogMac2() { @@ -174,3 +175,8 @@ void TabModalConfirmDialogMac2::AcceptTabModalDialog() { void TabModalConfirmDialogMac2::CancelTabModalDialog() { } + +void TabModalConfirmDialogMac2::OnConstrainedWindowClosed( + ConstrainedWindowMac2* window) { + delete this; +} diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi index bf765c7..4f720e3 100644 --- a/chrome/chrome_browser_ui.gypi +++ b/chrome/chrome_browser_ui.gypi @@ -385,8 +385,6 @@ 'browser/ui/cocoa/constrained_window/constrained_window_button.mm', 'browser/ui/cocoa/constrained_window/constrained_window_control_utils.h', 'browser/ui/cocoa/constrained_window/constrained_window_control_utils.mm', - 'browser/ui/cocoa/constrained_window/constrained_window_controller.h', - 'browser/ui/cocoa/constrained_window/constrained_window_controller.mm', 'browser/ui/cocoa/constrained_window/constrained_window_custom_window.h', 'browser/ui/cocoa/constrained_window/constrained_window_custom_window.h', 'browser/ui/cocoa/constrained_window/constrained_window_custom_window.mm', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index f7fc4b6..86aa4e6 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -3078,7 +3078,6 @@ 'browser/ui/cocoa/applescript/window_applescript_test.mm', 'browser/ui/cocoa/web_intent_sheet_controller_browsertest.mm', 'browser/ui/cocoa/browser_window_controller_browsertest.mm', - 'browser/ui/cocoa/constrained_window/constrained_window_controller_browsertest.mm', 'browser/ui/cocoa/constrained_window/constrained_window_mac_browsertest.mm', 'browser/ui/cocoa/extensions/extension_install_dialog_controller_browsertest.mm', 'browser/ui/cocoa/extensions/extension_install_prompt_test_utils.h', |