From cc825828ae6b2525431948587a486cc2e3852f67 Mon Sep 17 00:00:00 2001 From: Jennifer Apacible Date: Mon, 14 Mar 2016 11:28:09 -0700 Subject: [PATCH] [Reland] Enable AutoResize for Constrained Web Dialogs for Mac. Currently only constrained web dialogs for views (Linux/Windows) are able to autoresize. This change implements the option to pass in minimum and maximum sizes and enabling autoresizing functionality for OSX. This change adds two static functions for options on whether to create a ConstrainedWindowsMac that autoresizes or is of fixed size. The first two patches were reverted because of flaky tests on Mac 10.9. Those patches can be found at: 1. https://codereview.chromium.org/1430023002 2. https://codereview.chromium.org/1446623003 After some investigation, we found that the failures are being caused by an occlusion notifications in cocoa, which is not expected in browser tests. This is currently mac-only. By disabling these notifications in browser tests, we see this patch passing on the swarming bots that were previously failing. See http://crbug/558585. The patch to disable occlusion notifications can be found at: https://codereview.chromium.org/1762883002/ The third patch (same as second patch) broke dialogs like print preview. This was because the change used ui::kWindowSizeDeterminedLater for the dialog window's frame for all constrained web dialogs on mac, not just autoresizable dialogs. That has been amended in this change. That patch can be found here: https://codereview.chromium.org/1699763002/ BUG=217034 Review URL: https://codereview.chromium.org/1781433002 Cr-Commit-Position: refs/heads/master@{#380352} (cherry picked from commit 740c4109f954a11c544c02132a89fabab767e7ab) Review URL: https://codereview.chromium.org/1796273004 . Cr-Commit-Position: refs/branch-heads/2661@{#218} Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081} --- .../cocoa/autofill/autofill_dialog_cocoa.mm | 4 +- .../card_unmask_prompt_view_bridge.mm | 4 +- .../ui/cocoa/certificate_viewer_mac.mm | 8 +- .../constrained_web_dialog_delegate_mac.mm | 160 ++++++++++++++++-- .../constrained_window_custom_sheet.mm | 4 + .../constrained_window_mac.h | 29 ++++ .../constrained_window_mac.mm | 50 +++++- .../constrained_window_mac_browsertest.mm | 26 +-- .../constrained_window_sheet.h | 2 + ...rained_window_sheet_controller_unittest.mm | 4 + .../constrained_window_web_dialog_sheet.h | 1 + .../constrained_window_web_dialog_sheet.mm | 18 +- .../content_settings/collected_cookies_mac.mm | 3 +- .../device_permissions_dialog_controller.mm | 4 +- .../extension_install_dialog_controller.mm | 4 +- .../media_galleries_dialog_cocoa.mm | 4 +- chrome/browser/ui/cocoa/login_prompt_cocoa.mm | 4 +- .../one_click_signin_dialog_controller.mm | 6 +- ...rofile_signin_confirmation_dialog_cocoa.mm | 2 +- .../ui/cocoa/profiles/user_manager_mac.mm | 6 +- ...ingle_web_contents_dialog_manager_cocoa.mm | 4 +- .../ssl_client_certificate_selector_cocoa.mm | 8 +- .../ui/cocoa/tab_modal_confirm_dialog_mac.mm | 2 +- ...contents_modal_dialog_manager_views_mac.mm | 4 + .../constrained_web_dialog_delegate_base.cc | 5 + .../constrained_web_dialog_delegate_base.h | 5 + .../ui/webui/constrained_web_dialog_ui.h | 2 + .../constrained_web_dialog_ui_browsertest.cc | 13 +- 28 files changed, 316 insertions(+), 70 deletions(-) diff --git a/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm b/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm index 4a80bbde542f9..1731bb358a7bb 100644 --- a/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm +++ b/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm @@ -38,8 +38,8 @@ base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:[sheet_delegate_ window]]); - constrained_window_.reset( - new ConstrainedWindowMac(this, delegate_->GetWebContents(), sheet)); + constrained_window_ = + CreateAndShowWebModalDialogMac(this, delegate_->GetWebContents(), sheet); [sheet_delegate_ show]; } diff --git a/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm b/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm index 2a9d770ee7be4..b4f949e8622e4 100644 --- a/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm +++ b/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm @@ -87,8 +87,8 @@ [window setContentView:[view_controller_ view]]; base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]); - constrained_window_.reset( - new ConstrainedWindowMac(this, web_contents_, sheet)); + constrained_window_ = + CreateAndShowWebModalDialogMac(this, web_contents_, sheet); } void CardUnmaskPromptViewBridge::ControllerGone() { diff --git a/chrome/browser/ui/cocoa/certificate_viewer_mac.mm b/chrome/browser/ui/cocoa/certificate_viewer_mac.mm index 5c41b94f781af..1adb0f166bd86 100644 --- a/chrome/browser/ui/cocoa/certificate_viewer_mac.mm +++ b/chrome/browser/ui/cocoa/certificate_viewer_mac.mm @@ -134,8 +134,8 @@ - (void)displayForWebContents:(content::WebContents*)webContents { panel_.reset([[SFCertificatePanel alloc] init]); [panel_ setPolicies:(id) policies.get()]; - constrainedWindow_.reset( - new ConstrainedWindowMac(observer_.get(), webContents, self)); + constrainedWindow_ = + CreateAndShowWebModalDialogMac(observer_.get(), webContents, self); } - (NSWindow*)overlayWindow { @@ -189,6 +189,10 @@ - (void)updateSheetPosition { // NOOP } +- (void)resizeWithNewSize:(NSSize)preferredSize { + // NOOP +} + - (NSWindow*)sheetWindow { return panel_; } diff --git a/chrome/browser/ui/cocoa/constrained_web_dialog_delegate_mac.mm b/chrome/browser/ui/cocoa/constrained_web_dialog_delegate_mac.mm index 0ce4df8dabd21..74206ed239546 100644 --- a/chrome/browser/ui/cocoa/constrained_web_dialog_delegate_mac.mm +++ b/chrome/browser/ui/cocoa/constrained_web_dialog_delegate_mac.mm @@ -11,7 +11,11 @@ #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.h" #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h" #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.h" +#include "chrome/browser/ui/webui/chrome_web_contents_handler.h" +#include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" +#include "ui/base/cocoa/window_size_constants.h" #include "ui/gfx/geometry/size.h" #include "ui/web_dialogs/web_dialog_delegate.h" #include "ui/web_dialogs/web_dialog_ui.h" @@ -23,19 +27,61 @@ namespace { +class ConstrainedWebDialogDelegateMac; + +// This class is to trigger a resize to the dialog window when +// ResizeDueToAutoResize() is invoked. +class WebDialogWebContentsDelegateMac + : public ui::WebDialogWebContentsDelegate { + public: + WebDialogWebContentsDelegateMac(content::BrowserContext* browser_context, + content::WebContentsObserver* observer, + ConstrainedWebDialogDelegateBase* delegate) + : ui::WebDialogWebContentsDelegate(browser_context, + new ChromeWebContentsHandler()), + observer_(observer), + delegate_(delegate) { + } + ~WebDialogWebContentsDelegateMac() override {} + + void ResizeDueToAutoResize(content::WebContents* source, + const gfx::Size& preferred_size) override { + if (!observer_->web_contents()) + return; + delegate_->ResizeToGivenSize(preferred_size); + } + + private: + // These members must outlive the instance. + content::WebContentsObserver* const observer_; + ConstrainedWebDialogDelegateBase* delegate_; + + DISALLOW_COPY_AND_ASSIGN(WebDialogWebContentsDelegateMac); +}; + class ConstrainedWebDialogDelegateMac : public ConstrainedWebDialogDelegateBase { public: ConstrainedWebDialogDelegateMac( content::BrowserContext* browser_context, - WebDialogDelegate* delegate) - : ConstrainedWebDialogDelegateBase(browser_context, delegate, NULL) {} + WebDialogDelegate* delegate, + content::WebContentsObserver* observer) + : ConstrainedWebDialogDelegateBase(browser_context, delegate, + new WebDialogWebContentsDelegateMac(browser_context, observer, + this)) {} // WebDialogWebContentsDelegate interface. void CloseContents(WebContents* source) override { window_->CloseWebContentsModalDialog(); } + // ConstrainedWebDialogDelegateBase: + void ResizeToGivenSize(const gfx::Size size) override { + NSSize updated_preferred_size = NSMakeSize(size.width(), + size.height()); + [window_->sheet() resizeWithNewSize:updated_preferred_size]; + } + void set_window(ConstrainedWindowMac* window) { window_ = window; } ConstrainedWindowMac* window() const { return window_; } @@ -50,13 +96,16 @@ void CloseContents(WebContents* source) override { class ConstrainedWebDialogDelegateViewMac : public ConstrainedWindowMacDelegate, - public ConstrainedWebDialogDelegate { + public ConstrainedWebDialogDelegate, + public content::WebContentsObserver { public: ConstrainedWebDialogDelegateViewMac( content::BrowserContext* browser_context, WebDialogDelegate* delegate, - content::WebContents* web_contents); + content::WebContents* web_contents, + const gfx::Size& min_size, + const gfx::Size& max_size); ~ConstrainedWebDialogDelegateViewMac() override {} // ConstrainedWebDialogDelegate interface @@ -75,16 +124,37 @@ void ReleaseWebContentsOnDialogClose() override { gfx::NativeWindow GetNativeDialog() override { return window_; } WebContents* GetWebContents() override { return impl_->GetWebContents(); } gfx::Size GetMinimumSize() const override { - NOTIMPLEMENTED(); - return gfx::Size(); + return min_size_; } gfx::Size GetMaximumSize() const override { - NOTIMPLEMENTED(); - return gfx::Size(); + return max_size_; } gfx::Size GetPreferredSize() const override { - NOTIMPLEMENTED(); - return gfx::Size(); + gfx::Size size; + if (!impl_->closed_via_webui()) { + NSRect frame = [window_ frame]; + size = gfx::Size(frame.size.width, frame.size.height); + } + return size; + } + + // content::WebContentsObserver: + void RenderViewCreated(content::RenderViewHost* render_view_host) override { + if (IsDialogAutoResizable()) + EnableAutoResize(); + } + void RenderViewHostChanged(content::RenderViewHost* old_host, + content::RenderViewHost* new_host) override { + if (IsDialogAutoResizable()) + EnableAutoResize(); + } + void DocumentOnLoadCompletedInMainFrame() override { + if (!IsDialogAutoResizable()) + return; + + EnableAutoResize(); + if (GetWebContents()) + constrained_window_->ShowWebContentsModalDialog(); } // ConstrainedWindowMacDelegate interface @@ -95,25 +165,56 @@ void OnConstrainedWindowClosed(ConstrainedWindowMac* window) override { } private: + void EnableAutoResize() { + if (!GetWebContents()) + return; + + content::RenderViewHost* render_view_host = + GetWebContents()->GetRenderViewHost(); + render_view_host->EnableAutoResize(min_size_, max_size_); + } + + // Whether or not the dialog is autoresizable is determined based on whether + // |max_size_| was specified. + bool IsDialogAutoResizable() { + return !max_size_.IsEmpty(); + } + scoped_ptr impl_; scoped_ptr constrained_window_; base::scoped_nsobject window_; + // Minimum and maximum sizes to determine dialog bounds for auto-resizing. + const gfx::Size min_size_; + const gfx::Size max_size_; + DISALLOW_COPY_AND_ASSIGN(ConstrainedWebDialogDelegateViewMac); }; ConstrainedWebDialogDelegateViewMac::ConstrainedWebDialogDelegateViewMac( content::BrowserContext* browser_context, WebDialogDelegate* delegate, - content::WebContents* web_contents) - : impl_(new ConstrainedWebDialogDelegateMac(browser_context, delegate)) { + content::WebContents* web_contents, + const gfx::Size& min_size, + const gfx::Size& max_size) + : content::WebContentsObserver(web_contents), + impl_(new ConstrainedWebDialogDelegateMac(browser_context, delegate, + this)), + min_size_(min_size), + max_size_(max_size) { + if (IsDialogAutoResizable()) + Observe(GetWebContents()); + // Create a window to hold web_contents in the constrained sheet: gfx::Size size; delegate->GetDialogSize(&size); - NSRect frame = NSMakeRect(0, 0, size.width(), size.height()); + // The window size for autoresizing dialogs will be determined at a later + // time. + NSRect frame = IsDialogAutoResizable() ? ui::kWindowSizeDeterminedLater : + NSMakeRect(0, 0, size.width(), size.height()); - window_.reset( - [[ConstrainedWindowCustomWindow alloc] initWithContentRect:frame]); + window_.reset([[ConstrainedWindowCustomWindow alloc] + initWithContentRect:frame]); [GetWebContents()->GetNativeView() setFrame:frame]; [GetWebContents()->GetNativeView() setAutoresizingMask: NSViewWidthSizable|NSViewHeightSizable]; @@ -122,8 +223,14 @@ void OnConstrainedWindowClosed(ConstrainedWindowMac* window) override { base::scoped_nsobject sheet( [[WebDialogConstrainedWindowSheet alloc] initWithCustomWindow:window_ webDialogDelegate:delegate]); - constrained_window_.reset(new ConstrainedWindowMac( - this, web_contents, sheet)); + + if (IsDialogAutoResizable()) { + constrained_window_ = CreateWebModalDialogMac( + this, web_contents, sheet); + } else { + constrained_window_ = CreateAndShowWebModalDialogMac( + this, web_contents, sheet); + } impl_->set_window(constrained_window_.get()); } @@ -135,6 +242,23 @@ void OnConstrainedWindowClosed(ConstrainedWindowMac* window) override { // Deleted when the dialog closes. ConstrainedWebDialogDelegateViewMac* constrained_delegate = new ConstrainedWebDialogDelegateViewMac( - browser_context, delegate, web_contents); + browser_context, delegate, web_contents, + gfx::Size(), gfx::Size()); + return constrained_delegate; +} + +ConstrainedWebDialogDelegate* ShowConstrainedWebDialogWithAutoResize( + content::BrowserContext* browser_context, + WebDialogDelegate* delegate, + content::WebContents* web_contents, + const gfx::Size& min_size, + const gfx::Size& max_size) { + DCHECK(!min_size.IsEmpty()); + DCHECK(!max_size.IsEmpty()); + // Deleted when the dialog closes. + ConstrainedWebDialogDelegateViewMac* constrained_delegate = + new ConstrainedWebDialogDelegateViewMac( + browser_context, delegate, web_contents, + min_size, max_size); return constrained_delegate; } diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_sheet.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_sheet.mm index aac594676b41a..ab67fc6804b9d 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_sheet.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_sheet.mm @@ -73,6 +73,10 @@ - (void)updateSheetPosition { [customWindow_ setFrameOrigin:origin]; } +- (void)resizeWithNewSize:(NSSize)size { + // NOOP +} + - (NSWindow*)sheetWindow { return customWindow_; } diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h index e3d981edd473d..33aa9dd47efb0 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.h @@ -7,6 +7,9 @@ #import +#include "base/mac/scoped_nsobject.h" +#include "components/web_modal/web_contents_modal_dialog_manager.h" + namespace content { class WebContents; } @@ -21,6 +24,18 @@ class ConstrainedWindowMacDelegate { virtual void OnConstrainedWindowClosed(ConstrainedWindowMac* window) = 0; }; +// Creates a ConstrainedWindowMac, shows the dialog, and returns it. +scoped_ptr CreateAndShowWebModalDialogMac( + ConstrainedWindowMacDelegate* delegate, + content::WebContents* web_contents, + id sheet); + +// Creates a ConstrainedWindowMac and returns it. +scoped_ptr CreateWebModalDialogMac( + ConstrainedWindowMacDelegate* delegate, + content::WebContents* web_contents, + id sheet); + // 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. @@ -31,6 +46,9 @@ class ConstrainedWindowMac { id sheet); ~ConstrainedWindowMac(); + // Shows the constrained window. + void ShowWebContentsModalDialog(); + // Closes the constrained window. void CloseWebContentsModalDialog(); @@ -38,13 +56,24 @@ class ConstrainedWindowMac { void set_manager(SingleWebContentsDialogManagerCocoa* manager) { manager_ = manager; } + id sheet() const { return sheet_.get(); } // Called by |manager_| when the dialog is closing. void OnDialogClosing(); + // Whether or not the dialog was shown. If the dialog is auto-resizable, it + // is hidden until its WebContents initially loads. + bool DialogWasShown(); + + // Gets the dialog manager for |web_contents_|. + web_modal::WebContentsModalDialogManager* GetDialogManager(); + private: ConstrainedWindowMacDelegate* delegate_; // weak, owns us. SingleWebContentsDialogManagerCocoa* manager_; // weak, owned by WCMDM. + content::WebContents* web_contents_; // weak, owned by dialog initiator. + base::scoped_nsprotocol> sheet_; + scoped_ptr native_manager_; }; #endif // CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_MAC_ diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.mm index 16f755c1971d4..683eb689999da 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_mac.mm @@ -11,35 +11,59 @@ #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet.h" #import "chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.h" #include "components/guest_view/browser/guest_view_base.h" -#include "components/web_modal/web_contents_modal_dialog_manager.h" #include "content/public/browser/browser_thread.h" using web_modal::WebContentsModalDialogManager; +scoped_ptr CreateAndShowWebModalDialogMac( + ConstrainedWindowMacDelegate* delegate, + content::WebContents* web_contents, + id sheet) { + ConstrainedWindowMac* window = + new ConstrainedWindowMac(delegate, web_contents, sheet); + window->ShowWebContentsModalDialog(); + return scoped_ptr(window); +} + +scoped_ptr CreateWebModalDialogMac( + ConstrainedWindowMacDelegate* delegate, + content::WebContents* web_contents, + id sheet) { + return scoped_ptr( + new ConstrainedWindowMac(delegate, web_contents, sheet)); +} + ConstrainedWindowMac::ConstrainedWindowMac( ConstrainedWindowMacDelegate* delegate, content::WebContents* web_contents, id sheet) - : delegate_(delegate) { + : delegate_(delegate), + sheet_([sheet retain]) { DCHECK(sheet); // |web_contents| may be embedded within a chain of nested GuestViews. If it // is, follow the chain of embedders to the outermost WebContents and use it. - web_contents = + web_contents_ = guest_view::GuestViewBase::GetTopLevelWebContents(web_contents); - auto manager = WebContentsModalDialogManager::FromWebContents(web_contents); - scoped_ptr native_manager( - new SingleWebContentsDialogManagerCocoa(this, sheet, manager)); - manager->ShowDialogWithManager([sheet sheetWindow], - std::move(native_manager)); + native_manager_.reset( + new SingleWebContentsDialogManagerCocoa(this, sheet_.get(), + GetDialogManager())); } ConstrainedWindowMac::~ConstrainedWindowMac() { CHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + native_manager_.reset(); DCHECK(!manager_); } +void ConstrainedWindowMac::ShowWebContentsModalDialog() { + scoped_ptr dialog_manager; + dialog_manager.reset(native_manager_.release()); + GetDialogManager()->ShowDialogWithManager( + [sheet_.get() sheetWindow], std::move(dialog_manager)); +} + void ConstrainedWindowMac::CloseWebContentsModalDialog() { if (manager_) manager_->Close(); @@ -49,3 +73,13 @@ if (delegate_) delegate_->OnConstrainedWindowClosed(this); } + +bool ConstrainedWindowMac::DialogWasShown() { + // If the dialog was shown, |native_manager_| would have been released. + return !native_manager_; +} + +WebContentsModalDialogManager* ConstrainedWindowMac::GetDialogManager() { + DCHECK(web_contents_); + return WebContentsModalDialogManager::FromWebContents(web_contents_); +} 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 8e6cedab97c2f..48e48a4446928 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 @@ -79,14 +79,15 @@ void SetUpOnMainThread() override { IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, ShowInInactiveTab) { // Show dialog in non active tab. NiceMock delegate; - ConstrainedWindowMac dialog(&delegate, tab0_, sheet_); + scoped_ptr dialog = + CreateAndShowWebModalDialogMac(&delegate, tab0_, sheet_); EXPECT_EQ(0.0, [sheet_window_ alphaValue]); // Switch to inactive tab. browser()->tab_strip_model()->ActivateTabAt(0, true); EXPECT_EQ(1.0, [sheet_window_ alphaValue]); - dialog.CloseWebContentsModalDialog(); + dialog->CloseWebContentsModalDialog(); } // If a tab has never been shown then the associated tab view for the web @@ -105,7 +106,8 @@ void SetUpOnMainThread() override { // Show dialog and verify that it's not visible yet. NiceMock delegate; - ConstrainedWindowMac dialog(&delegate, tab2, sheet_); + scoped_ptr dialog = + CreateAndShowWebModalDialogMac(&delegate, tab2, sheet_); EXPECT_FALSE([sheet_window_ isVisible]); // Activate the tab and verify that the constrained window is shown. @@ -114,25 +116,27 @@ void SetUpOnMainThread() override { EXPECT_TRUE([sheet_window_ isVisible]); EXPECT_EQ(1.0, [sheet_window_ alphaValue]); - dialog.CloseWebContentsModalDialog(); + dialog->CloseWebContentsModalDialog(); } // Test that adding a sheet disables tab dragging. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, TabDragging) { NiceMock delegate; - ConstrainedWindowMac dialog(&delegate, tab1_, sheet_); + scoped_ptr dialog = + CreateAndShowWebModalDialogMac(&delegate, tab1_, sheet_); // Verify that the dialog disables dragging. EXPECT_TRUE([controller_ isTabDraggable:tab_view0_]); EXPECT_FALSE([controller_ isTabDraggable:tab_view1_]); - dialog.CloseWebContentsModalDialog(); + dialog->CloseWebContentsModalDialog(); } // Test that closing a browser window with a sheet works. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, BrowserWindowClose) { NiceMock delegate; - ConstrainedWindowMac dialog(&delegate, tab1_, sheet_); + scoped_ptr dialog( + CreateAndShowWebModalDialogMac(&delegate, tab1_, sheet_)); EXPECT_EQ(1.0, [sheet_window_ alphaValue]); // Close the browser window. @@ -146,7 +150,8 @@ void SetUpOnMainThread() override { // Test that closing a tab with a sheet works. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, TabClose) { NiceMock delegate; - ConstrainedWindowMac dialog(&delegate, tab1_, sheet_); + scoped_ptr dialog( + CreateAndShowWebModalDialogMac(&delegate, tab1_, sheet_)); EXPECT_EQ(1.0, [sheet_window_ alphaValue]); // Close the tab. @@ -160,7 +165,8 @@ void SetUpOnMainThread() override { // Test that the sheet is still visible after entering and exiting fullscreen. IN_PROC_BROWSER_TEST_F(ConstrainedWindowMacTest, BrowserWindowFullscreen) { NiceMock delegate; - ConstrainedWindowMac dialog(&delegate, tab1_, sheet_); + scoped_ptr dialog( + CreateAndShowWebModalDialogMac(&delegate, tab1_, sheet_)); EXPECT_EQ(1.0, [sheet_window_ alphaValue]); // Toggle fullscreen twice: one for entering and one for exiting. @@ -180,5 +186,5 @@ void SetUpOnMainThread() override { EXPECT_EQ(1.0, [sheet_window_ alphaValue]); } - dialog.CloseWebContentsModalDialog(); + dialog->CloseWebContentsModalDialog(); } diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet.h b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet.h index 45495564d8e55..cf24f7da52b1b 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet.h +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet.h @@ -24,6 +24,8 @@ - (void)updateSheetPosition; +- (void)resizeWithNewSize:(NSSize)size; + @property(readonly, nonatomic) NSWindow* sheetWindow; @end 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 05038cf70fc87..6566d21d404e6 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 @@ -59,6 +59,10 @@ - (void)makeSheetKeyAndOrderFront { - (void)updateSheetPosition { } +- (void)resizeWithNewSize:(NSSize)size { + // NOOP +} + - (NSWindow*)sheetWindow { return [alert_ window]; } diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.h b/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.h index 57ba754af0e04..7c37e234605e8 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.h +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.h @@ -14,6 +14,7 @@ class WebDialogDelegate; // Represents a custom sheet for web dialog. @interface WebDialogConstrainedWindowSheet : CustomConstrainedWindowSheet { @private + NSSize current_size_; ui::WebDialogDelegate* web_dialog_delegate_; // Weak. } diff --git a/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.mm b/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.mm index 3c71f00e0a190..775ae15466549 100644 --- a/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.mm +++ b/chrome/browser/ui/cocoa/constrained_window/constrained_window_web_dialog_sheet.mm @@ -22,9 +22,25 @@ - (void)updateSheetPosition { if (web_dialog_delegate_) { gfx::Size size; web_dialog_delegate_->GetDialogSize(&size); - [customWindow_ setContentSize:NSMakeSize(size.width(), size.height())]; + + // If the dialog has autoresizing enabled, |size| will be empty. Use the + // last known dialog size. + NSSize content_size = size.IsEmpty() ? current_size_ : + NSMakeSize(size.width(), size.height()); + [customWindow_ setContentSize:content_size]; } [super updateSheetPosition]; } +- (void)resizeWithNewSize:(NSSize)size { + current_size_ = size; + [customWindow_ setContentSize:current_size_]; + + // self's updateSheetPosition() sets |customWindow_|'s contentSize to a + // fixed dialog size. Here, we want to resize to |size| instead. Use + // super rather than self to bypass the setContentSize() call for the fixed + // size. + [super updateSheetPosition]; +} + @end diff --git a/chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.mm b/chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.mm index 679c21283ed51..7c85ffe8827ab 100644 --- a/chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.mm +++ b/chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.mm @@ -66,8 +66,7 @@ base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:[sheet_controller_ window]]); - window_.reset(new ConstrainedWindowMac( - this, web_contents, sheet)); + window_ = CreateAndShowWebModalDialogMac(this, web_contents, sheet); } CollectedCookiesMac::~CollectedCookiesMac() { diff --git a/chrome/browser/ui/cocoa/extensions/device_permissions_dialog_controller.mm b/chrome/browser/ui/cocoa/extensions/device_permissions_dialog_controller.mm index 6babaf28c4f36..881aff0fe092b 100644 --- a/chrome/browser/ui/cocoa/extensions/device_permissions_dialog_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/device_permissions_dialog_controller.mm @@ -33,8 +33,8 @@ base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]); - constrained_window_.reset( - new ConstrainedWindowMac(this, web_contents, sheet)); + constrained_window_ = + CreateAndShowWebModalDialogMac(this, web_contents, sheet); } DevicePermissionsDialogController::~DevicePermissionsDialogController() { 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 826a3bcf201bd..4685a25b07c48 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_install_dialog_controller.mm @@ -60,8 +60,8 @@ new ExtensionInstallDialogController(show_params, done_callback, base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]); - constrained_window_.reset(new ConstrainedWindowMac( - this, show_params->GetParentWebContents(), sheet)); + constrained_window_ = CreateAndShowWebModalDialogMac( + this, show_params->GetParentWebContents(), sheet); std::string event_name = ExperienceSamplingEvent::kExtensionInstallDialog; event_name.append( diff --git a/chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.mm b/chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.mm index 5dd89494fa108..37b47da105587 100644 --- a/chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.mm +++ b/chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.mm @@ -102,8 +102,8 @@ - (void)onAuxiliaryButton:(id)sender { base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:[alert_ window]]); - window_.reset(new ConstrainedWindowMac( - this, controller->WebContents(), sheet)); + window_ = CreateAndShowWebModalDialogMac( + this, controller->WebContents(), sheet); } } diff --git a/chrome/browser/ui/cocoa/login_prompt_cocoa.mm b/chrome/browser/ui/cocoa/login_prompt_cocoa.mm index 2c24253b33472..cd1a8443d4d47 100644 --- a/chrome/browser/ui/cocoa/login_prompt_cocoa.mm +++ b/chrome/browser/ui/cocoa/login_prompt_cocoa.mm @@ -80,8 +80,8 @@ void BuildViewImpl(const base::string16& authority, base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:[sheet_controller_ window]]); - constrained_window_.reset(new ConstrainedWindowMac( - this, requesting_contents, sheet)); + constrained_window_ = CreateAndShowWebModalDialogMac( + this, requesting_contents, sheet); NotifyAuthNeeded(); } diff --git a/chrome/browser/ui/cocoa/one_click_signin_dialog_controller.mm b/chrome/browser/ui/cocoa/one_click_signin_dialog_controller.mm index f0156f2b4e7a1..a8f9a846e5428 100644 --- a/chrome/browser/ui/cocoa/one_click_signin_dialog_controller.mm +++ b/chrome/browser/ui/cocoa/one_click_signin_dialog_controller.mm @@ -30,15 +30,15 @@ base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]); - constrained_window_.reset(new ConstrainedWindowMac( - this, web_contents, sheet)); + constrained_window_ = + CreateAndShowWebModalDialogMac(this, web_contents, sheet); } OneClickSigninDialogController::~OneClickSigninDialogController() { } void OneClickSigninDialogController::OnConstrainedWindowClosed( - ConstrainedWindowMac* window) { + ConstrainedWindowMac* window) { [view_controller_ viewWillClose]; base::MessageLoop::current()->DeleteSoon(FROM_HERE, this); } diff --git a/chrome/browser/ui/cocoa/profiles/profile_signin_confirmation_dialog_cocoa.mm b/chrome/browser/ui/cocoa/profiles/profile_signin_confirmation_dialog_cocoa.mm index a46b4ebea4857..e600717423929 100644 --- a/chrome/browser/ui/cocoa/profiles/profile_signin_confirmation_dialog_cocoa.mm +++ b/chrome/browser/ui/cocoa/profiles/profile_signin_confirmation_dialog_cocoa.mm @@ -59,7 +59,7 @@ new ProfileSigninConfirmationDialogCocoa(browser, [[window contentView] addSubview:[controller_ view]]; base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]); - window_.reset(new ConstrainedWindowMac(this, web_contents, sheet)); + window_ = CreateAndShowWebModalDialogMac(this, web_contents, sheet); } ProfileSigninConfirmationDialogCocoa::~ProfileSigninConfirmationDialogCocoa() { diff --git a/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm b/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm index 48a02249c65b0..d7536b72a7095 100644 --- a/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm +++ b/chrome/browser/ui/cocoa/profiles/user_manager_mac.mm @@ -203,9 +203,9 @@ - (id)initWithProfile:(Profile*)profile base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:[self window]]); - constrained_window_.reset( - new ConstrainedWindowMac( - webContentsDelegate_.get(), webContents_, sheet)); + constrained_window_ = + CreateAndShowWebModalDialogMac( + webContentsDelegate_.get(), webContents_, sheet); // The close button needs to call CloseWebContentsModalDialog() on the // constrained window isntead of just [window close] so grab a reference to diff --git a/chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.mm b/chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.mm index 6b1d507a741cf..4bab88226e163 100644 --- a/chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.mm +++ b/chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.mm @@ -58,8 +58,10 @@ [[ConstrainedWindowSheetController controllerForSheet:sheet_] closeSheet:sheet_]; client_->set_manager(nullptr); + bool dialog_was_open = client_->DialogWasShown(); client_->OnDialogClosing(); // |client_| might delete itself here. - delegate_->WillClose(dialog()); // Deletes |this|. + if (dialog_was_open) + delegate_->WillClose(dialog()); // Deletes |this|. } void SingleWebContentsDialogManagerCocoa::Focus() { diff --git a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm index 8c7d66db57ab6..4a06708658991 100644 --- a/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm +++ b/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa.mm @@ -179,8 +179,8 @@ - (void)displayForWebContents:(content::WebContents*)webContents { CFRelease(sslPolicy); } - constrainedWindow_.reset( - new ConstrainedWindowMac(observer_.get(), webContents, self)); + constrainedWindow_ = + CreateAndShowWebModalDialogMac(observer_.get(), webContents, self); observer_->StartObserving(); } @@ -260,6 +260,10 @@ - (void)updateSheetPosition { // NOOP } +- (void)resizeWithNewSize:(NSSize)size { + // NOOP +} + - (NSWindow*)sheetWindow { return panel_; } 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 6935ce1ac530b..e0e7f54765732 100644 --- a/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm +++ b/chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm @@ -94,7 +94,7 @@ - (void)onLinkClicked:(id)sender { base::scoped_nsobject sheet( [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:[alert_ window]]); - window_.reset(new ConstrainedWindowMac(this, web_contents, sheet)); + window_ = CreateAndShowWebModalDialogMac(this, web_contents, sheet); delegate_->set_close_delegate(this); } diff --git a/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac.mm b/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac.mm index 1eefbd150a7e2..31f057fa9e603 100644 --- a/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac.mm +++ b/chrome/browser/ui/cocoa/web_contents_modal_dialog_manager_views_mac.mm @@ -96,6 +96,10 @@ - (void)updateSheetPosition { [customWindow_ setFrameOrigin:origin]; } +- (void)resizeWithNewSize:(NSSize)size { + // NOOP +} + - (NSWindow*)sheetWindow { return customWindow_; } diff --git a/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc index fd60a4b0d6cce..db4c5476bd222 100644 --- a/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc +++ b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc @@ -113,3 +113,8 @@ gfx::Size ConstrainedWebDialogDelegateBase::GetPreferredSize() const { NOTREACHED(); return gfx::Size(); } + +void ConstrainedWebDialogDelegateBase::ResizeToGivenSize( + const gfx::Size size) { + NOTREACHED(); +} diff --git a/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h index fee49310d0879..a43814ab5598f 100644 --- a/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h +++ b/chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h @@ -24,6 +24,8 @@ class ConstrainedWebDialogDelegateBase : public ConstrainedWebDialogDelegate, public ui::WebDialogWebContentsDelegate { public: + // |browser_context| and |delegate| must outlive |this| instance, whereas + // |this| will take ownership of |tab_delegate|. ConstrainedWebDialogDelegateBase(content::BrowserContext* browser_context, ui::WebDialogDelegate* delegate, WebDialogWebContentsDelegate* tab_delegate); @@ -47,6 +49,9 @@ class ConstrainedWebDialogDelegateBase content::WebContents* source, const content::NativeWebKeyboardEvent& event) override; + // Resize the dialog to the given size. + virtual void ResizeToGivenSize(const gfx::Size size); + private: scoped_ptr web_dialog_delegate_; diff --git a/chrome/browser/ui/webui/constrained_web_dialog_ui.h b/chrome/browser/ui/webui/constrained_web_dialog_ui.h index 4f7159e5804a4..39cc74bca4124 100644 --- a/chrome/browser/ui/webui/constrained_web_dialog_ui.h +++ b/chrome/browser/ui/webui/constrained_web_dialog_ui.h @@ -51,6 +51,8 @@ class ConstrainedWebDialogDelegate { // Returns the maximum size for the dialog. virtual gfx::Size GetMaximumSize() const = 0; + // Returns the preferred size for the dialog, or an empty size if + // the dialog has been closed. virtual gfx::Size GetPreferredSize() const = 0; protected: diff --git a/chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc b/chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc index fbdf04f841ae2..c3c0f36155bc4 100644 --- a/chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc +++ b/chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc @@ -26,7 +26,6 @@ using web_modal::WebContentsModalDialogManager; namespace { -#if !defined(OS_MACOSX) static const char kTestDataURL[] = "data:text/html," "" "