Commit 8fe1291a authored by Hitoshi Yoshida's avatar Hitoshi Yoshida Committed by Commit Bot

Revert "Don't use constrained dialogs for macOS certificate viewer."

This reverts commit 662e18cb.

Reason for revert: This CL seems to be a trigger of a test failure on Mac.

Bug: 1110172

Original change's description:
> Don't use constrained dialogs for macOS certificate viewer.
> 
> Instead parent the sheet to the specified gfx::NativeWindow directly.
> This avoids a host of bugs around the native constrained dialogs
> machinery, but does mean the dialog is now window-modal rather than
> tab-modal. On Windows, where we also use the native dialog, the sheet is
> similarly window-modal and the certificate viewer only ever opens in
> response to the user clicking in browser-owned UI or DevTools, so there
> shouldn't be an abuse concern.
> 
> See https://crbug.com/1098786#c4
> 
> Bug: 762915, 1078158, 1098786
> Change-Id: Iafeee58fd04aff9f8b88e8c2f030fe18913f81f2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2313160
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: David Benjamin <davidben@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#791907}

TBR=ellyjones@chromium.org,davidben@chromium.org

Change-Id: I8e29389c8a609daeed12b13d50b3d8d27f8959d6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 762915
Bug: 1078158
Bug: 1098786
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2320913Reviewed-by: default avatarHitoshi Yoshida <peria@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792133}
parent 442d8319
...@@ -5,37 +5,67 @@ ...@@ -5,37 +5,67 @@
#import <Cocoa/Cocoa.h> #import <Cocoa/Cocoa.h>
#import <SecurityInterface/SFCertificatePanel.h> #import <SecurityInterface/SFCertificatePanel.h>
#include "base/bind.h"
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#include "base/mac/scoped_cftyperef.h" #include "base/mac/scoped_cftyperef.h"
#import "base/mac/scoped_nsobject.h" #import "base/mac/scoped_nsobject.h"
#include "base/notreached.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/certificate_viewer.h" #include "chrome/browser/certificate_viewer.h"
#include "components/constrained_window/constrained_window_views.h"
#include "components/remote_cocoa/browser/window.h" #include "components/remote_cocoa/browser/window.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h"
#include "content/public/browser/web_contents.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
#include "net/cert/x509_util_ios_and_mac.h" #include "net/cert/x509_util_ios_and_mac.h"
#include "net/cert/x509_util_mac.h" #include "net/cert/x509_util_mac.h"
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"
@interface SFCertificatePanel (SystemPrivate)
// A system-private interface that dismisses a panel whose sheet was started by
// -beginSheetForWindow:
// modalDelegate:
// didEndSelector:
// contextInfo:
// certificates:
// showGroup:
// as though the user clicked the button identified by returnCode. Verified
// present in 10.8.
- (void)_dismissWithCode:(NSInteger)code;
@end
@interface SSLCertificateViewerMac : NSObject @interface SSLCertificateViewerMac : NSObject
// Initializes |certificates_| with the certificate chain for a given // Initializes |certificates_| with the certificate chain for a given
// certificate. // certificate.
- (instancetype)initWithCertificate:(net::X509Certificate*)certificate; - (instancetype)initWithCertificate:(net::X509Certificate*)certificate
forWebContents:(content::WebContents*)webContents;
// Shows the certificate viewer as a Cocoa sheet. // Shows the certificate viewer as a Cocoa sheet.
- (void)showCertificateSheet:(NSWindow*)window; - (void)showCertificateSheet:(NSWindow*)window;
// Called when the certificate viewer Cocoa sheet is closed. // Closes the certificate viewer sheet.
- (void)closeCertificateSheet;
- (void)setOverlayWindow:(views::Widget*)overlayWindow;
// Closes the certificate viewer Cocoa sheet.
- (void)sheetDidEnd:(NSWindow*)parent - (void)sheetDidEnd:(NSWindow*)parent
returnCode:(NSInteger)returnCode returnCode:(NSInteger)returnCode
context:(void*)context; context:(void*)context;
@end @end
@implementation SSLCertificateViewerMac { @implementation SSLCertificateViewerMac {
// The corresponding list of certificates.
base::scoped_nsobject<NSArray> _certificates; base::scoped_nsobject<NSArray> _certificates;
base::scoped_nsobject<SFCertificatePanel> _panel; base::scoped_nsobject<SFCertificatePanel> _panel;
NSWindow* _remoteViewsCloneWindow;
// Invisible overlay window used to block interaction with the tab underneath.
views::Widget* _overlayWindow;
} }
- (instancetype)initWithCertificate:(net::X509Certificate*)certificate { - (instancetype)initWithCertificate:(net::X509Certificate*)certificate
forWebContents:(content::WebContents*)webContents {
if ((self = [super init])) { if ((self = [super init])) {
base::ScopedCFTypeRef<CFArrayRef> certChain( base::ScopedCFTypeRef<CFArrayRef> certChain(
net::x509_util::CreateSecCertificateArrayForX509Certificate( net::x509_util::CreateSecCertificateArrayForX509Certificate(
...@@ -89,15 +119,6 @@ ...@@ -89,15 +119,6 @@
} }
- (void)showCertificateSheet:(NSWindow*)window { - (void)showCertificateSheet:(NSWindow*)window {
// TODO(https://crbug.com/913303): The certificate viewer's interface to
// Cocoa should be wrapped in a mojo interface in order to allow
// instantiating across processes. As a temporary solution, create a
// transparent in-process window to the front.
if (remote_cocoa::IsWindowRemote(window)) {
_remoteViewsCloneWindow =
remote_cocoa::CreateInProcessTransparentClone(window);
window = _remoteViewsCloneWindow;
}
[_panel beginSheetForWindow:window [_panel beginSheetForWindow:window
modalDelegate:self modalDelegate:self
didEndSelector:@selector(sheetDidEnd:returnCode:context:) didEndSelector:@selector(sheetDidEnd:returnCode:context:)
...@@ -106,20 +127,83 @@ ...@@ -106,20 +127,83 @@
showGroup:YES]; showGroup:YES];
} }
- (void)closeCertificateSheet {
// Closing the sheet using -[NSApp endSheet:] doesn't work so use the private
// method. If the sheet is already closed then this is a call on nil and thus
// a no-op.
[_panel _dismissWithCode:NSModalResponseCancel];
}
- (void)sheetDidEnd:(NSWindow*)parent - (void)sheetDidEnd:(NSWindow*)parent
returnCode:(NSInteger)returnCode returnCode:(NSInteger)returnCode
context:(void*)context { context:(void*)context {
[_remoteViewsCloneWindow close]; _overlayWindow->Close(); // Asynchronously releases |self|.
_remoteViewsCloneWindow = nil;
_panel.reset(); _panel.reset();
} }
- (void)setOverlayWindow:(views::Widget*)overlayWindow {
_overlayWindow = overlayWindow;
}
@end @end
namespace {
// A fully transparent, borderless web-modal dialog used to display the
// OS-provided window-modal sheet that displays certificate information.
class CertificateAnchorWidgetDelegate : public views::WidgetDelegateView {
public:
CertificateAnchorWidgetDelegate(content::WebContents* web_contents,
net::X509Certificate* cert)
: certificate_viewer_([[SSLCertificateViewerMac alloc]
initWithCertificate:cert
forWebContents:web_contents]) {
constrained_window::ShowWebModalDialogWithOverlayViews(
this, web_contents,
base::BindOnce(&CertificateAnchorWidgetDelegate::ShowSheet,
weak_factory_.GetWeakPtr()));
}
~CertificateAnchorWidgetDelegate() override {
// Note that the SFCertificatePanel takes a reference to its delegate in its
// -beginSheetForWindow:... method (bad SFCertificatePanel!) so break the
// retain cycle by explicitly canceling the dialog.
[certificate_viewer_ closeCertificateSheet];
[remote_views_clone_window_ close];
}
// WidgetDelegate:
ui::ModalType GetModalType() const override { return ui::MODAL_TYPE_CHILD; }
private:
void ShowSheet(views::Widget* overlay_window) {
NSWindow* overlay_ns_window =
overlay_window->GetNativeWindow().GetNativeNSWindow();
// TODO(https://crbug.com/913303): The certificate viewer's interface to
// Cocoa should be wrapped in a mojo interface in order to allow
// instantiating across processes. As a temporary solution, create a
// transparent in-process window to the front.
if (remote_cocoa::IsWindowRemote(overlay_ns_window)) {
remote_views_clone_window_ =
remote_cocoa::CreateInProcessTransparentClone(overlay_ns_window);
overlay_ns_window = remote_views_clone_window_;
}
[certificate_viewer_ showCertificateSheet:overlay_ns_window];
[certificate_viewer_ setOverlayWindow:overlay_window];
}
base::scoped_nsobject<SSLCertificateViewerMac> certificate_viewer_;
NSWindow* remote_views_clone_window_ = nil;
base::WeakPtrFactory<CertificateAnchorWidgetDelegate> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(CertificateAnchorWidgetDelegate);
};
} // namespace
void ShowCertificateViewer(content::WebContents* web_contents, void ShowCertificateViewer(content::WebContents* web_contents,
gfx::NativeWindow parent, gfx::NativeWindow parent,
net::X509Certificate* cert) { net::X509Certificate* cert) {
base::scoped_nsobject<SSLCertificateViewerMac> viewer( // Shows a new widget, which owns the delegate.
[[SSLCertificateViewerMac alloc] initWithCertificate:cert]); new CertificateAnchorWidgetDelegate(web_contents, cert);
[viewer showCertificateSheet:parent.GetNativeNSWindow()];
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment