Commit 22e1e692 authored by David Benjamin's avatar David Benjamin Committed by Commit Bot

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

This relands
https://chromium-review.googlesource.com/c/chromium/src/+/2313160 which
was reverted in
https://chromium-review.googlesource.com/c/chromium/src/+/2320913 due to
some test failures.

This CL just removes the offending tests. HideShow is no longer
applicable for a window-modal dialog, and Basic no longer works now that
we're using a plain OS sheet instead of WebContentsModalDialogManager
and some SPI to dismiss the sheet.

Bug: 762915, 1078158, 1098786, 1110172
Change-Id: Ic3c128a6f2eea0ae3f403c778e2d44444e3aa25f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2324112Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793186}
parent e295c833
// Copyright (c) 2013 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 <Cocoa/Cocoa.h>
#include "chrome/browser/certificate_viewer.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/web_modal/web_contents_modal_dialog_manager.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test.h"
#include "net/cert/x509_certificate.h"
#include "net/test/cert_test_util.h"
#include "net/test/test_data_directory.h"
#import "testing/gtest_mac.h"
#include "ui/base/cocoa/window_size_constants.h"
using web_modal::WebContentsModalDialogManager;
using SSLCertificateViewerMacTest = InProcessBrowserTest;
namespace {
scoped_refptr<net::X509Certificate> GetSampleCertificate() {
return net::ImportCertFromFile(net::GetTestCertsDirectory(),
"mit.davidben.der");
}
void CheckCertificateViewerVisibility(NSWindow* overlay_window,
NSWindow* dialog_sheet,
bool visible) {
CGFloat alpha = visible ? 1.0 : 0.0;
BOOL ignore_events = visible ? NO : YES;
SCOPED_TRACE(testing::Message() << "visible=" << visible);
// The overlay window underneath the certificate viewer should block mouse
// events only if the certificate viewer is visible.
EXPECT_EQ(ignore_events, [overlay_window ignoresMouseEvents]);
// Check certificate viewer sheet visibility and if it accepts mouse events.
EXPECT_EQ(alpha, [dialog_sheet alphaValue]);
EXPECT_EQ(ignore_events, [dialog_sheet ignoresMouseEvents]);
}
} // namespace
IN_PROC_BROWSER_TEST_F(SSLCertificateViewerMacTest, Basic) {
scoped_refptr<net::X509Certificate> cert = GetSampleCertificate();
ASSERT_TRUE(cert.get());
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
NSWindow* window =
web_contents->GetTopLevelNativeWindow().GetNativeNSWindow();
WebContentsModalDialogManager* web_contents_modal_dialog_manager =
WebContentsModalDialogManager::FromWebContents(web_contents);
EXPECT_FALSE(web_contents_modal_dialog_manager->IsDialogActive());
ShowCertificateViewer(web_contents, window, cert.get());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(web_contents_modal_dialog_manager->IsDialogActive());
WebContentsModalDialogManager::TestApi test_api(
web_contents_modal_dialog_manager);
test_api.CloseAllDialogs();
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(web_contents_modal_dialog_manager->IsDialogActive());
}
// Test that switching to another tab correctly hides the sheet.
IN_PROC_BROWSER_TEST_F(SSLCertificateViewerMacTest, HideShow) {
scoped_refptr<net::X509Certificate> cert = GetSampleCertificate();
ASSERT_TRUE(cert.get());
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
NSWindow* window =
web_contents->GetTopLevelNativeWindow().GetNativeNSWindow();
WebContentsModalDialogManager* web_contents_modal_dialog_manager =
WebContentsModalDialogManager::FromWebContents(web_contents);
// Account for any child windows that might be present before the certificate
// viewer is open.
NSUInteger num_child_windows = [[window childWindows] count];
ShowCertificateViewer(web_contents, window, cert.get());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(web_contents_modal_dialog_manager->IsDialogActive());
EXPECT_EQ(num_child_windows + 1, [[window childWindows] count]);
// Assume the last child is the overlay window that was added.
NSWindow* overlay_window = [[window childWindows] lastObject];
NSWindow* dialog_sheet = [overlay_window attachedSheet];
EXPECT_TRUE(dialog_sheet);
NSRect sheet_frame = [dialog_sheet frame];
// Verify the certificate viewer is showing and accepts mouse events.
CheckCertificateViewerVisibility(overlay_window, dialog_sheet, true);
// Switch to another tab and verify that |overlay_window| and |dialog_sheet|
// are not blocking mouse events, and |dialog_sheet| is hidden.
AddBlankTabAndShow(browser());
CheckCertificateViewerVisibility(overlay_window, dialog_sheet, false);
EXPECT_NSEQ(sheet_frame, [dialog_sheet frame]);
// Switch back and verify that the sheet is shown.
chrome::SelectNumberedTab(browser(), 0);
base::RunLoop().RunUntilIdle();
CheckCertificateViewerVisibility(overlay_window, dialog_sheet, true);
}
......@@ -5,67 +5,37 @@
#import <Cocoa/Cocoa.h>
#import <SecurityInterface/SFCertificatePanel.h>
#include "base/bind.h"
#include "base/mac/foundation_util.h"
#include "base/mac/scoped_cftyperef.h"
#import "base/mac/scoped_nsobject.h"
#include "base/memory/weak_ptr.h"
#include "base/notreached.h"
#include "chrome/browser/certificate_viewer.h"
#include "components/constrained_window/constrained_window_views.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_util_ios_and_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
// Initializes |certificates_| with the certificate chain for a given
// certificate.
- (instancetype)initWithCertificate:(net::X509Certificate*)certificate
forWebContents:(content::WebContents*)webContents;
- (instancetype)initWithCertificate:(net::X509Certificate*)certificate;
// Shows the certificate viewer as a Cocoa sheet.
- (void)showCertificateSheet:(NSWindow*)window;
// Closes the certificate viewer sheet.
- (void)closeCertificateSheet;
- (void)setOverlayWindow:(views::Widget*)overlayWindow;
// Closes the certificate viewer Cocoa sheet.
// Called when the certificate viewer Cocoa sheet is closed.
- (void)sheetDidEnd:(NSWindow*)parent
returnCode:(NSInteger)returnCode
context:(void*)context;
@end
@implementation SSLCertificateViewerMac {
// The corresponding list of certificates.
base::scoped_nsobject<NSArray> _certificates;
base::scoped_nsobject<SFCertificatePanel> _panel;
// Invisible overlay window used to block interaction with the tab underneath.
views::Widget* _overlayWindow;
NSWindow* _remoteViewsCloneWindow;
}
- (instancetype)initWithCertificate:(net::X509Certificate*)certificate
forWebContents:(content::WebContents*)webContents {
- (instancetype)initWithCertificate:(net::X509Certificate*)certificate {
if ((self = [super init])) {
base::ScopedCFTypeRef<CFArrayRef> certChain(
net::x509_util::CreateSecCertificateArrayForX509Certificate(
......@@ -119,6 +89,15 @@
}
- (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
modalDelegate:self
didEndSelector:@selector(sheetDidEnd:returnCode:context:)
......@@ -127,83 +106,20 @@
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
returnCode:(NSInteger)returnCode
context:(void*)context {
_overlayWindow->Close(); // Asynchronously releases |self|.
[_remoteViewsCloneWindow close];
_remoteViewsCloneWindow = nil;
_panel.reset();
}
- (void)setOverlayWindow:(views::Widget*)overlayWindow {
_overlayWindow = overlayWindow;
}
@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,
gfx::NativeWindow parent,
net::X509Certificate* cert) {
// Shows a new widget, which owns the delegate.
new CertificateAnchorWidgetDelegate(web_contents, cert);
base::scoped_nsobject<SSLCertificateViewerMac> viewer(
[[SSLCertificateViewerMac alloc] initWithCertificate:cert]);
[viewer showCertificateSheet:parent.GetNativeNSWindow()];
}
......@@ -2684,7 +2684,6 @@ if (!is_android) {
"../browser/ui/cocoa/share_menu_controller_browsertest.mm",
"../browser/ui/cocoa/task_manager_mac_browsertest.mm",
"../browser/ui/cocoa/touchbar/browser_window_touch_bar_controller_browsertest.mm",
"../browser/ui/views/certificate_viewer_mac_browsertest.mm",
"../browser/ui/views/frame/browser_non_client_frame_view_mac_browsertest.cc",
"../browser/ui/views/ssl_client_certificate_selector_mac_browsertest.mm",
"../common/profiler/stack_sampling_browsertest.cc",
......
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