MacViews: Refactor certificate viewer tests to work for MacViews browser.

chrome/browser/ui/cocoa/certificate_viewer_mac_browsertest.mm

Index: chrome/browser/ui/cocoa/certificate_viewer_mac_browsertest.mm
diff --git a/chrome/browser/ui/cocoa/certificate_viewer_mac_browsertest.mm b/chrome/browser/ui/cocoa/certificate_viewer_mac_browsertest.mm
index 95abb67638a8f0fc3583f89f91fa8be553bfe7c1..76ddc59f12bf8ca25b144836156be35eea4a32c2 100644
--- a/chrome/browser/ui/cocoa/certificate_viewer_mac_browsertest.mm
+++ b/chrome/browser/ui/cocoa/certificate_viewer_mac_browsertest.mm
@@ -6,7 +6,7 @@
 
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
-#include "chrome/browser/ui/cocoa/certificate_viewer_mac_cocoa.h"
+#include "chrome/browser/ui/cocoa/certificate_viewer_mac.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"
@@ -20,16 +20,31 @@
 
 using web_modal::WebContentsModalDialogManager;
 
-typedef InProcessBrowserTest SSLCertificateViewerCocoaTest;
+typedef InProcessBrowserTest SSLCertificateViewerMacTest;
 
 namespace {
 scoped_refptr<net::X509Certificate> GetSampleCertificate() {
   return net::ImportCertFromFile(net::GetTestCertsDirectory(),
                                  "mit.davidben.der");
 }
+
+void CheckCertificateViewerVisibility(gfx::NativeWindow overlay_window,

[tapted, 2016/06/07 05:33:43]

nit: these can just use NSWindow instead of gfx::NativeWindow which is a bit
more abstract. more in SLCertificateViewerMacTest.HideShow

[Patti Lor, 2016/06/08 00:14:37]

Done.

+                                      gfx::NativeWindow dialog_sheet,
+                                      bool visible) {
+  CGFloat alpha = visible ? 1.0 : 0.0;
+  BOOL ignore_events = visible ? NO : YES;
+
+  // Check the overlay window underneath the certificate viewer is blocking
+  // mouse events.
+  EXPECT_EQ(ignore_events, [overlay_window ignoresMouseEvents]);

[tapted, 2016/06/07 05:33:42]

One thing to be aware of: the output of the tests in the bots is just the line
number, so we don't know if a failure on this line was from a call with visible
= true or visible = false. Sometimes that's important to know.

There's an easy fix though - at the start of this function, do

  SCOPED_TRACE(testing::Message() << "visible=" << visible);

[Patti Lor, 2016/06/08 00:14:37]

Done.

+  // Check the certificate viewer sheet is visible and accepting mouse events.
+  ASSERT_TRUE(dialog_sheet);

[tapted, 2016/06/07 05:33:43]

EXPECT_TRUE

(ASSERT has odd semantics -- it's basically "EXPECT(..); (if it fails) return;"
e.g. it only works in functions that return `void` and it doesn't terminate the
program. Usually we just go with EXPECT, even if a crash would result later)

But I think this particular check makes more sense in the test body (see below),
so it can be removed from here

[Patti Lor, 2016/06/08 00:14:38]

Done.

+  EXPECT_EQ(alpha, [dialog_sheet alphaValue]);
+  EXPECT_EQ(ignore_events, [dialog_sheet ignoresMouseEvents]);
+}
 } // namespace

[tapted, 2016/06/07 05:33:43]

nit: blank line before (there should be one at the top of the namespace too)

[Patti Lor, 2016/06/08 00:14:37]

