Skip to content

Commit

Permalink
[Reland] Enable AutoResize for Constrained Web Dialogs for Mac.
Browse files Browse the repository at this point in the history
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 740c410)

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

Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#218}
Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
  • Loading branch information
japacible committed Mar 14, 2016
1 parent a1dbf58 commit cc82582
Show file tree
Hide file tree
Showing 28 changed files with 316 additions and 70 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
base::scoped_nsobject<CustomConstrainedWindowSheet> 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];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@
[window setContentView:[view_controller_ view]];
base::scoped_nsobject<CustomConstrainedWindowSheet> sheet(
[[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]);
constrained_window_.reset(
new ConstrainedWindowMac(this, web_contents_, sheet));
constrained_window_ =
CreateAndShowWebModalDialogMac(this, web_contents_, sheet);
}

void CardUnmaskPromptViewBridge::ControllerGone() {
Expand Down
8 changes: 6 additions & 2 deletions chrome/browser/ui/cocoa/certificate_viewer_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -189,6 +189,10 @@ - (void)updateSheetPosition {
// NOOP
}

- (void)resizeWithNewSize:(NSSize)preferredSize {
// NOOP
}

- (NSWindow*)sheetWindow {
return panel_;
}
Expand Down
160 changes: 142 additions & 18 deletions chrome/browser/ui/cocoa/constrained_web_dialog_delegate_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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_; }

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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<ConstrainedWebDialogDelegateMac> impl_;
scoped_ptr<ConstrainedWindowMac> constrained_window_;
base::scoped_nsobject<NSWindow> 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];
Expand All @@ -122,8 +223,14 @@ void OnConstrainedWindowClosed(ConstrainedWindowMac* window) override {
base::scoped_nsobject<WebDialogConstrainedWindowSheet> 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());
}
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ - (void)updateSheetPosition {
[customWindow_ setFrameOrigin:origin];
}

- (void)resizeWithNewSize:(NSSize)size {
// NOOP
}

- (NSWindow*)sheetWindow {
return customWindow_;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

#import <Cocoa/Cocoa.h>

#include "base/mac/scoped_nsobject.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h"

namespace content {
class WebContents;
}
Expand All @@ -21,6 +24,18 @@ class ConstrainedWindowMacDelegate {
virtual void OnConstrainedWindowClosed(ConstrainedWindowMac* window) = 0;
};

// Creates a ConstrainedWindowMac, shows the dialog, and returns it.
scoped_ptr<ConstrainedWindowMac> CreateAndShowWebModalDialogMac(
ConstrainedWindowMacDelegate* delegate,
content::WebContents* web_contents,
id<ConstrainedWindowSheet> sheet);

// Creates a ConstrainedWindowMac and returns it.
scoped_ptr<ConstrainedWindowMac> CreateWebModalDialogMac(
ConstrainedWindowMacDelegate* delegate,
content::WebContents* web_contents,
id<ConstrainedWindowSheet> 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.
Expand All @@ -31,20 +46,34 @@ class ConstrainedWindowMac {
id<ConstrainedWindowSheet> sheet);
~ConstrainedWindowMac();

// Shows the constrained window.
void ShowWebContentsModalDialog();

// Closes the constrained window.
void CloseWebContentsModalDialog();

SingleWebContentsDialogManagerCocoa* manager() const { return manager_; }
void set_manager(SingleWebContentsDialogManagerCocoa* manager) {
manager_ = manager;
}
id<ConstrainedWindowSheet> 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<id<ConstrainedWindowSheet>> sheet_;
scoped_ptr<SingleWebContentsDialogManagerCocoa> native_manager_;
};

#endif // CHROME_BROWSER_UI_COCOA_CONSTRAINED_WINDOW_CONSTRAINED_WINDOW_MAC_
Loading

0 comments on commit cc82582

Please sign in to comment.