Done.

 
-IN_PROC_BROWSER_TEST_F(SSLCertificateViewerCocoaTest, Basic) {
+IN_PROC_BROWSER_TEST_F(SSLCertificateViewerMacTest, Basic) {
   scoped_refptr<net::X509Certificate> cert = GetSampleCertificate();
   ASSERT_TRUE(cert.get());
   content::WebContents* web_contents =
@@ -52,36 +67,43 @@ IN_PROC_BROWSER_TEST_F(SSLCertificateViewerCocoaTest, Basic) {
 }
 
 // Test that switching to another tab correctly hides the sheet.
-IN_PROC_BROWSER_TEST_F(SSLCertificateViewerCocoaTest, HideShow) {
+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();
 
-  // TODO(patricialor): Display the certificate viewer in a cross-toolkit manner
-  // and retrieve |sheetWindow| via the WebContentsModalDialogManager::TestApi.
-  // See https://crbug.com/613880.
-  SSLCertificateViewerCocoa* viewer =
-      [[SSLCertificateViewerCocoa alloc] initWithCertificate:cert.get()];
-  [viewer displayForWebContents:web_contents];
+  gfx::NativeWindow window = web_contents->GetTopLevelNativeWindow();
+  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());
   content::RunAllPendingInMessageLoop();
+  EXPECT_TRUE(web_contents_modal_dialog_manager->IsDialogActive());
+
+  ASSERT_TRUE((num_child_windows + 1) == [[window childWindows] count]);

[tapted, 2016/06/07 05:33:42]

EXPECT_EQ(num_child_windows + 1, [[window childWindows] count]));

[Patti Lor, 2016/06/08 00:14:37]

Done.

+  gfx::NativeWindow overlay_window = [[window childWindows] lastObject];

[tapted, 2016/06/07 05:33:43]

comment before:

// Assume the last child is the overlay window that was added.

[Patti Lor, 2016/06/08 00:14:37]

Done.

+  gfx::NativeWindow dialog_sheet = [overlay_window attachedSheet];

[tapted, 2016/06/07 05:33:42]

EXPECT_TRUE(dialog_sheet) after this

[Patti Lor, 2016/06/08 00:14:37]

Done.

+  NSRect sheetFrame = [dialog_sheet frame];

[tapted, 2016/06/07 05:33:43]

sheetFrame -> sheet_frame

[Patti Lor, 2016/06/08 00:14:37]

Done.

 
-  // TODO(patricialor): Remove the overlayWindow accessor from
-  // SSLCertificateViewerMac when the overlay is obtained via
-  // WebContentsModalDialogManager::TestApi instead - https://crbug.com/613880.
-  NSWindow* sheetWindow = [[viewer overlayWindow] attachedSheet];
-  NSRect sheetFrame = [sheetWindow frame];
-  EXPECT_EQ(1.0, [sheetWindow alphaValue]);
+  // Verify the certificate viewer is showing and accepts mouse events.
+  CheckCertificateViewerVisibility(overlay_window, dialog_sheet, true);
 
-  // Switch to another tab and verify that the sheet is hidden.
+  // Switch to another tab and verify that |overlay_window| and |dialog_sheet|
+  // are not blocking mouse events, and |dialog_sheet| is hidden.
   AddBlankTabAndShow(browser());
-  EXPECT_EQ(0.0, [sheetWindow alphaValue]);
-  EXPECT_NSEQ(sheetFrame, [sheetWindow frame]);
+  CheckCertificateViewerVisibility(overlay_window, dialog_sheet, false);
+  EXPECT_NSEQ(sheetFrame, [dialog_sheet frame]);
 
   // Switch back and verify that the sheet is shown.
   chrome::SelectNumberedTab(browser(), 0);
+  CheckCertificateViewerVisibility(overlay_window, dialog_sheet, true);
+
+  WebContentsModalDialogManager::TestApi test_api(
+      web_contents_modal_dialog_manager);
+  test_api.CloseAllDialogs();

[tapted, 2016/06/07 05:33:43]

was this needed for something? (comment?)

[Patti Lor, 2016/06/08 00:14:38]

Nope, removed.

   content::RunAllPendingInMessageLoop();
-  EXPECT_EQ(1.0, [sheetWindow alphaValue]);
-  EXPECT_NSEQ(sheetFrame, [sheetWindow frame]);
 }