diff --git a/DEPS b/DEPS index 2d691da..4cb13098 100644 --- a/DEPS +++ b/DEPS
@@ -74,7 +74,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '3f330693a01cac6dbfdbdcef1346ca9b59c09ee5', + 'skia_revision': 'e562f413ea3877842cbb8bc8711053ab06c9d9d9', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other.
diff --git a/ash/public/cpp/shelf_prefs.cc b/ash/public/cpp/shelf_prefs.cc index 92be37a..c64d89e6 100644 --- a/ash/public/cpp/shelf_prefs.cc +++ b/ash/public/cpp/shelf_prefs.cc
@@ -34,7 +34,7 @@ // displays. // * A user-set value for the specified display. // * A user-set value in |local_path| or |path|, if no per-display settings are -// ever specified (see http://crbug.com/173719 for why). |local_path| is +// ever specified (see http://crbug.com/173719 for why), |local_path| is // preferred. See comment in |kShelfAlignment| as to why we consider two // prefs and why |local_path| is preferred. // * A value recommended by policy. This is a single value that applies to all @@ -88,7 +88,7 @@ int64_t display_id, const char* pref_key, const std::string& value) { - if (display_id < 0) + if (display_id == display::kInvalidDisplayId) return; // Avoid DictionaryPrefUpdate's notifications for read but unmodified prefs.
diff --git a/ash/shelf/shelf_controller_unittest.cc b/ash/shelf/shelf_controller_unittest.cc index 223c88da..2973bd0 100644 --- a/ash/shelf/shelf_controller_unittest.cc +++ b/ash/shelf/shelf_controller_unittest.cc
@@ -266,6 +266,44 @@ EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS, shelf2->auto_hide_behavior()); } +// Ensures that pre-Unified Mode per-display shelf settings don't prevent us +// from changing the shelf settings in unified mode. +TEST_F(ShelfControllerPrefsTest, ShelfRespectsPerDisplayPrefsUnified) { + UpdateDisplay("1024x768,800x600"); + + // Before enabling Unified Mode, set the shelf alignment for one of the two + // displays, so that we have a per-display shelf alignment setting. + ASSERT_FALSE(display_manager()->IsInUnifiedMode()); + const int64_t non_unified_primary_id = GetPrimaryDisplay().id(); + PrefService* prefs = + Shell::Get()->session_controller()->GetLastActiveUserPrefService(); + Shelf* shelf = GetShelfForDisplay(non_unified_primary_id); + EXPECT_EQ(SHELF_ALIGNMENT_BOTTOM, shelf->alignment()); + SetShelfAlignmentPref(prefs, non_unified_primary_id, SHELF_ALIGNMENT_LEFT); + EXPECT_EQ(SHELF_ALIGNMENT_LEFT, shelf->alignment()); + + // Switch to Unified Mode, and expect to be able to change the shelf + // alignment. + display_manager()->SetUnifiedDesktopEnabled(true); + ASSERT_TRUE(display_manager()->IsInUnifiedMode()); + const int64_t unified_id = display::kUnifiedDisplayId; + ASSERT_EQ(unified_id, GetPrimaryDisplay().id()); + + shelf = GetShelfForDisplay(unified_id); + EXPECT_EQ(SHELF_ALIGNMENT_BOTTOM, shelf->alignment()); + EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_NEVER, shelf->auto_hide_behavior()); + + SetShelfAlignmentPref(prefs, unified_id, SHELF_ALIGNMENT_LEFT); + SetShelfAutoHideBehaviorPref(prefs, unified_id, + SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS); + + EXPECT_EQ(SHELF_ALIGNMENT_LEFT, shelf->alignment()); + EXPECT_EQ(SHELF_AUTO_HIDE_BEHAVIOR_ALWAYS, shelf->auto_hide_behavior()); + + SetShelfAlignmentPref(prefs, unified_id, SHELF_ALIGNMENT_RIGHT); + EXPECT_EQ(SHELF_ALIGNMENT_RIGHT, shelf->alignment()); +} + // Ensure shelf settings are correct after display swap, see crbug.com/748291 TEST_F(ShelfControllerPrefsTest, ShelfSettingsValidAfterDisplaySwap) { // Simulate adding an external display at the lock screen.
diff --git a/build/android/pylib/local/device/local_device_environment.py b/build/android/pylib/local/device/local_device_environment.py index f716477..1403504 100644 --- a/build/android/pylib/local/device/local_device_environment.py +++ b/build/android/pylib/local/device/local_device_environment.py
@@ -206,7 +206,9 @@ #override def TearDown(self): - if self.trace_output: + if self.trace_output and self._trace_all: + instrumentation_tracing.stop_instrumenting() + elif self.trace_output: self.DisableTracing() if not self._devices:
diff --git a/build/android/pylib/utils/instrumentation_tracing.py b/build/android/pylib/utils/instrumentation_tracing.py index 7e00c58..f1d03a0d 100644 --- a/build/android/pylib/utils/instrumentation_tracing.py +++ b/build/android/pylib/utils/instrumentation_tracing.py
@@ -17,6 +17,7 @@ import contextlib import functools import inspect +import os import re import sys import threading @@ -108,7 +109,16 @@ included = set() excluded = set() + tracing_pid = os.getpid() + def traceFunction(frame, event, arg): + del arg + + # Don't try to trace in subprocesses. + if os.getpid() != tracing_pid: + sys.settrace(None) + return None + # pylint: disable=unused-argument if event not in ("call", "return"): return None
diff --git a/chrome/app/chromeos_strings.grdp b/chrome/app/chromeos_strings.grdp index b06e1cd6..fb64a066 100644 --- a/chrome/app/chromeos_strings.grdp +++ b/chrome/app/chromeos_strings.grdp
@@ -4602,7 +4602,7 @@ Google Assistant </message> <message name="IDS_VOICE_INTERACTION_VALUE_PROP_LOAD_ERROR_MESSAGE" desc="Load error message of the voice interaction value prop dialog."> - Google Assistant was unable to load, please retry. + Google Assistant was unable to load, please check your network connection and retry. </message> <message name="IDS_VOICE_INTERACTION_VALUE_PROP_SKIP_BUTTON" desc="Button label for skipping voice interaction value prop upon error."> Skip
diff --git a/chrome/browser/interstitials/security_interstitial_page_test_utils.cc b/chrome/browser/interstitials/security_interstitial_page_test_utils.cc index 4cbc4c2..0afee824 100644 --- a/chrome/browser/interstitials/security_interstitial_page_test_utils.cc +++ b/chrome/browser/interstitials/security_interstitial_page_test_utils.cc
@@ -15,6 +15,7 @@ #include "components/security_interstitials/content/security_interstitial_page.h" #include "components/security_interstitials/core/controller_client.h" #include "content/public/browser/interstitial_page.h" +#include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" #include "content/public/test/browser_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" @@ -22,9 +23,8 @@ namespace chrome_browser_interstitials { -bool IsInterstitialDisplayingText( - const content::InterstitialPage* const interstitial, - const std::string& text) { +bool IsInterstitialDisplayingText(content::RenderFrameHost* interstitial_frame, + const std::string& text) { // It's valid for |text| to contain "\'", but simply look for "'" instead // since this function is used for searching host names and a predefined // string. @@ -35,8 +35,8 @@ text.c_str(), security_interstitials::CMD_TEXT_FOUND, security_interstitials::CMD_TEXT_NOT_FOUND); int result = 0; - EXPECT_TRUE(content::ExecuteScriptAndExtractInt(interstitial->GetMainFrame(), - command, &result)); + EXPECT_TRUE(content::ExecuteScriptAndExtractInt(interstitial_frame, command, + &result)); return result == security_interstitials::CMD_TEXT_FOUND; } @@ -65,8 +65,9 @@ if (!WaitForRenderFrameReady(contents->GetInterstitialPage()->GetMainFrame())) return testing::AssertionFailure() << "Render frame not ready"; - if (IsInterstitialDisplayingText(contents->GetInterstitialPage(), - kHostnameJSUnicode)) { + if (IsInterstitialDisplayingText( + contents->GetInterstitialPage()->GetMainFrame(), + kHostnameJSUnicode)) { return testing::AssertionSuccess(); } return testing::AssertionFailure() << "Interstitial not displaying text";
diff --git a/chrome/browser/interstitials/security_interstitial_page_test_utils.h b/chrome/browser/interstitials/security_interstitial_page_test_utils.h index 95c3eaf..48269bb4 100644 --- a/chrome/browser/interstitials/security_interstitial_page_test_utils.h +++ b/chrome/browser/interstitials/security_interstitial_page_test_utils.h
@@ -11,7 +11,6 @@ #include "testing/gtest/include/gtest/gtest.h" namespace content { -class InterstitialPage; class WebContents; } @@ -23,9 +22,13 @@ namespace chrome_browser_interstitials { -bool IsInterstitialDisplayingText( - const content::InterstitialPage* const interstitial, - const std::string& text); +// Looks for text in the |textContent| of |interstitial_frame|'s body and +// returns true if found. This can be used for either transient or committed +// interstitials. For the former, pass +// web_contents->GetInterstitialPage()->GetMainFrame() as the first argument, +// and for the latter, just pass web_contents->GetMainFrame(). +bool IsInterstitialDisplayingText(content::RenderFrameHost* interstitial_frame, + const std::string& text); // This class is used for testing the display of IDN names in security // interstitials.
diff --git a/chrome/browser/policy/policy_browsertest.cc b/chrome/browser/policy/policy_browsertest.cc index f758546..3f84c59 100644 --- a/chrome/browser/policy/policy_browsertest.cc +++ b/chrome/browser/policy/policy_browsertest.cc
@@ -3578,7 +3578,7 @@ ASSERT_TRUE(content::WaitForRenderFrameReady(interstitial->GetMainFrame())); EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial, "proceed-link")); + interstitial->GetMainFrame(), "proceed-link")); EXPECT_NE(base::UTF8ToUTF16("OK"), browser()->tab_strip_model()->GetActiveWebContents()->GetTitle()); @@ -3684,7 +3684,7 @@ // The interstitial should display the proceed link. EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial, "proceed-link")); + interstitial->GetMainFrame(), "proceed-link")); } // Test that when SSL error overriding is disallowed by policy, the @@ -3721,7 +3721,7 @@ // The interstitial should not display the proceed link. EXPECT_FALSE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial, "proceed-link")); + interstitial->GetMainFrame(), "proceed-link")); // The interstitial should not proceed, even if the command is sent in // some other way (e.g., via the keyboard shortcut).
diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc index b0cb9634..841e97a 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc +++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc
@@ -22,7 +22,6 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/lifetime/application_lifetime.h" -#include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/service_process/service_process_control.h" #include "chrome/common/chrome_switches.h" @@ -30,7 +29,6 @@ #include "components/prefs/pref_service.h" #include "content/public/browser/browser_thread.h" #include "printing/backend/print_backend.h" -#include "ui/message_center/notification.h" using content::BrowserThread;
diff --git a/chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc b/chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc index e49edc877..5be6b40 100644 --- a/chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc +++ b/chrome/browser/ssl/captive_portal_blocking_page_browsertest.cc
@@ -164,19 +164,21 @@ EXPECT_TRUE( WaitForRenderFrameReady(contents->GetInterstitialPage()->GetMainFrame())); EXPECT_EQ(expect_wifi == EXPECT_WIFI_YES, - IsInterstitialDisplayingText(contents->GetInterstitialPage(), - "Wi-Fi")); + IsInterstitialDisplayingText( + contents->GetInterstitialPage()->GetMainFrame(), "Wi-Fi")); if (!wifi_ssid.empty()) { EXPECT_EQ(expect_wifi_ssid == EXPECT_WIFI_SSID_YES, - IsInterstitialDisplayingText(contents->GetInterstitialPage(), - wifi_ssid)); + IsInterstitialDisplayingText( + contents->GetInterstitialPage()->GetMainFrame(), wifi_ssid)); } EXPECT_EQ(expect_login_url == EXPECT_LOGIN_URL_YES, - IsInterstitialDisplayingText(contents->GetInterstitialPage(), - expected_login_hostname)); + IsInterstitialDisplayingText( + contents->GetInterstitialPage()->GetMainFrame(), + expected_login_hostname)); EXPECT_EQ(expect_login_url == EXPECT_LOGIN_URL_NO, - IsInterstitialDisplayingText(contents->GetInterstitialPage(), - kGenericLoginURLText)); + IsInterstitialDisplayingText( + contents->GetInterstitialPage()->GetMainFrame(), + kGenericLoginURLText)); // Check that a red/dangerous lock icon is showing on the interstitial. SecurityStateTabHelper* helper =
diff --git a/chrome/browser/ssl/ssl_blocking_page.h b/chrome/browser/ssl/ssl_blocking_page.h index aabe97bf..803d7c2 100644 --- a/chrome/browser/ssl/ssl_blocking_page.h +++ b/chrome/browser/ssl/ssl_blocking_page.h
@@ -31,7 +31,6 @@ } class CertReportHelper; -class SSLUITest; class ChromeMetricsHelper; // This class is responsible for showing/hiding the interstitial page that is @@ -75,7 +74,7 @@ protected: friend class policy::PolicyTest_SSLErrorOverridingDisallowed_Test; - friend class SSLUITest; + friend class SSLUITestBase; SSLBlockingPage( content::WebContents* web_contents,
diff --git a/chrome/browser/ssl/ssl_browsertest.cc b/chrome/browser/ssl/ssl_browsertest.cc index e7b03b79..e15e25b 100644 --- a/chrome/browser/ssl/ssl_browsertest.cc +++ b/chrome/browser/ssl/ssl_browsertest.cc
@@ -253,6 +253,11 @@ } // namespace CertError +bool AreCommittedInterstitialsEnabled() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kCommittedInterstitials); +} + void CheckSecurityState(WebContents* tab, net::CertStatus expected_error, security_state::SecurityLevel expected_security_level, @@ -265,7 +270,7 @@ AuthState::Check(*entry, expected_authentication_state); } -void CheckProceedLinkExists(content::RenderViewHost* rvh) { +void CheckProceedLinkExists(WebContents* tab) { int result = security_interstitials::CMD_ERROR; const std::string javascript = base::StringPrintf( "domAutomationController.send(" @@ -273,7 +278,11 @@ "? (%d) : (%d))", security_interstitials::CMD_TEXT_NOT_FOUND, security_interstitials::CMD_TEXT_FOUND); - ASSERT_TRUE(content::ExecuteScriptAndExtractInt(rvh, javascript, &result)); + ASSERT_TRUE(content::ExecuteScriptAndExtractInt( + AreCommittedInterstitialsEnabled() + ? tab->GetMainFrame() + : tab->GetInterstitialPage()->GetMainFrame(), + javascript, &result)); EXPECT_EQ(security_interstitials::CMD_TEXT_FOUND, result); } @@ -422,32 +431,16 @@ EXPECT_TRUE(state->ShouldUpgradeToSSL(kHstsTestHostName)); } -bool AreCommittedInterstitialsEnabled() { - return base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kCommittedInterstitials); -} - -// A getter for the tab's InterstitialPageDelegate that is agnostic about -// whether we are using committed interstitials. Returns null if -// there is no delegate. -content::InterstitialPageDelegate* GetInterstitialPageDelegate( - content::WebContents* tab) { +bool IsShowingInterstitial(content::WebContents* tab) { if (AreCommittedInterstitialsEnabled()) { SSLErrorTabHelper* helper = SSLErrorTabHelper::FromWebContents(tab); if (!helper) { - return nullptr; + return false; } - return helper->GetBlockingPageForCurrentlyCommittedNavigationForTesting(); + return helper->GetBlockingPageForCurrentlyCommittedNavigationForTesting() != + nullptr; } - - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - if (!interstitial_page) - return nullptr; - return interstitial_page->GetDelegateForTesting(); -} - -bool IsShowingInterstitial(content::WebContents* tab) { - return GetInterstitialPageDelegate(tab) != nullptr; + return tab->GetInterstitialPage() != nullptr; } // Waits until an interstitial is showing. @@ -455,18 +448,63 @@ // TODO(crbug.com/752372): This should not be needed for committed // interstitials. Replace all call sites directly with the assert. void WaitForInterstitial(content::WebContents* tab) { - if (!IsShowingInterstitial(tab)) { - ASSERT_TRUE(!AreCommittedInterstitialsEnabled()); + if (!AreCommittedInterstitialsEnabled()) { content::WaitForInterstitialAttach(tab); + ASSERT_TRUE(IsShowingInterstitial(tab)); + ASSERT_TRUE( + WaitForRenderFrameReady(tab->GetInterstitialPage()->GetMainFrame())); + } else { + ASSERT_TRUE(IsShowingInterstitial(tab)); + ASSERT_TRUE(WaitForRenderFrameReady(tab->GetMainFrame())); } - ASSERT_TRUE(IsShowingInterstitial(tab)); +} + +void ExpectInterstitialHeading(content::WebContents* tab, + const std::string& expected_heading) { + if (!AreCommittedInterstitialsEnabled()) { + ASSERT_TRUE(tab->GetInterstitialPage()); + } + content::RenderFrameHost* frame = + AreCommittedInterstitialsEnabled() + ? tab->GetMainFrame() + : tab->GetInterstitialPage()->GetMainFrame(); + EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( + frame, expected_heading)); +} + +// The functions below might start causing tests to fail if you change the +// strings that appear on interstitials. If that happens, it's fine to update +// the keywords that are checked for in each interstitial. But the keywords +// should remain fairly unique for each interstitial to ensure that the tests +// check that the proper interstitial comes up. For example, it wouldn't be good +// to simply look for the word "security" because that likely shows up on lots +// of different types of interstitials, not just the type being tested for. + +void ExpectCaptivePortalInterstitial(content::WebContents* tab) { + ExpectInterstitialHeading(tab, "Connect to"); +} + +void ExpectSSLInterstitial(content::WebContents* tab) { + ExpectInterstitialHeading(tab, "Your connection is not private"); +} + +void ExpectMITMInterstitial(content::WebContents* tab) { + ExpectInterstitialHeading(tab, "An application is stopping"); +} + +void ExpectSuperfishInterstitial(content::WebContents* tab) { + ExpectInterstitialHeading(tab, "Software on your computer is stopping"); +} + +void ExpectBadClockInterstitial(content::WebContents* tab) { + ExpectInterstitialHeading(tab, "Your clock is"); } } // namespace -class SSLUITest : public InProcessBrowserTest { +class SSLUITestBase : public InProcessBrowserTest { public: - SSLUITest() + SSLUITestBase() : https_server_(net::EmbeddedTestServer::TYPE_HTTPS), https_server_expired_(net::EmbeddedTestServer::TYPE_HTTPS), https_server_mismatched_(net::EmbeddedTestServer::TYPE_HTTPS), @@ -558,23 +596,21 @@ } void ProceedThroughInterstitial(WebContents* tab) { - content::WindowedNotificationObserver observer( - content::NOTIFICATION_LOAD_STOP, - content::Source<NavigationController>(&tab->GetController())); + content::TestNavigationObserver nav_observer(tab, 1); SendInterstitialCommand(tab, security_interstitials::CMD_PROCEED); - observer.Wait(); + nav_observer.Wait(); } - void SendInterstitialCommand( + virtual void DontProceedThroughInterstitial(WebContents* tab) { + SendInterstitialCommand(tab, security_interstitials::CMD_DONT_PROCEED); + WaitForInterstitialDetach(tab); + } + + virtual void SendInterstitialCommand( WebContents* tab, security_interstitials::SecurityInterstitialCommand command) { - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - SSLBlockingPage* ssl_interstitial = static_cast<SSLBlockingPage*>( - interstitial_page->GetDelegateForTesting()); - ssl_interstitial->CommandReceived(base::IntToString(command)); + tab->GetInterstitialPage()->GetDelegateForTesting()->CommandReceived( + base::IntToString(command)); } static void GetFilePathWithHostAndPortReplacement( @@ -619,6 +655,11 @@ "/ssl/top_frame.html", replacement_text_top_frame, top_frame_path); } + virtual SSLBlockingPage* GetSSLBlockingPage(WebContents* tab) { + return static_cast<SSLBlockingPage*>( + tab->GetInterstitialPage()->GetDelegateForTesting()); + } + // Helper function for testing invalid certificate chain reporting. void TestBrokenHTTPSReporting( certificate_reporting_test_utils::OptIn opt_in, @@ -649,10 +690,8 @@ base::Unretained(&reporter_callback)), expect_report); - ASSERT_TRUE(tab->GetInterstitialPage() != nullptr); - SSLBlockingPage* interstitial_page = static_cast<SSLBlockingPage*>( - tab->GetInterstitialPage()->GetDelegateForTesting()); - ASSERT_TRUE(interstitial_page != nullptr); + SSLBlockingPage* interstitial_page = GetSSLBlockingPage(tab); + ASSERT_TRUE(interstitial_page); interstitial_page->SetSSLCertReporterForTesting( std::move(ssl_cert_reporter)); @@ -664,10 +703,7 @@ if (proceed == SSL_INTERSTITIAL_PROCEED) { ProceedThroughInterstitial(tab); } else { - // Click "Take me back" - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - interstitial_page->DontProceed(); + DontProceedThroughInterstitial(tab); } if (expect_report == @@ -711,8 +747,9 @@ ui_test_utils::NavigateToURL(browser, https_server_expired_.GetURL("/title1.html")); - WebContents* tab = browser->tab_strip_model()->GetActiveWebContents(); + WaitForInterstitial(tab); + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); @@ -723,15 +760,13 @@ base::Unretained(&reporter_callback)), expect_report); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(BadClockBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ExpectBadClockInterstitial(tab); BadClockBlockingPage* clock_page = static_cast<BadClockBlockingPage*>( tab->GetInterstitialPage()->GetDelegateForTesting()); clock_page->SetSSLCertReporterForTesting(std::move(ssl_cert_reporter)); EXPECT_EQ(std::string(), reporter_callback.GetLatestHostnameReported()); - interstitial_page->DontProceed(); + DontProceedThroughInterstitial(tab); if (expect_report == certificate_reporting_test_utils::CERT_REPORT_EXPECTED) { @@ -770,7 +805,7 @@ // test fixture method because the whole test fixture class is friended by // SSLBlockingPage. security_interstitials::SecurityInterstitialControllerClient* - GetControllerClientFromInterstitialPage(SSLBlockingPage* ssl_interstitial) { + GetControllerClientFromSSLBlockingPage(SSLBlockingPage* ssl_interstitial) { return ssl_interstitial->controller(); } @@ -817,67 +852,71 @@ policy::MockConfigurationPolicyProvider policy_provider_; - DISALLOW_COPY_AND_ASSIGN(SSLUITest); + DISALLOW_COPY_AND_ASSIGN(SSLUITestBase); }; -// Runs each test both *without* and *with* committed interstitials enabled. -// The boolean parameter indicates whether interstitial in the test are -// committed. -class SSLUITestTransientAndCommitted - : public SSLUITest, - public testing::WithParamInterface<bool> { - protected: - void SetUp() override { SSLUITest::SetUp(); } +class SSLUITest : public SSLUITestBase, + public testing::WithParamInterface<bool> { + public: + SSLUITest() : SSLUITestBase() {} - void SetUpCommandLine(base::CommandLine* command_line) override { - SSLUITest::SetUpCommandLine(command_line); + protected: + void MaybeSetUpCommittedInterstitialCommandLine( + base::CommandLine* command_line) { if (IsCommittedInterstitialTest()) { command_line->AppendSwitch(switches::kCommittedInterstitials); - // Forcing PlzNavigate means we run tests with PlzNavigate on the - // renderer_side_navigation bots, but it prevents us from having to add an - // early return check to every individual test. - command_line->AppendSwitch(switches::kEnableBrowserSideNavigation); } } - // SSLUITest: - void ProceedThroughInterstitial(WebContents* tab) { - if (!IsCommittedInterstitialTest()) { - SSLUITest::ProceedThroughInterstitial(tab); - return; - } - - content::WindowedNotificationObserver observer( - content::NOTIFICATION_LOAD_STOP, - content::Source<NavigationController>(&tab->GetController())); - GetControllerClientFromInterstitialPage(GetBlockingPage(tab))->Proceed(); - observer.Wait(); + void SetUpCommandLine(base::CommandLine* command_line) override { + SSLUITestBase::SetUpCommandLine(command_line); + MaybeSetUpCommittedInterstitialCommandLine(command_line); } - // SSLUITest: - void SendInterstitialCommand( - WebContents* tab, - security_interstitials::SecurityInterstitialCommand command) { - // TODO(crbug.com/785077): Execute script inside the interstitial. - GetInterstitialPageDelegate(tab)->CommandReceived( - base::IntToString(command)); + SSLBlockingPage* GetSSLBlockingPage(WebContents* tab) override { + if (IsCommittedInterstitialTest()) { + SSLErrorTabHelper* helper = SSLErrorTabHelper::FromWebContents(tab); + if (!helper) { + return nullptr; + } + return static_cast<SSLBlockingPage*>( + helper->GetBlockingPageForCurrentlyCommittedNavigationForTesting()); + } + return SSLUITestBase::GetSSLBlockingPage(tab); } bool IsCommittedInterstitialTest() const { return GetParam(); } - SSLBlockingPage* GetBlockingPage(WebContents* tab) { - return static_cast<SSLBlockingPage*>(GetInterstitialPageDelegate(tab)); + void DontProceedThroughInterstitial(WebContents* tab) override { + if (IsCommittedInterstitialTest()) { + content::TestNavigationObserver nav_observer(tab, 1); + SendInterstitialCommand(tab, security_interstitials::CMD_DONT_PROCEED); + nav_observer.Wait(); + } else { + SSLUITestBase::DontProceedThroughInterstitial(tab); + } } + + void SendInterstitialCommand( + WebContents* tab, + security_interstitials::SecurityInterstitialCommand command) override { + // TODO(crbug.com/785077): Execute script inside the interstitial. + if (IsCommittedInterstitialTest()) { + SSLErrorTabHelper* helper = SSLErrorTabHelper::FromWebContents(tab); + helper->GetBlockingPageForCurrentlyCommittedNavigationForTesting() + ->CommandReceived(base::IntToString(command)); + return; + } + SSLUITestBase::SendInterstitialCommand(tab, command); + } + + private: + DISALLOW_COPY_AND_ASSIGN(SSLUITest); }; -// We can declare the parametrized tests using INSTANTIATE_TEST_CASE_P anywhere. -// Since the tests are scattered throughout the file, we might as well declare -// them here. -INSTANTIATE_TEST_CASE_P(, - SSLUITestTransientAndCommitted, - ::testing::Values(false, true)); +INSTANTIATE_TEST_CASE_P(, SSLUITest, ::testing::Values(false, true)); -using SSLUITestCommitted = SSLUITestTransientAndCommitted; +using SSLUITestCommitted = SSLUITest; INSTANTIATE_TEST_CASE_P(, SSLUITestCommitted, ::testing::Values(true)); class SSLUITestBlock : public SSLUITest { @@ -886,21 +925,29 @@ // Browser will not run insecure content. void SetUpCommandLine(base::CommandLine* command_line) override { + MaybeSetUpCommittedInterstitialCommandLine(command_line); // By overriding SSLUITest, we won't apply the flag that allows running // insecure content. } }; +INSTANTIATE_TEST_CASE_P(, SSLUITestBlock, ::testing::Values(false, true)); + class SSLUITestIgnoreCertErrors : public SSLUITest { public: SSLUITestIgnoreCertErrors() : SSLUITest() {} void SetUpCommandLine(base::CommandLine* command_line) override { + SSLUITest::SetUpCommandLine(command_line); // Browser will ignore certificate errors. command_line->AppendSwitch(switches::kIgnoreCertificateErrors); } }; +INSTANTIATE_TEST_CASE_P(, + SSLUITestIgnoreCertErrors, + ::testing::Values(false, true)); + static std::string MakeCertSPKIFingerprint(net::X509Certificate* cert) { net::HashValue hash = GetSPKIHash(cert->cert_buffer()); std::string hash_base64; @@ -914,6 +961,7 @@ class SSLUITestIgnoreCertErrorsBySPKIHTTPS : public SSLUITest { protected: void SetUpCommandLine(base::CommandLine* command_line) override { + SSLUITest::SetUpCommandLine(command_line); std::string whitelist_flag = MakeCertSPKIFingerprint( https_server_mismatched_.GetCertificate().get()); // Browser will ignore certificate errors for chains matching one of the @@ -923,11 +971,16 @@ } }; +INSTANTIATE_TEST_CASE_P(, + SSLUITestIgnoreCertErrorsBySPKIHTTPS, + ::testing::Values(false, true)); + class SSLUITestIgnoreCertErrorsBySPKIWSS : public SSLUITest { public: SSLUITestIgnoreCertErrorsBySPKIWSS() : SSLUITest() {} void SetUpCommandLine(base::CommandLine* command_line) override { + SSLUITest::SetUpCommandLine(command_line); std::string whitelist_flag = MakeCertSPKIFingerprint(wss_server_expired_.GetCertificate().get()); // Browser will ignore certificate errors for chains matching one of the @@ -937,16 +990,25 @@ } }; +INSTANTIATE_TEST_CASE_P(, + SSLUITestIgnoreCertErrorsBySPKIWSS, + ::testing::Values(false, true)); + class SSLUITestIgnoreLocalhostCertErrors : public SSLUITest { public: SSLUITestIgnoreLocalhostCertErrors() : SSLUITest() {} void SetUpCommandLine(base::CommandLine* command_line) override { + SSLUITest::SetUpCommandLine(command_line); // Browser will ignore certificate errors on localhost. command_line->AppendSwitch(switches::kAllowInsecureLocalhost); } }; +INSTANTIATE_TEST_CASE_P(, + SSLUITestIgnoreLocalhostCertErrors, + ::testing::Values(false, true)); + class SSLUITestWithExtendedReporting : public SSLUITest { public: SSLUITestWithExtendedReporting() : SSLUITest() { @@ -956,6 +1018,12 @@ } }; +// TODO(estark): enable these tests for committed interstitials, which requires +// some refactoring of how reporting and metrics work. https://crbug.com/792324 +INSTANTIATE_TEST_CASE_P(, + SSLUITestWithExtendedReporting, + ::testing::Values(false)); + class SSLUITestHSTS : public SSLUITest { public: void SetUpOnMainThread() override { @@ -968,8 +1036,10 @@ } }; +INSTANTIATE_TEST_CASE_P(, SSLUITestHSTS, ::testing::Values(false, true)); + // Visits a regular page over http. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, TestHTTP) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTP) { ASSERT_TRUE(embedded_test_server()->Start()); ui_test_utils::NavigateToURL( @@ -983,8 +1053,7 @@ // be OK). // TODO(jcampan): test that bad HTTPS content is blocked (otherwise we'll give // the secure cookies away!). -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestHTTPWithBrokenHTTPSResource) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPWithBrokenHTTPSResource) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1000,7 +1069,7 @@ browser()->tab_strip_model()->GetActiveWebContents(), AuthState::NONE); } -IN_PROC_BROWSER_TEST_F(SSLUITest, TestBrokenHTTPSWithInsecureContent) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestBrokenHTTPSWithInsecureContent) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1025,7 +1094,7 @@ // Tests that the NavigationEntry gets marked as active mixed content, // even if there is a certificate error. Regression test for // https://crbug.com/593950. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestBrokenHTTPSWithActiveInsecureContent) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestBrokenHTTPSWithActiveInsecureContent) { ASSERT_TRUE(https_server_expired_.Start()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -1091,7 +1160,7 @@ // Tests that the mixed content flags are reset when going back to an existing // navigation entry that had mixed content. Regression test for // https://crbug.com/750649. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, GoBackToMixedContent) { +IN_PROC_BROWSER_TEST_P(SSLUITest, GoBackToMixedContent) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -1123,8 +1192,7 @@ } // Tests that the mixed content flags are not reset for an in-page navigation. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - MixedContentWithSameDocumentNavigation) { +IN_PROC_BROWSER_TEST_P(SSLUITest, MixedContentWithSameDocumentNavigation) { ASSERT_TRUE(https_server_.Start()); // Navigate to a URL and dynamically load mixed content. @@ -1154,7 +1222,7 @@ // Tests that the WebContents's flag for displaying content with cert // errors get cleared upon navigation. -IN_PROC_BROWSER_TEST_F(SSLUITest, +IN_PROC_BROWSER_TEST_P(SSLUITest, DisplayedContentWithCertErrorsClearedOnNavigation) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1185,7 +1253,7 @@ content::SSLStatus::DISPLAYED_CONTENT_WITH_CERT_ERRORS); } -IN_PROC_BROWSER_TEST_F(SSLUITest, TestBrokenHTTPSMetricsReporting_Proceed) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestBrokenHTTPSMetricsReporting_Proceed) { ASSERT_TRUE(https_server_expired_.Start()); base::HistogramTester histograms; const std::string decision_histogram = @@ -1221,7 +1289,7 @@ security_interstitials::MetricsHelper::TOTAL_VISITS, 1); } -IN_PROC_BROWSER_TEST_F(SSLUITest, TestBrokenHTTPSMetricsReporting_DontProceed) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestBrokenHTTPSMetricsReporting_DontProceed) { ASSERT_TRUE(https_server_expired_.Start()); base::HistogramTester histograms; const std::string decision_histogram = @@ -1259,7 +1327,7 @@ } // Visits a page over OK https: -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, TestOKHTTPS) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestOKHTTPS) { ASSERT_TRUE(https_server_.Start()); ui_test_utils::NavigateToURL(browser(), @@ -1270,8 +1338,7 @@ } // Visits a page with https error and proceed: -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestHTTPSExpiredCertAndProceed) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSExpiredCertAndProceed) { ASSERT_TRUE(https_server_expired_.Start()); ui_test_utils::NavigateToURL( @@ -1290,8 +1357,7 @@ } // Visits a page in an app window with https error and proceed: -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - InAppTestHTTPSExpiredCertAndProceed) { +IN_PROC_BROWSER_TEST_P(SSLUITest, InAppTestHTTPSExpiredCertAndProceed) { auto feature_list = base::MakeUnique<base::test::ScopedFeatureList>(); feature_list->InitAndEnableFeature(features::kDesktopPWAWindowing); @@ -1344,7 +1410,7 @@ // Visits a page with https error and don't proceed (and ensure we can still // navigate at that point): -IN_PROC_BROWSER_TEST_F(SSLUITest, TestInterstitialCrossSiteNavigation) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestInterstitialCrossSiteNavigation) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_mismatched_.Start()); @@ -1367,14 +1433,10 @@ WaitForInterstitial(tab); CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID, AuthState::SHOWING_INTERSTITIAL); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); // Simulate user clicking "Take me back". - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - interstitial_page->DontProceed(); - WaitForInterstitialDetach(tab); + DontProceedThroughInterstitial(tab); // We should be back to the original good page. CheckAuthenticatedState(tab, AuthState::NONE); @@ -1386,7 +1448,7 @@ } // Test that localhost pages don't show an interstitial. -IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreLocalhostCertErrors, +IN_PROC_BROWSER_TEST_P(SSLUITestIgnoreLocalhostCertErrors, TestNoInterstitialOnLocalhost) { ASSERT_TRUE(https_server_.Start()); @@ -1414,7 +1476,7 @@ EXPECT_EQ(title, expected_title); } -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSErrorCausedByClockUsingBuildTime) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSErrorCausedByClockUsingBuildTime) { ASSERT_TRUE(https_server_expired_.Start()); // Set up the build and current clock times to be more than a year apart. @@ -1429,16 +1491,13 @@ https_server_expired_.GetURL("/title1.html")); WebContents* clock_tab = browser()->tab_strip_model()->GetActiveWebContents(); WaitForInterstitial(clock_tab); - InterstitialPage* clock_interstitial = clock_tab->GetInterstitialPage(); - ASSERT_TRUE(clock_interstitial); - EXPECT_EQ(BadClockBlockingPage::kTypeForTesting, - clock_interstitial->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectBadClockInterstitial(clock_tab)); CheckSecurityState(clock_tab, net::CERT_STATUS_DATE_INVALID, security_state::DANGEROUS, AuthState::SHOWING_INTERSTITIAL); } -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSErrorCausedByClockUsingNetwork) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSErrorCausedByClockUsingNetwork) { ASSERT_TRUE(https_server_expired_.Start()); // Set network forward ten minutes, which is sufficient to trigger @@ -1453,17 +1512,14 @@ https_server_expired_.GetURL("/title1.html")); WebContents* clock_tab = browser()->tab_strip_model()->GetActiveWebContents(); WaitForInterstitial(clock_tab); - InterstitialPage* clock_interstitial = clock_tab->GetInterstitialPage(); - ASSERT_TRUE(clock_interstitial); - EXPECT_EQ(BadClockBlockingPage::kTypeForTesting, - clock_interstitial->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectBadClockInterstitial(clock_tab)); CheckSecurityState(clock_tab, net::CERT_STATUS_DATE_INVALID, security_state::DANGEROUS, AuthState::SHOWING_INTERSTITIAL); } // Visits a page with https error and then goes back using Browser::GoBack. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBackViaButton) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSExpiredCertAndGoBackViaButton) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1485,20 +1541,22 @@ chrome::GoBack(browser(), WindowOpenDisposition::CURRENT_TAB); content::WaitForLoadStop(tab); - // Make sure we haven't changed the previous RFH. Prevents regression of - // http://crbug.com/82667. - EXPECT_EQ(rfh, tab->GetMainFrame()); + if (!IsCommittedInterstitialTest()) { + // Make sure we haven't changed the previous RFH. Prevents regression of + // http://crbug.com/82667. This is only applicable to pre-committed + // interstitials. With committed interstitials, the interstitial is a + // committed error page, so going back from it to a different site can be a + // cross-site transition. + EXPECT_EQ(rfh, tab->GetMainFrame()); + } // We should be back at the original good page. - EXPECT_FALSE(browser() - ->tab_strip_model() - ->GetActiveWebContents() - ->GetInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(tab)); CheckUnauthenticatedState(tab, AuthState::NONE); } // Visits a page with https error and then goes back using GoToOffset. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBackViaMenu) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSExpiredCertAndGoBackViaMenu) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1515,21 +1573,21 @@ CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); - // Simulate user clicking and holding on back button (crbug.com/37215). + // Simulate user clicking and holding on back button (crbug.com/37215). With + // committed interstitials enabled, this triggers a navigation. + content::TestNavigationObserver nav_observer(tab); tab->GetController().GoToOffset(-1); + if (IsCommittedInterstitialTest()) + nav_observer.Wait(); // We should be back at the original good page. - EXPECT_FALSE(browser() - ->tab_strip_model() - ->GetActiveWebContents() - ->GetInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(tab)); CheckUnauthenticatedState(tab, AuthState::NONE); } // Visits a page with https error and then goes back using the DONT_PROCEED // interstitial command. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestHTTPSExpiredCertGoBackUsingCommand) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSExpiredCertGoBackUsingCommand) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1561,7 +1619,11 @@ } // Visits a page with https error and then goes forward using GoToOffset. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoForward) { +// +// This test is not enabled for committed interstitials because committed +// interstitials wipe out forward history like other committed navigations and +// committed error pages. +IN_PROC_BROWSER_TEST_F(SSLUITestBase, TestHTTPSExpiredCertAndGoForward) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -1604,10 +1666,7 @@ } // We should be showing the second good page. - EXPECT_FALSE(browser() - ->tab_strip_model() - ->GetActiveWebContents() - ->GetInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(tab)); CheckUnauthenticatedState(tab, AuthState::NONE); EXPECT_FALSE(tab->GetController().CanGoForward()); NavigationEntry* entry4 = tab->GetController().GetActiveEntry(); @@ -1615,7 +1674,7 @@ } // Visits a page with revocation checking enabled and a valid OCSP response. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, TestHTTPSOCSPOk) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSOCSPOk) { EnableRevocationChecking(); ASSERT_TRUE(https_server_ocsp_ok_.Start()); @@ -1637,7 +1696,7 @@ } // Visits a page with revocation checking enabled and a revoked OCSP response. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, TestHTTPSOCSPRevoked) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSOCSPRevoked) { EnableRevocationChecking(); ASSERT_TRUE(https_server_ocsp_revoked_.Start()); @@ -1653,8 +1712,7 @@ // Visit a HTTP page which request WSS connection to a server providing invalid // certificate. Close the page while WSS connection waits for SSLManager's // response from UI thread. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestWSSInvalidCertAndClose) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestWSSInvalidCertAndClose) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(wss_server_expired_.Start()); @@ -1701,7 +1759,7 @@ // Visit a HTTPS page and proceeds despite an invalid certificate. The page // requests WSS connection to the same origin host to check if WSS connection // share certificates policy with HTTPS correcly. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestWSSInvalidCertAndGoForward) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestWSSInvalidCert) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(wss_server_expired_.Start()); @@ -1716,7 +1774,7 @@ ui_test_utils::NavigateToURL(browser(), wss_server_expired_.GetURL("connect_check.html") .ReplaceComponents(replacements)); - WaitForInterstitialAttach(tab); + WaitForInterstitial(tab); CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); @@ -1731,7 +1789,7 @@ // Ensure that non-standard origins are marked as neutral when the // MarkNonSecureAs Dangerous flag is enabled. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, MarkFileAsNonSecure) { +IN_PROC_BROWSER_TEST_P(SSLUITest, MarkFileAsNonSecure) { base::test::ScopedCommandLine scoped_command_line; scoped_command_line.GetProcessCommandLine()->AppendSwitchASCII( security_state::switches::kMarkHttpAs, @@ -1753,7 +1811,7 @@ // Ensure that about-protocol origins are marked as neutral when the // MarkNonSecureAs Dangerous flag is enabled. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, MarkAboutAsNonSecure) { +IN_PROC_BROWSER_TEST_P(SSLUITest, MarkAboutAsNonSecure) { base::test::ScopedCommandLine scoped_command_line; scoped_command_line.GetProcessCommandLine()->AppendSwitchASCII( security_state::switches::kMarkHttpAs, @@ -1774,7 +1832,7 @@ } // Data URLs should always be marked as non-secure. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, MarkDataAsNonSecure) { +IN_PROC_BROWSER_TEST_P(SSLUITest, MarkDataAsNonSecure) { content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); ASSERT_TRUE(contents); @@ -1791,7 +1849,7 @@ // Ensure that HTTP-protocol origins are marked as Dangerous when the // MarkNonSecureAs Dangerous flag is enabled. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, MarkHTTPAsDangerous) { +IN_PROC_BROWSER_TEST_P(SSLUITest, MarkHTTPAsDangerous) { base::test::ScopedCommandLine scoped_command_line; scoped_command_line.GetProcessCommandLine()->AppendSwitchASCII( security_state::switches::kMarkHttpAs, @@ -1816,7 +1874,7 @@ // Ensure that blob-protocol origins are marked as neutral when the // MarkNonSecureAs Dangerous flag is enabled. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, MarkBlobAsNonSecure) { +IN_PROC_BROWSER_TEST_P(SSLUITest, MarkBlobAsNonSecure) { base::test::ScopedCommandLine scoped_command_line; scoped_command_line.GetProcessCommandLine()->AppendSwitchASCII( security_state::switches::kMarkHttpAs, @@ -1839,12 +1897,12 @@ } #if defined(USE_NSS_CERTS) -class SSLUITestWithClientCert : public SSLUITest { +class SSLUITestWithClientCert : public SSLUITestBase { public: SSLUITestWithClientCert() : cert_db_(NULL) {} void SetUpOnMainThread() override { - SSLUITest::SetUpOnMainThread(); + SSLUITestBase::SetUpOnMainThread(); base::RunLoop loop; GetNSSCertDatabaseForProfile( @@ -1929,7 +1987,7 @@ // link with a blank target). This is to test that the lack of navigation entry // does not cause any problems (it was causing a crasher, see // http://crbug.com/19941). -IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSErrorWithNoNavEntry) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestHTTPSErrorWithNoNavEntry) { ASSERT_TRUE(https_server_expired_.Start()); const GURL url = https_server_expired_.GetURL("/ssl/google.htm"); @@ -1942,13 +2000,10 @@ // We should have an interstitial page showing. WaitForInterstitial(tab2); - ASSERT_TRUE(tab2->GetInterstitialPage()); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, tab2->GetInterstitialPage() - ->GetDelegateForTesting() - ->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab2)); } -IN_PROC_BROWSER_TEST_F(SSLUITest, TestBadHTTPSDownload) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestBadHTTPSDownload) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); GURL url_non_dangerous = embedded_test_server()->GetURL("/title1.html"); @@ -1985,29 +2040,28 @@ content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT); // Proceed through the SSL interstitial. This doesn't use - // |ProceedThroughInterstitial| since no page load will commit. + // ProceedThroughInterstitial() since no page load will commit. WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); - ASSERT_TRUE(tab != NULL); - ASSERT_TRUE(tab->GetInterstitialPage() != NULL); - ASSERT_EQ( - SSLBlockingPage::kTypeForTesting, - tab->GetInterstitialPage()->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_TRUE(tab); + WaitForInterstitial(tab); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); { + content::TestNavigationObserver nav_observer(tab); content::WindowedNotificationObserver observer( chrome::NOTIFICATION_DOWNLOAD_INITIATED, content::NotificationService::AllSources()); - tab->GetInterstitialPage()->Proceed(); + SendInterstitialCommand(tab, security_interstitials::CMD_PROCEED); observer.Wait(); + if (IsCommittedInterstitialTest()) + nav_observer.Wait(); } // There should still be an interstitial at this point. Press the // back button on the browser. Note that this doesn't wait for a // NAV_ENTRY_COMMITTED notification because going back with an // active interstitial simply hides the interstitial. - ASSERT_TRUE(tab->GetInterstitialPage() != NULL); - ASSERT_EQ( - SSLBlockingPage::kTypeForTesting, - tab->GetInterstitialPage()->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_TRUE(IsShowingInterstitial(tab)); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(chrome::CanGoBack(browser())); chrome::GoBack(browser(), WindowOpenDisposition::CURRENT_TAB); @@ -2019,8 +2073,7 @@ // // Visits a page that displays insecure content. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestDisplaysInsecureContent) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestDisplaysInsecureContent) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2039,8 +2092,7 @@ } // Visits a page that displays an insecure form. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestDisplaysInsecureForm) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestDisplaysInsecureForm) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2059,7 +2111,7 @@ // Test that if the user proceeds and the checkbox is checked, a report // is sent or not sent depending on the Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSProceedReporting) { certificate_reporting_test_utils::ExpectReport expect_report = certificate_reporting_test_utils::GetReportExpectedFromFinch(); @@ -2070,7 +2122,7 @@ // Test that if the user goes back and the checkbox is checked, a report // is sent or not sent depending on the Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSGoBackReporting) { certificate_reporting_test_utils::ExpectReport expect_report = certificate_reporting_test_utils::GetReportExpectedFromFinch(); @@ -2081,7 +2133,7 @@ // User proceeds, checkbox is shown but unchecked. Reports should never // be sent, regardless of Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSProceedReportingWithNoOptIn) { TestBrokenHTTPSReporting( certificate_reporting_test_utils::EXTENDED_REPORTING_DO_NOT_OPT_IN, @@ -2091,7 +2143,7 @@ // User goes back, checkbox is shown but unchecked. Reports should never // be sent, regardless of Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSGoBackShowYesCheckNoParamYesReportNo) { TestBrokenHTTPSReporting( certificate_reporting_test_utils::EXTENDED_REPORTING_DO_NOT_OPT_IN, @@ -2101,7 +2153,7 @@ // User proceeds, checkbox is not shown but checked -> we expect no // report. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSProceedShowNoCheckYesReportNo) { if (base::FieldTrialList::FindFullName( CertReportHelper::kFinchExperimentName) == @@ -2115,7 +2167,7 @@ // Browser is incognito, user proceeds, checkbox has previously opted in // -> no report, regardless of Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSInIncognitoReportNo) { TestBrokenHTTPSReporting( certificate_reporting_test_utils::EXTENDED_REPORTING_OPT_IN, @@ -2126,7 +2178,7 @@ // Test that reports don't get sent when extended reporting opt-in is // disabled by policy. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBrokenHTTPSNoReportingWhenDisallowed) { browser()->profile()->GetPrefs()->SetBoolean( prefs::kSafeBrowsingExtendedReportingOptInAllowed, false); @@ -2138,7 +2190,7 @@ // Checkbox is shown but unchecked. Reports should never be sent, regardless of // Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBadClockReportingWithNoOptIn) { TestBadClockReporting( certificate_reporting_test_utils::EXTENDED_REPORTING_DO_NOT_OPT_IN, @@ -2147,7 +2199,7 @@ // Test that when the interstitial closes and the checkbox is checked, a report // is sent or not sent depending on the Finch config. -IN_PROC_BROWSER_TEST_F(SSLUITestWithExtendedReporting, +IN_PROC_BROWSER_TEST_P(SSLUITestWithExtendedReporting, TestBadClockReportingWithOptIn) { certificate_reporting_test_utils::ExpectReport expect_report = certificate_reporting_test_utils::GetReportExpectedFromFinch(); @@ -2159,8 +2211,7 @@ // Visits a page that runs insecure content and tries to suppress the insecure // content warnings by randomizing location.hash. // Based on http://crbug.com/8706 -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestRunsInsecuredContentRandomizeHash) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRunsInsecuredContentRandomizeHash) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2177,7 +2228,7 @@ // - For the good SSL case, the iframe and images should be properly displayed. // - For the bad SSL case, the iframe contents shouldn't be displayed and images // and scripts should be filtered out entirely. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, TestUnsafeContents) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestUnsafeContents) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); // Enable popups without user gesture. @@ -2252,7 +2303,7 @@ #define MAYBE_TestDisplaysInsecureContentLoadedFromJS \ TestDisplaysInsecureContentLoadedFromJS #endif -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, +IN_PROC_BROWSER_TEST_P(SSLUITest, MAYBE_TestDisplaysInsecureContentLoadedFromJS) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2284,8 +2335,7 @@ // Visits two pages from the same origin: one that displays insecure content and // one that doesn't. The test checks that we do not propagate the insecure // content state from one to the other. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestDisplaysInsecureContentTwoTabs) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestDisplaysInsecureContentTwoTabs) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2326,8 +2376,7 @@ // Visits two pages from the same origin: one that runs insecure content and one // that doesn't. The test checks that we propagate the insecure content state // from one to the other. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestRunsInsecureContentTwoTabs) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRunsInsecureContentTwoTabs) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2375,8 +2424,7 @@ // Visits a page with an image over http. Visits another page over https // referencing that same image over http (hoping it is coming from the webcore // memory cache). -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestDisplaysCachedInsecureContent) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestDisplaysCachedInsecureContent) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2402,8 +2450,7 @@ // Visits a page with script over http. Visits another page over https // referencing that same script over http (hoping it is coming from the webcore // memory cache). -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestRunsCachedInsecureContent) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRunsCachedInsecureContent) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2429,8 +2476,7 @@ // This test ensures the CN invalid status does not 'stick' to a certificate // (see bug #1044942) and that it depends on the host-name. // Test if disabled due to flakiness http://crbug.com/368280 . -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - DISABLED_TestCNInvalidStickiness) { +IN_PROC_BROWSER_TEST_P(SSLUITest, DISABLED_TestCNInvalidStickiness) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_mismatched_.Start()); @@ -2464,7 +2510,7 @@ } // Test that navigating to a #ref does not change a bad security state. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestRefNavigation) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRefNavigation) { ASSERT_TRUE(https_server_expired_.Start()); ui_test_utils::NavigateToURL( @@ -2489,7 +2535,7 @@ // Tests that closing a page that opened a pop-up with an interstitial does not // crash the browser (crbug.com/1966). -IN_PROC_BROWSER_TEST_F(SSLUITest, TestCloseTabWithUnsafePopup) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestCloseTabWithUnsafePopup) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2506,9 +2552,9 @@ content::WindowedNotificationObserver popup_observer( chrome::NOTIFICATION_TAB_ADDED, content::NotificationService::AllSources()); - content::WindowedNotificationObserver nav_observer( - content::NOTIFICATION_NAV_ENTRY_COMMITTED, - content::NotificationService::AllSources()); + content::TestNavigationObserver nav_observer( + https_server_expired_.GetURL("/ssl/bad_iframe.html")); + nav_observer.StartWatchingNewWebContents(); ASSERT_EQ(1u, chrome::GetBrowserCount(browser()->profile())); ui_test_utils::NavigateToURL( browser(), embedded_test_server()->GetURL(replacement_path)); @@ -2520,15 +2566,19 @@ Browser* popup_browser = chrome::FindBrowserWithProfile(browser()->profile()); WebContents* popup = popup_browser->tab_strip_model()->GetActiveWebContents(); EXPECT_NE(popup, tab1); - nav_observer.Wait(); - // Since the popup is showing an interstitial, it shouldn't have a last - // committed entry. + if (IsCommittedInterstitialTest()) + nav_observer.Wait(); WaitForInterstitial(popup); - EXPECT_FALSE(popup->GetController().GetLastCommittedEntry()); + // Since the popup is showing an interstitial, it shouldn't have a last + // committed entry (except when committed interstitials are enabled, in which + // case an interstitial is a committed entry). + if (!IsCommittedInterstitialTest()) { + EXPECT_FALSE(popup->GetController().GetLastCommittedEntry()); + } ASSERT_TRUE(popup->GetController().GetVisibleEntry()); EXPECT_EQ(https_server_expired_.GetURL("/ssl/bad_iframe.html"), popup->GetController().GetVisibleEntry()->GetURL()); - EXPECT_TRUE(popup->ShowingInterstitialPage()); + EXPECT_TRUE(IsShowingInterstitial(popup)); // Add another tab to make sure the browser does not exit when we close // the first tab. @@ -2544,7 +2594,8 @@ } // Visit a page over bad https that is a redirect to a page with good https. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectBadToGoodHTTPS) { +// TODO(estark): switch to SSLUITest when https://crbug.com/792221 is fixed. +IN_PROC_BROWSER_TEST_F(SSLUITestBase, TestRedirectBadToGoodHTTPS) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2565,7 +2616,7 @@ } // Visit a page over good https that is a redirect to a page with bad https. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectGoodToBadHTTPS) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRedirectGoodToBadHTTPS) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2585,8 +2636,7 @@ } // Visit a page over http that is a redirect to a page with good HTTPS. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestRedirectHTTPToGoodHTTPS) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRedirectHTTPToGoodHTTPS) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2602,7 +2652,7 @@ } // Visit a page over http that is a redirect to a page with bad HTTPS. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestRedirectHTTPToBadHTTPS) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRedirectHTTPToBadHTTPS) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2624,8 +2674,7 @@ // Visit a page over https that is a redirect to a page with http (to make sure // we don't keep the secure state). -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestRedirectHTTPSToHTTP) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestRedirectHTTPSToHTTP) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2679,9 +2728,13 @@ DISALLOW_COPY_AND_ASSIGN(SSLUITestWaitForDOMNotification); }; +INSTANTIATE_TEST_CASE_P(, + SSLUITestWaitForDOMNotification, + ::testing::Values(false, true)); + // Tests that a mixed resource which includes HTTP in the redirect chain // is marked as mixed content, even if the end result is HTTPS. -IN_PROC_BROWSER_TEST_F(SSLUITestWaitForDOMNotification, +IN_PROC_BROWSER_TEST_P(SSLUITestWaitForDOMNotification, TestMixedContentWithHTTPInRedirectChain) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -2738,7 +2791,7 @@ // Visits a page to which we could not connect (bad port) over http and https // and make sure the security style is correct. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, TestConnectToBadPort) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestConnectToBadPort) { ui_test_utils::NavigateToURL(browser(), GURL("http://localhost:17")); CheckUnauthenticatedState( browser()->tab_strip_model()->GetActiveWebContents(), @@ -2760,8 +2813,7 @@ // - navigate to a bad HTTPS (expect unsafe content and filtered frame), then // back // - navigate to HTTP (expect insecure content), then back -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestGoodFrameNavigation) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestGoodFrameNavigation) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2869,7 +2921,7 @@ // From a bad HTTPS top frame: // - navigate to an OK HTTPS frame (expected to be still authentication broken). -IN_PROC_BROWSER_TEST_F(SSLUITest, TestBadFrameNavigation) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestBadFrameNavigation) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2904,8 +2956,7 @@ // From an HTTP top frame, navigate to good and bad HTTPS (security state should // stay unauthenticated). -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - TestUnauthenticatedFrameNavigation) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestUnauthenticatedFrameNavigation) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -2966,10 +3017,12 @@ enum class OffMainThreadFetchMode { kEnabled, kDisabled }; enum class SSLUIWorkerFetchTestType { kUseFetch, kUseImportScripts }; +// TODO(estark): adapt this test class to work with committed interstitials. +// https://crbug.com/752327 class SSLUIWorkerFetchTest : public testing::WithParamInterface< std::pair<OffMainThreadFetchMode, SSLUIWorkerFetchTestType>>, - public SSLUITest { + public SSLUITestBase { public: SSLUIWorkerFetchTest() { EXPECT_TRUE(tmp_dir_.CreateUniqueTempDir()); @@ -3424,7 +3477,7 @@ // Visits a page with unsafe content and makes sure that if a user exception // to the certificate error is present, the image is loaded and script // executes. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeContentsWithUserException) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestUnsafeContentsWithUserException) { WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); ASSERT_NO_FATAL_FAILURE(SetUpUnsafeContentsWithUserException( "/ssl/page_with_unsafe_contents.html")); @@ -3475,7 +3528,7 @@ } // Like the test above, but only displaying inactive content (an image). -IN_PROC_BROWSER_TEST_F(SSLUITest, TestUnsafeImageWithUserException) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestUnsafeImageWithUserException) { WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); ASSERT_NO_FATAL_FAILURE( SetUpUnsafeContentsWithUserException("/ssl/page_with_unsafe_image.html")); @@ -3503,7 +3556,7 @@ // Test that when the browser blocks displaying insecure content (iframes), // the indicator shows a secure page, because the blocking made the otherwise // unsafe page safe (the notification of this state is handled by other means) -IN_PROC_BROWSER_TEST_F(SSLUITestBlock, TestBlockDisplayingInsecureIframe) { +IN_PROC_BROWSER_TEST_P(SSLUITestBlock, TestBlockDisplayingInsecureIframe) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -3523,7 +3576,7 @@ // indicator shows a secure page, because the blocking made the otherwise // unsafe page safe (the notification of this state is handled by other // means). -IN_PROC_BROWSER_TEST_F(SSLUITestBlock, TestBlockRunningInsecureContent) { +IN_PROC_BROWSER_TEST_P(SSLUITestBlock, TestBlockRunningInsecureContent) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -3542,7 +3595,7 @@ // Visit a page and establish a WebSocket connection over bad https with // --ignore-certificate-errors. The connection should be established without // interstitial page showing. -IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreCertErrors, TestWSS) { +IN_PROC_BROWSER_TEST_P(SSLUITestIgnoreCertErrors, TestWSS) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(wss_server_expired_.Start()); @@ -3570,7 +3623,7 @@ // --ignore-certificate-errors-spki-list. The connection should be established // without interstitial page showing. #if !defined(OS_CHROMEOS) // Chrome OS does not support the flag. -IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreCertErrorsBySPKIWSS, TestWSSExpired) { +IN_PROC_BROWSER_TEST_P(SSLUITestIgnoreCertErrorsBySPKIWSS, TestWSSExpired) { ASSERT_TRUE(wss_server_expired_.Start()); // Setup page title observer. @@ -3597,7 +3650,7 @@ // Test that HTTPS pages with a bad certificate don't show an interstitial if // the public key matches a value from --ignore-certificate-errors-spki-list. #if !defined(OS_CHROMEOS) // Chrome OS does not support the flag. -IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreCertErrorsBySPKIHTTPS, TestHTTPS) { +IN_PROC_BROWSER_TEST_P(SSLUITestIgnoreCertErrorsBySPKIHTTPS, TestHTTPS) { ASSERT_TRUE(https_server_mismatched_.Start()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -3618,7 +3671,7 @@ // Test subresources from an origin with a bad certificate are loaded if the // public key matches a value from --ignore-certificate-errors-spki-list. #if !defined(OS_CHROMEOS) // Chrome OS does not support the flag. -IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreCertErrorsBySPKIHTTPS, +IN_PROC_BROWSER_TEST_P(SSLUITestIgnoreCertErrorsBySPKIHTTPS, TestInsecureSubresource) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_mismatched_.Start()); @@ -3645,7 +3698,8 @@ // Verifies that the interstitial can proceed, even if JavaScript is disabled. // http://crbug.com/322948 -IN_PROC_BROWSER_TEST_F(SSLUITest, TestInterstitialJavaScriptProceeds) { +// TODO(estark): fix for committed interstitials. https://crbug.com/792135 +IN_PROC_BROWSER_TEST_F(SSLUITestBase, TestInterstitialJavaScriptProceeds) { HostContentSettingsMapFactory::GetForProfile(browser()->profile()) ->SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_JAVASCRIPT, CONTENT_SETTING_BLOCK); @@ -3657,10 +3711,8 @@ CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - EXPECT_TRUE(WaitForRenderFrameReady(interstitial_page->GetMainFrame())); + WaitForInterstitial(tab); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); content::WindowedNotificationObserver observer( content::NOTIFICATION_LOAD_STOP, @@ -3670,7 +3722,7 @@ base::StringPrintf("window.domAutomationController.send(%d);", security_interstitials::CMD_PROCEED); ASSERT_TRUE(content::ExecuteScriptAndExtractInt( - interstitial_page->GetMainFrame(), javascript, &result)); + tab->GetInterstitialPage()->GetMainFrame(), javascript, &result)); // The above will hang without the fix. EXPECT_EQ(1, result); observer.Wait(); @@ -3680,7 +3732,8 @@ // Verifies that the interstitial can go back, even if JavaScript is disabled. // http://crbug.com/322948 -IN_PROC_BROWSER_TEST_F(SSLUITest, TestInterstitialJavaScriptGoesBack) { +// TODO(estark): fix for committed interstitials. https://crbug.com/792135 +IN_PROC_BROWSER_TEST_F(SSLUITestBase, TestInterstitialJavaScriptGoesBack) { HostContentSettingsMapFactory::GetForProfile(browser()->profile()) ->SetDefaultContentSetting(CONTENT_SETTINGS_TYPE_JAVASCRIPT, CONTENT_SETTING_BLOCK); @@ -3691,29 +3744,23 @@ browser(), https_server_expired_.GetURL("/ssl/google.html")); CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); + WaitForInterstitial(tab); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); - content::WindowedNotificationObserver observer( - content::NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED, - content::NotificationService::AllSources()); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - content::RenderViewHost* interstitial_rvh = - interstitial_page->GetMainFrame()->GetRenderViewHost(); int result = security_interstitials::CMD_ERROR; const std::string javascript = base::StringPrintf("window.domAutomationController.send(%d);", security_interstitials::CMD_DONT_PROCEED); - ASSERT_TRUE(content::ExecuteScriptAndExtractInt(interstitial_rvh, javascript, - &result)); + ASSERT_TRUE(content::ExecuteScriptAndExtractInt( + tab->GetInterstitialPage()->GetMainFrame(), javascript, &result)); // The above will hang without the fix. EXPECT_EQ(0, result); - observer.Wait(); + WaitForInterstitialDetach(tab); EXPECT_EQ("about:blank", tab->GetVisibleURL().spec()); } // Verifies that an overridable interstitial has a proceed link. -IN_PROC_BROWSER_TEST_F(SSLUITest, ProceedLinkOverridable) { +IN_PROC_BROWSER_TEST_P(SSLUITest, ProceedLinkOverridable) { ASSERT_TRUE(https_server_expired_.Start()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); ui_test_utils::NavigateToURL( @@ -3722,16 +3769,9 @@ CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - content::RenderViewHost* interstitial_rvh = - interstitial_page->GetMainFrame()->GetRenderViewHost(); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); - content::RenderFrameHost* rfh = interstitial_page->GetMainFrame(); - ASSERT_TRUE(content::WaitForRenderFrameReady(rfh)); - - ASSERT_NO_FATAL_FAILURE(CheckProceedLinkExists(interstitial_rvh)); + ASSERT_NO_FATAL_FAILURE(CheckProceedLinkExists(tab)); } // Verifies that an overridable committed interstitial has a proceed link. @@ -3747,15 +3787,13 @@ CheckSecurityState(tab, net::CERT_STATUS_DATE_INVALID, security_state::DANGEROUS, AuthState::SHOWING_ERROR); - content::RenderFrameHost* rfh = tab->GetMainFrame(); - ASSERT_TRUE(content::WaitForRenderFrameReady(rfh)); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); - content::RenderViewHost* rvh = tab->GetRenderViewHost(); - ASSERT_NO_FATAL_FAILURE(CheckProceedLinkExists(rvh)); + ASSERT_NO_FATAL_FAILURE(CheckProceedLinkExists(tab)); } // Verifies that a non-overridable interstitial does not have a proceed link. -IN_PROC_BROWSER_TEST_F(SSLUITestHSTS, TestInterstitialOptionsNonOverridable) { +IN_PROC_BROWSER_TEST_P(SSLUITestHSTS, TestInterstitialOptionsNonOverridable) { ASSERT_TRUE(https_server_expired_.Start()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -3772,14 +3810,8 @@ tab, net::CERT_STATUS_DATE_INVALID | net::CERT_STATUS_COMMON_NAME_INVALID, AuthState::SHOWING_INTERSTITIAL); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - content::RenderViewHost* interstitial_rvh = - interstitial_page->GetMainFrame()->GetRenderViewHost(); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); - content::RenderFrameHost* rfh = interstitial_page->GetMainFrame(); - ASSERT_TRUE(content::WaitForRenderFrameReady(rfh)); int result = security_interstitials::CMD_ERROR; const std::string javascript = base::StringPrintf( "domAutomationController.send(" @@ -3787,14 +3819,17 @@ "? (%d) : (%d))", security_interstitials::CMD_TEXT_NOT_FOUND, security_interstitials::CMD_TEXT_FOUND); - ASSERT_TRUE(content::ExecuteScriptAndExtractInt(interstitial_rvh, javascript, - &result)); + ASSERT_TRUE(content::ExecuteScriptAndExtractInt( + AreCommittedInterstitialsEnabled() + ? tab->GetMainFrame() + : tab->GetInterstitialPage()->GetMainFrame(), + javascript, &result)); EXPECT_EQ(security_interstitials::CMD_TEXT_NOT_FOUND, result); } // Verifies that links in the interstitial open in a new tab. // https://crbug.com/717616 -IN_PROC_BROWSER_TEST_F(SSLUITest, TestInterstitialLinksOpenInNewTab) { +IN_PROC_BROWSER_TEST_P(SSLUITest, TestInterstitialLinksOpenInNewTab) { ASSERT_TRUE(https_server_.Start()); ASSERT_TRUE(https_server_expired_.Start()); @@ -3803,22 +3838,17 @@ ui_test_utils::NavigateToURL( browser(), https_server_expired_.GetURL("/ssl/google.html")); WaitForInterstitial(browser()->tab_strip_model()->GetActiveWebContents()); - InterstitialPage* interstitial_page = interstitial_tab->GetInterstitialPage(); - ASSERT_TRUE( - content::WaitForRenderFrameReady(interstitial_page->GetMainFrame())); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(interstitial_tab)); CheckAuthenticationBrokenState(interstitial_tab, net::CERT_STATUS_DATE_INVALID, AuthState::SHOWING_INTERSTITIAL); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); content::TestNavigationObserver nav_observer(nullptr); nav_observer.StartWatchingNewWebContents(); - SSLBlockingPage* ssl_interstitial = - static_cast<SSLBlockingPage*>(interstitial_page->GetDelegateForTesting()); + SSLBlockingPage* ssl_interstitial = GetSSLBlockingPage(interstitial_tab); security_interstitials::SecurityInterstitialControllerClient* client = - GetControllerClientFromInterstitialPage(ssl_interstitial); + GetControllerClientFromSSLBlockingPage(ssl_interstitial); // Mock out the help center URL so that our test will hit the test server // instead of a real server. @@ -3832,13 +3862,8 @@ EXPECT_EQ(1, browser()->tab_strip_model()->count()); - int result = security_interstitials::CMD_ERROR; - const std::string javascript = - base::StringPrintf("window.domAutomationController.send(%d);", - security_interstitials::CMD_OPEN_HELP_CENTER); - ASSERT_TRUE(content::ExecuteScriptAndExtractInt( - interstitial_page->GetMainFrame(), javascript, &result)); - EXPECT_EQ(security_interstitials::CMD_OPEN_HELP_CENTER, result); + SendInterstitialCommand(interstitial_tab, + security_interstitials::CMD_OPEN_HELP_CENTER); nav_observer.Wait(); @@ -3851,8 +3876,7 @@ // Verifies that switching tabs, while showing interstitial page, will not // affect the visibility of the interstitial. // https://crbug.com/381439 -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - InterstitialNotAffectedByHideShow) { +IN_PROC_BROWSER_TEST_P(SSLUITest, InterstitialNotAffectedByHideShow) { ASSERT_TRUE(https_server_expired_.Start()); ASSERT_TRUE(https_server_.Start()); @@ -3879,7 +3903,7 @@ // through the interstitial, the decision to proceed is initially remembered. // However, if this is followed by another visit, and a good certificate // is seen for the same host, the original exception is forgotten. -IN_PROC_BROWSER_TEST_F(SSLUITest, BadCertFollowedByGoodCert) { +IN_PROC_BROWSER_TEST_P(SSLUITest, BadCertFollowedByGoodCert) { // It is necessary to use |https_server_expired_| rather than // |https_server_mismatched| because the former shares a host with // |https_server_| and cert exceptions are per host. @@ -3935,7 +3959,7 @@ // Tests that the SSLStatus of a navigation entry for an SSL // interstitial matches the navigation entry once the interstitial is // clicked through. https://crbug.com/529456 -IN_PROC_BROWSER_TEST_F(SSLUITest, +IN_PROC_BROWSER_TEST_P(SSLUITest, SSLStatusMatchesOnInterstitialAndAfterProceed) { ASSERT_TRUE(https_server_expired_.Start()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -3944,7 +3968,7 @@ ui_test_utils::NavigateToURL( browser(), https_server_expired_.GetURL("/ssl/google.html")); WaitForInterstitial(tab); - EXPECT_TRUE(tab->ShowingInterstitialPage()); + EXPECT_TRUE(IsShowingInterstitial(tab)); content::NavigationEntry* entry = tab->GetController().GetActiveEntry(); ASSERT_TRUE(entry); @@ -3963,7 +3987,7 @@ // As above, but for a bad clock interstitial. Tests that a clock // interstitial's SSLStatus matches the SSLStatus of the HTTPS page // after proceeding through a normal SSL interstitial. -IN_PROC_BROWSER_TEST_F(SSLUITest, +IN_PROC_BROWSER_TEST_P(SSLUITest, SSLStatusMatchesonClockInterstitialAndAfterProceed) { ASSERT_TRUE(https_server_expired_.Start()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -3979,10 +4003,7 @@ ui_test_utils::NavigateToURL(browser(), https_server_expired_.GetURL("/title1.html")); WaitForInterstitial(tab); - InterstitialPage* clock_interstitial = tab->GetInterstitialPage(); - ASSERT_TRUE(clock_interstitial); - EXPECT_EQ(BadClockBlockingPage::kTypeForTesting, - clock_interstitial->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectBadClockInterstitial(tab)); // Grab the SSLStatus on the clock interstitial. content::NavigationEntry* entry = tab->GetController().GetActiveEntry(); @@ -3995,12 +4016,9 @@ ui_test_utils::NavigateToURL(browser(), https_server_expired_.GetURL("/title1.html")); WaitForInterstitial(tab); - InterstitialPage* ssl_interstitial = tab->GetInterstitialPage(); - ASSERT_TRUE(ssl_interstitial); - EXPECT_EQ(SSLBlockingPage::kTypeForTesting, - ssl_interstitial->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); ProceedThroughInterstitial(tab); - EXPECT_FALSE(tab->ShowingInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(tab)); // Grab the SSLStatus from the page and check that it is the same as // on the clock interstitial. @@ -4181,6 +4199,7 @@ ~SSLNetworkTimeBrowserTest() override {} void SetUpCommandLine(base::CommandLine* command_line) override { + SSLUITest::SetUpCommandLine(command_line); command_line->AppendSwitchASCII( switches::kForceFieldTrials, "SSLNetworkTimeBrowserTestFieldTrial/Enabled/"); @@ -4239,9 +4258,13 @@ DISALLOW_COPY_AND_ASSIGN(SSLNetworkTimeBrowserTest); }; +INSTANTIATE_TEST_CASE_P(, + SSLNetworkTimeBrowserTest, + ::testing::Values(false, true)); + // Tests that if an on-demand network time fetch returns that the clock // is okay, a normal SSL interstitial is shown. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, OnDemandFetchClockOk) { +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, OnDemandFetchClockOk) { ASSERT_TRUE(https_server_expired_.Start()); // Use a testing clock set to the time that GoodTimeResponseHandler // returns, to simulate the system clock matching the network time. @@ -4282,16 +4305,13 @@ observer.Wait(); WaitForInterstitial(contents); - EXPECT_TRUE(contents->ShowingInterstitialPage()); - InterstitialPage* interstitial_page = contents->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + EXPECT_TRUE(IsShowingInterstitial(contents)); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(contents)); } // Tests that if an on-demand network time fetch returns that the clock // is wrong, a bad clock interstitial is shown. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, OnDemandFetchClockWrong) { +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, OnDemandFetchClockWrong) { ASSERT_TRUE(https_server_expired_.Start()); // Use a testing clock set to a time that is different from what // GoodTimeResponseHandler returns, simulating a system clock that is @@ -4335,16 +4355,12 @@ observer.Wait(); WaitForInterstitial(contents); - EXPECT_TRUE(contents->ShowingInterstitialPage()); - InterstitialPage* interstitial_page = contents->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - ASSERT_EQ(BadClockBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectBadClockInterstitial(contents)); } // Tests that if the timeout expires before the network time fetch // returns, then a normal SSL interstitial is shown. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, TimeoutExpiresBeforeFetchCompletes) { ASSERT_TRUE(https_server_expired_.Start()); // Set the timer to fire immediately. @@ -4355,11 +4371,7 @@ ASSERT_TRUE(contents); WaitForInterstitial(contents); - EXPECT_TRUE(contents->ShowingInterstitialPage()); - InterstitialPage* interstitial_page = contents->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(contents)); // Navigate away, and then trigger the network time response; no crash should // occur. @@ -4371,7 +4383,7 @@ // Tests that if the user stops the page load before either the network // time fetch completes or the timeout expires, then there is no interstitial. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, StopBeforeTimeoutExpires) { +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, StopBeforeTimeoutExpires) { ASSERT_TRUE(https_server_expired_.Start()); // Set the timer to a long delay. SSLErrorHandler::SetInterstitialDelayForTesting( @@ -4395,7 +4407,7 @@ // Make sure that the |SSLErrorHandler| is deleted. EXPECT_FALSE(SSLErrorHandler::FromWebContents(contents)); - EXPECT_FALSE(contents->ShowingInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(contents)); EXPECT_FALSE(contents->IsLoading()); // Navigate away, and then trigger the network time response; no crash should @@ -4408,7 +4420,7 @@ // Tests that if the user reloads the page before either the network // time fetch completes or the timeout expires, then there is no interstitial. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, ReloadBeforeTimeoutExpires) { +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, ReloadBeforeTimeoutExpires) { ASSERT_TRUE(https_server_expired_.Start()); // Set the timer to a long delay. SSLErrorHandler::SetInterstitialDelayForTesting( @@ -4443,7 +4455,7 @@ // Tests that if the user navigates away before either the network time // fetch completes or the timeout expires, then there is no // interstitial. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, NavigateAwayBeforeTimeoutExpires) { ASSERT_TRUE(https_server_expired_.Start()); ASSERT_TRUE(https_server_.Start()); @@ -4480,7 +4492,7 @@ // Tests that if the user closes the tab before the network time fetch // completes, it doesn't cause a crash. -IN_PROC_BROWSER_TEST_F(SSLNetworkTimeBrowserTest, +IN_PROC_BROWSER_TEST_P(SSLNetworkTimeBrowserTest, CloseTabBeforeNetworkFetchCompletes) { ASSERT_TRUE(https_server_expired_.Start()); // Set the timer to fire immediately. @@ -4491,11 +4503,7 @@ ASSERT_TRUE(contents); WaitForInterstitial(contents); - EXPECT_TRUE(contents->ShowingInterstitialPage()); - InterstitialPage* interstitial_page = contents->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(contents)); // Open a second tab, close the first, and then trigger the network time // response and wait for the response; no crash should occur. @@ -4553,7 +4561,8 @@ } // namespace -class CommonNameMismatchBrowserTest : public CertVerifierBrowserTest { +class CommonNameMismatchBrowserTest : public CertVerifierBrowserTest, + public testing::WithParamInterface<bool> { public: CommonNameMismatchBrowserTest() : CertVerifierBrowserTest() {} @@ -4561,6 +4570,9 @@ // Enable finch experiment for SSL common name mismatch handling. command_line->AppendSwitchASCII(switches::kForceFieldTrials, "SSLCommonNameMismatchHandling/Enabled/"); + if (GetParam()) { + command_line->AppendSwitch(switches::kCommittedInterstitials); + } } void SetUpOnMainThread() override { @@ -4578,10 +4590,14 @@ } }; +INSTANTIATE_TEST_CASE_P(, + CommonNameMismatchBrowserTest, + ::testing::Values(false, true)); + // Visit the URL www.mail.example.com on a server that presents a valid // certificate for mail.example.com. Verify that the page navigates to // mail.example.com. -IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, +IN_PROC_BROWSER_TEST_P(CommonNameMismatchBrowserTest, ShouldShowWWWSubdomainMismatchInterstitial) { net::EmbeddedTestServer https_server_example_domain( net::EmbeddedTestServer::TYPE_HTTPS); @@ -4641,7 +4657,7 @@ // Visit the URL example.org on a server that presents a valid certificate // for www.example.org. Verify that the page redirects to www.example.org. -IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, +IN_PROC_BROWSER_TEST_P(CommonNameMismatchBrowserTest, CheckWWWSubdomainMismatchInverse) { net::EmbeddedTestServer https_server_example_domain( net::EmbeddedTestServer::TYPE_HTTPS); @@ -4711,7 +4727,7 @@ // valid certificate for www.example.org. In this case, www.example.org // redirects to http://example.org, and the SSL error should not be redirected // to this URL. -IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, +IN_PROC_BROWSER_TEST_P(CommonNameMismatchBrowserTest, WWWSubdomainMismatch_StopOnRedirects) { net::EmbeddedTestServer https_server_example_domain( net::EmbeddedTestServer::TYPE_HTTPS); @@ -4774,7 +4790,7 @@ // - No suggested URL check results arrive, causing the tab to appear as loading // indefinitely (also because the timer has a long timeout). // - Stopping the page load shouldn't result in any interstitials. -IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, +IN_PROC_BROWSER_TEST_P(CommonNameMismatchBrowserTest, InterstitialStopNavigationWhileLoading) { net::EmbeddedTestServer https_server_example_domain( net::EmbeddedTestServer::TYPE_HTTPS); @@ -4831,13 +4847,13 @@ SSLErrorHandler::FromWebContents(contents); // Make sure that the |SSLErrorHandler| is deleted. EXPECT_FALSE(ssl_error_handler); - EXPECT_FALSE(contents->ShowingInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(contents)); EXPECT_FALSE(contents->IsLoading()); } // Same as above, but instead of stopping, the loading page is reloaded. The end // result is the same. (i.e. page load stops, no interstitials shown) -IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, +IN_PROC_BROWSER_TEST_P(CommonNameMismatchBrowserTest, InterstitialReloadNavigationWhileLoading) { net::EmbeddedTestServer https_server_example_domain( net::EmbeddedTestServer::TYPE_HTTPS); @@ -4892,13 +4908,13 @@ SSLErrorHandler::FromWebContents(contents); // Make sure that the |SSLErrorHandler| is deleted. EXPECT_FALSE(ssl_error_handler); - EXPECT_FALSE(contents->ShowingInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(contents)); EXPECT_FALSE(contents->IsLoading()); } // Same as above, but instead of reloading, the page is navigated away. The // new page should load, and no interstitials should be shown. -IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, +IN_PROC_BROWSER_TEST_P(CommonNameMismatchBrowserTest, InterstitialNavigateAwayWhileLoading) { net::EmbeddedTestServer https_server_example_domain( net::EmbeddedTestServer::TYPE_HTTPS); @@ -4955,7 +4971,7 @@ SSLErrorHandler::FromWebContents(contents); // Make sure that the |SSLErrorHandler| is deleted. EXPECT_FALSE(ssl_error_handler); - EXPECT_FALSE(contents->ShowingInterstitialPage()); + EXPECT_FALSE(IsShowingInterstitial(contents)); EXPECT_FALSE(contents->IsLoading()); } @@ -5004,7 +5020,7 @@ AuthState::SHOWING_INTERSTITIAL); } -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, RestoreHasSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, RestoreHasSSLState) { ASSERT_TRUE(https_server_.Start()); GURL url(https_server_.GetURL("/ssl/google.html")); ui_test_utils::NavigateToURL(browser(), url); @@ -5070,7 +5086,7 @@ // Simulate a browser-initiated in-page navigation in a restored tab. // https://crbug.com/662267 -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, +IN_PROC_BROWSER_TEST_P(SSLUITest, BrowserInitiatedExistingPageAfterRestoreHasSSLState) { SetupRestoredTabWithNavigation(&https_server_, browser()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -5082,7 +5098,7 @@ } // Simulate a renderer-initiated in-page navigation in a restored tab. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, +IN_PROC_BROWSER_TEST_P(SSLUITest, RendererInitiatedExistingPageAfterRestoreHasSSLState) { SetupRestoredTabWithNavigation(&https_server_, browser()); WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); @@ -5123,7 +5139,7 @@ // Check that SSL state isn't stale when navigating to an existing page that // gives a different response. This covers the case of going from http to // https. http://crbug.com/792221 -IN_PROC_BROWSER_TEST_F(SSLUITest, ExistingPageHTTPToHTTPSSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, ExistingPageHTTPToHTTPSSSLState) { ASSERT_TRUE(https_server_.Start()); int count = 0; std::string relative_url = "/foo"; @@ -5147,7 +5163,7 @@ // Check that SSL state isn't stale when navigating to an existing page that // gives a different response. This covers the case of going from https to // http URL. http://crbug.com/792221 -IN_PROC_BROWSER_TEST_F(SSLUITest, ExistingPageHTTPSToHTTPSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, ExistingPageHTTPSToHTTPSSLState) { ASSERT_TRUE(embedded_test_server()->Start()); int count = 0; std::string relative_url = "/foo"; @@ -5177,8 +5193,7 @@ // Checks that a restore followed immediately by a history navigation doesn't // lose SSL state. // Disabled since this is a test for bug 738177. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - DISABLED_RestoreThenNavigateHasSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, DISABLED_RestoreThenNavigateHasSSLState) { // This race condition only happens with PlzNavigate. if (!content::IsBrowserSideNavigationEnabled()) return; @@ -5208,8 +5223,7 @@ // could happen when the user's login is expired and the server redirects them // to a login page. This will be considered a same document navigation but we // do want to update the SSL state. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - SameDocumentHasSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, SameDocumentHasSSLState) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -5248,7 +5262,7 @@ // Checks that if a redirect occurs while the page is loading, the SSL state // reflects the final URL. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, ClientRedirectSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, ClientRedirectSSLState) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -5263,8 +5277,7 @@ // Checks that if a redirect occurs while the page is loading from a mixed // content to a valid HTTPS page, the SSL state reflects the final URL. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - ClientRedirectFromMixedContentSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, ClientRedirectFromMixedContentSSLState) { ASSERT_TRUE(https_server_.Start()); GURL url = GURL( @@ -5279,8 +5292,7 @@ // Checks that if a redirect occurs while the page is loading from a valid HTTPS // page to a mixed content page, the SSL state reflects the final URL. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - ClientRedirectToMixedContentSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, ClientRedirectToMixedContentSSLState) { ASSERT_TRUE(embedded_test_server()->Start()); ASSERT_TRUE(https_server_.Start()); @@ -5295,8 +5307,7 @@ } // Checks that same-document navigations during page load preserve SSL state. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - SameDocumentNavigationDuringLoadSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, SameDocumentNavigationDuringLoadSSLState) { ASSERT_TRUE(https_server_.Start()); ui_test_utils::NavigateToURL( @@ -5308,8 +5319,7 @@ // Checks that same-document navigations after the page load preserve SSL // state. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, - SameDocumentNavigationAfterLoadSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, SameDocumentNavigationAfterLoadSSLState) { ASSERT_TRUE(https_server_.Start()); ui_test_utils::NavigateToURL(browser(), @@ -5320,7 +5330,7 @@ } // Checks that navigations after pushState maintain the SSL status. -IN_PROC_BROWSER_TEST_P(SSLUITestTransientAndCommitted, PushStateSSLState) { +IN_PROC_BROWSER_TEST_P(SSLUITest, PushStateSSLState) { ASSERT_TRUE(https_server_.Start()); ui_test_utils::NavigateToURL(browser(), @@ -5341,7 +5351,7 @@ // Regression test for http://crbug.com/635833 (crash when a window with no // NavigationEntry commits). -IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreLocalhostCertErrors, +IN_PROC_BROWSER_TEST_P(SSLUITestIgnoreLocalhostCertErrors, NoCrashOnLoadWithNoNavigationEntry) { ASSERT_TRUE(embedded_test_server()->Start()); @@ -5369,7 +5379,7 @@ // Tests that the captive portal certificate list is not used when the feature // is disabled via Finch. The list is passed to SSLErrorHandler via a proto. -IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListTest, Disabled) { +IN_PROC_BROWSER_TEST_P(SSLUICaptivePortalListTest, Disabled) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( {} /* enabled */, {kCaptivePortalCertificateList} /* disabled */); @@ -5392,9 +5402,7 @@ browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that the histogram for the SSL interstitial was recorded. @@ -5410,7 +5418,7 @@ // Tests that the captive portal certificate list is used when the feature // is enabled via Finch. The list is passed to SSLErrorHandler via a proto. -IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListTest, Enabled_FromProto) { +IN_PROC_BROWSER_TEST_P(SSLUICaptivePortalListTest, Enabled_FromProto) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( {kCaptivePortalCertificateList} /* enabled */, {} /* disabled */); @@ -5433,9 +5441,7 @@ browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectCaptivePortalInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histogram for the captive portal cert was recorded. @@ -5451,7 +5457,7 @@ // Tests the scenario where the OS reports a captive portal. A captive portal // interstitial should be displayed. -IN_PROC_BROWSER_TEST_F(SSLUITest, OSReportsCaptivePortal) { +IN_PROC_BROWSER_TEST_P(SSLUITest, OSReportsCaptivePortal) { ASSERT_TRUE(https_server_mismatched_.Start()); base::HistogramTester histograms; @@ -5464,9 +5470,7 @@ browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectCaptivePortalInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histogram for the captive portal cert was recorded. @@ -5483,7 +5487,7 @@ // Tests the scenario where the OS reports a captive portal but captive portal // interstitial feature is disabled. A captive portal interstitial should not be // displayed. -IN_PROC_BROWSER_TEST_F(SSLUITest, OSReportsCaptivePortal_FeatureDisabled) { +IN_PROC_BROWSER_TEST_P(SSLUITest, OSReportsCaptivePortal_FeatureDisabled) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( {} /* enabled */, {kCaptivePortalInterstitial} /* disabled */); @@ -5500,9 +5504,7 @@ browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histogram for the SSL interstitial was recorded. @@ -5550,7 +5552,8 @@ // embedded test server, but the test server can only serve a limited number of // predefined certificates. class SSLUICaptivePortalListResourceBundleTest - : public CertVerifierBrowserTest { + : public CertVerifierBrowserTest, + public testing::WithParamInterface<bool> { public: SSLUICaptivePortalListResourceBundleTest() : CertVerifierBrowserTest(), @@ -5558,6 +5561,12 @@ https_server_.ServeFilesFromSourceDirectory(base::FilePath(kDocRoot)); } + void SetUpCommandLine(base::CommandLine* command_line) override { + if (GetParam()) { + command_line->AppendSwitch(switches::kCommittedInterstitials); + } + } + void SetUp() override { CertVerifierBrowserTest::SetUp(); SSLErrorHandler::ResetConfigForTesting(); @@ -5588,9 +5597,7 @@ ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that the histogram for the captive portal cert was recorded. @@ -5628,11 +5635,15 @@ net::EmbeddedTestServer https_server_; }; +INSTANTIATE_TEST_CASE_P(, + SSLUICaptivePortalListResourceBundleTest, + ::testing::Values(false, true)); + } // namespace // Same as CaptivePortalCertificateList_Enabled_FromProto, but this time the // cert's SPKI hash is listed in ssl_error_assistant.asciipb. -IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest, Enabled) { +IN_PROC_BROWSER_TEST_P(SSLUICaptivePortalListResourceBundleTest, Enabled) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( {kCaptivePortalCertificateList} /* enabled */, {} /* disabled */); @@ -5650,9 +5661,7 @@ ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectCaptivePortalInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histogram for the captive portal cert was recorded. @@ -5669,7 +5678,7 @@ // Same as SSLUICaptivePortalListResourceBundleTest. Enabled, but this time the // proto is dynamically updated (e.g. by the component updater). The dynamic // update should always override the proto loaded from the resource bundle. -IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest, +IN_PROC_BROWSER_TEST_P(SSLUICaptivePortalListResourceBundleTest, Enabled_DynamicUpdate) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( @@ -5700,9 +5709,7 @@ ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that the histogram was recorded for an SSL interstitial. @@ -5732,9 +5739,7 @@ ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectCaptivePortalInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histogram was recorded for a captive portal interstitial. @@ -5764,9 +5769,7 @@ ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectCaptivePortalInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histogram was recorded for a captive portal interstitial. @@ -5784,7 +5787,7 @@ // Same as SSLUICaptivePortalNameMismatchTest, but this time the error is // authority-invalid. Captive portal interstitial should not be shown. -IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest, +IN_PROC_BROWSER_TEST_P(SSLUICaptivePortalListResourceBundleTest, Enabled_AuthorityInvalid) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( @@ -5797,7 +5800,7 @@ // Same as SSLUICaptivePortalListResourceBundleTest.Enabled_AuthorityInvalid, // but this time there are two errors (name mismatch + weak key). Captive portal // interstitial should not be shown when name mismatch isn't the only error. -IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalListResourceBundleTest, +IN_PROC_BROWSER_TEST_P(SSLUICaptivePortalListResourceBundleTest, Enabled_NameMismatchAndWeakKey) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures( @@ -5818,7 +5821,8 @@ char kTestMITMSoftwareName[] = "Misconfigured Firewall"; -class SSLUIMITMSoftwareTest : public CertVerifierBrowserTest { +class SSLUIMITMSoftwareTest : public CertVerifierBrowserTest, + public testing::WithParamInterface<bool> { public: SSLUIMITMSoftwareTest() : CertVerifierBrowserTest(), @@ -5835,6 +5839,12 @@ base::RetainedRef(browser()->profile()->GetRequestContext()))); } + void SetUpCommandLine(base::CommandLine* command_line) override { + if (GetParam()) { + command_line->AppendSwitch(switches::kCommittedInterstitials); + } + } + // Set up the cert verifier to return the error passed in as the cert_error // parameter. void SetUpCertVerifier(net::CertStatus cert_error) { @@ -5890,9 +5900,7 @@ SSLInterstitialTimerObserver interstitial_timer_observer(tab); ui_test_utils::NavigateToURL(browser(), GetHSTSTestURL()); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(MITMSoftwareBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectMITMInterstitial(tab)); EXPECT_FALSE(interstitial_timer_observer.timer_started()); // Check that the histograms for the MITM software interstitial were @@ -5921,9 +5929,7 @@ SSLInterstitialTimerObserver interstitial_timer_observer(tab); ui_test_utils::NavigateToURL(browser(), GetHSTSTestURL()); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that a MITM software interstitial was not recorded in histogram. @@ -5952,6 +5958,10 @@ DISALLOW_COPY_AND_ASSIGN(SSLUIMITMSoftwareTest); }; +INSTANTIATE_TEST_CASE_P(, + SSLUIMITMSoftwareTest, + ::testing::Values(false, true)); + // The SSLUIMITMSoftwareEnabled and Disabled test classes exist so that the // scoped feature list can be instantiated in the set up method of the class // rather than in the test itself. Bug crbug.com/713390 was causing some of the @@ -5976,6 +5986,10 @@ DISALLOW_COPY_AND_ASSIGN(SSLUIMITMSoftwareEnabledTest); }; +INSTANTIATE_TEST_CASE_P(, + SSLUIMITMSoftwareEnabledTest, + ::testing::Values(false, true)); + class SSLUIMITMSoftwareDisabledTest : public SSLUIMITMSoftwareTest { public: SSLUIMITMSoftwareDisabledTest() {} @@ -5993,11 +6007,15 @@ DISALLOW_COPY_AND_ASSIGN(SSLUIMITMSoftwareDisabledTest); }; +INSTANTIATE_TEST_CASE_P(, + SSLUIMITMSoftwareDisabledTest, + ::testing::Values(false, true)); + } // namespace // Tests that the MITM software interstitial is not displayed when the feature // is disabled by Finch. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareDisabledTest, DisabledWithFinch) { +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareDisabledTest, DisabledWithFinch) { SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID); SetUpMITMSoftwareCertList(kLargeVersionId); TestNoMITMSoftwareInterstitial(); @@ -6005,7 +6023,7 @@ // Tests that the MITM software interstitial is displayed when the feature is // enabled by Finch. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, EnabledWithFinch) { +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, EnabledWithFinch) { SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID); SetUpMITMSoftwareCertList(kLargeVersionId); TestMITMSoftwareInterstitial(); @@ -6014,7 +6032,7 @@ // Tests that if a certificates matches the common name of a known MITM software // cert on the list but not the organization name, the MITM software // interstitial will not be displayed. -IN_PROC_BROWSER_TEST_F( +IN_PROC_BROWSER_TEST_P( SSLUIMITMSoftwareEnabledTest, CertificateCommonNameMatchOnly_NoMITMSoftwareInterstitial) { base::HistogramTester histograms; @@ -6042,9 +6060,7 @@ SSLInterstitialTimerObserver interstitial_timer_observer(tab); ui_test_utils::NavigateToURL(browser(), GetHSTSTestURL()); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that a MITM software interstitial was not recorded in histogram. @@ -6062,7 +6078,7 @@ // Tests that if a certificates matches the organization name of a known MITM // software cert on the list but not the common name, the MITM software // interstitial will not be displayed. -IN_PROC_BROWSER_TEST_F( +IN_PROC_BROWSER_TEST_P( SSLUIMITMSoftwareEnabledTest, CertificateOrganizationMatchOnly_NoMITMSoftwareInterstitial) { base::HistogramTester histograms; @@ -6090,9 +6106,7 @@ SSLInterstitialTimerObserver interstitial_timer_observer(tab); ui_test_utils::NavigateToURL(browser(), GetHSTSTestURL()); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that a MITM software interstitial was not recorded in histogram. @@ -6109,7 +6123,7 @@ // Tests that if the certificate does not match any entry on the list of known // MITM software, the MITM software interstitial will not be displayed. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, NonMatchingCertificate_NoMITMSoftwareInterstitial) { base::HistogramTester histograms; @@ -6136,9 +6150,7 @@ SSLInterstitialTimerObserver interstitial_timer_observer(tab); ui_test_utils::NavigateToURL(browser(), GetHSTSTestURL()); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that a MITM software interstitial was not recorded in histogram. @@ -6155,7 +6167,7 @@ // Tests that if there is more than one error on the certificate the MITM // software interstitial will not be displayed. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, TwoCertErrors_NoMITMSoftwareInterstitial) { SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID | net::CERT_STATUS_COMMON_NAME_INVALID); @@ -6165,7 +6177,7 @@ // Tests that a certificate error other than |CERT_STATUS_AUTHORITY_INVALID| // will not trigger the MITM software interstitial. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, WrongCertError_NoMITMSoftwareInterstitial) { SetUpCertVerifier(net::CERT_STATUS_COMMON_NAME_INVALID); SetUpMITMSoftwareCertList(kLargeVersionId); @@ -6174,7 +6186,7 @@ // Tests that if the error on the certificate served is overridable the MITM // software interstitial will not be displayed. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, OverridableError_NoMITMSoftwareInterstitial) { base::HistogramTester histograms; @@ -6190,9 +6202,7 @@ ui_test_utils::NavigateToURL(browser(), https_server()->GetURL("/ssl/blank_page.html")); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); EXPECT_TRUE(interstitial_timer_observer.timer_started()); // Check that the histogram for an overridable SSL interstitial was @@ -6210,7 +6220,7 @@ // Tests that the correct strings are displayed on the interstitial in the // enterprise managed case. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, EnterpriseManaged) { +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, EnterpriseManaged) { SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID); SSLErrorHandler::SetEnterpriseManagedForTesting(true); ASSERT_TRUE(SSLErrorHandler::IsEnterpriseManagedFlagSetForTesting()); @@ -6218,13 +6228,6 @@ SetUpMITMSoftwareCertList(kLargeVersionId); TestMITMSoftwareInterstitial(); - InterstitialPage* interstitial_page = browser() - ->tab_strip_model() - ->GetActiveWebContents() - ->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - EXPECT_TRUE(WaitForRenderFrameReady(interstitial_page->GetMainFrame())); - const std::string expected_primary_paragraph = l10n_util::GetStringFUTF8( IDS_MITM_SOFTWARE_PRIMARY_PARAGRAPH_ENTERPRISE, net::EscapeForHTML(base::UTF8ToUTF16(kTestMITMSoftwareName))); @@ -6233,15 +6236,22 @@ net::EscapeForHTML(base::UTF8ToUTF16(kTestMITMSoftwareName)), l10n_util::GetStringUTF16(IDS_MITM_SOFTWARE_EXPLANATION)); + WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial_page, expected_primary_paragraph)); + AreCommittedInterstitialsEnabled() + ? tab->GetMainFrame() + : tab->GetInterstitialPage()->GetMainFrame(), + expected_explanation)); EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial_page, expected_explanation)); + AreCommittedInterstitialsEnabled() + ? tab->GetMainFrame() + : tab->GetInterstitialPage()->GetMainFrame(), + expected_primary_paragraph)); } // Tests that the correct strings are displayed on the interstitial in the // non-enterprise managed case. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, NotEnterpriseManaged) { +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, NotEnterpriseManaged) { SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID); SSLErrorHandler::SetEnterpriseManagedForTesting(false); ASSERT_TRUE(SSLErrorHandler::IsEnterpriseManagedFlagSetForTesting()); @@ -6249,13 +6259,6 @@ SetUpMITMSoftwareCertList(kLargeVersionId); TestMITMSoftwareInterstitial(); - InterstitialPage* interstitial_page = browser() - ->tab_strip_model() - ->GetActiveWebContents() - ->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - EXPECT_TRUE(WaitForRenderFrameReady(interstitial_page->GetMainFrame())); - // Don't check the primary paragraph in the non-enterprise case, because it // has escaped HTML characters which throw an error. const std::string expected_explanation = l10n_util::GetStringFUTF8( @@ -6263,14 +6266,18 @@ net::EscapeForHTML(base::UTF8ToUTF16(kTestMITMSoftwareName)), l10n_util::GetStringUTF16(IDS_MITM_SOFTWARE_EXPLANATION)); + WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial_page, expected_explanation)); + AreCommittedInterstitialsEnabled() + ? tab->GetMainFrame() + : tab->GetInterstitialPage()->GetMainFrame(), + expected_explanation)); } // Initialize MITMSoftware certificate list but set the version_id to zero. This // less than the version_id of the local resource bundle, so the dynamic // update will be ignored and a non-MITM interstitial will be shown. -IN_PROC_BROWSER_TEST_F(SSLUIMITMSoftwareEnabledTest, +IN_PROC_BROWSER_TEST_P(SSLUIMITMSoftwareEnabledTest, IgnoreDynamicUpdateWithSmallVersionId) { SetUpCertVerifier(net::CERT_STATUS_AUTHORITY_INVALID); SSLErrorHandler::SetEnterpriseManagedForTesting(false); @@ -6280,7 +6287,8 @@ TestNoMITMSoftwareInterstitial(); } -class SuperfishSSLUITest : public CertVerifierBrowserTest { +class SuperfishSSLUITest : public CertVerifierBrowserTest, + public testing::WithParamInterface<bool> { public: SuperfishSSLUITest() : CertVerifierBrowserTest(), @@ -6293,6 +6301,12 @@ ASSERT_TRUE(https_server_.Start()); } + void SetUpCommandLine(base::CommandLine* command_line) override { + if (GetParam()) { + command_line->AppendSwitch(switches::kCommittedInterstitials); + } + } + protected: void SetUpCertVerifier(bool use_superfish_cert) { net::CertVerifyResult verify_result; @@ -6376,9 +6390,11 @@ } }; +INSTANTIATE_TEST_CASE_P(, SuperfishSSLUITest, ::testing::Values(false, true)); + // Tests that the Superfish histogram is recorded properly when the Superfish // certificate is present. -IN_PROC_BROWSER_TEST_F(SuperfishSSLUITest, SuperfishRecorded) { +IN_PROC_BROWSER_TEST_P(SuperfishSSLUITest, SuperfishRecorded) { SetUpCertVerifier(true /* use superfish cert */); GURL url(https_server_.GetURL("/ssl/google.html")); base::HistogramTester histograms; @@ -6389,7 +6405,7 @@ // Tests that the Superfish histogram is recorded properly when the Superfish // certificate is not present. -IN_PROC_BROWSER_TEST_F(SuperfishSSLUITest, NoSuperfishRecorded) { +IN_PROC_BROWSER_TEST_P(SuperfishSSLUITest, NoSuperfishRecorded) { SetUpCertVerifier(false /* use superfish cert */); base::HistogramTester histograms; ui_test_utils::NavigateToURL(browser(), @@ -6400,7 +6416,7 @@ // Tests that the Superfish interstitial is shown when the Finch feature is // enabled and the Superfish certificate is present. -IN_PROC_BROWSER_TEST_F(SuperfishSSLUITest, SuperfishInterstitial) { +IN_PROC_BROWSER_TEST_P(SuperfishSSLUITest, SuperfishInterstitial) { base::HistogramTester histograms; const char kDecisionHistogram[] = "interstitial.superfish.decision"; const char kInteractionHistogram[] = "interstitial.superfish.interaction"; @@ -6415,18 +6431,7 @@ content::WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - EXPECT_TRUE(WaitForRenderFrameReady(interstitial_page->GetMainFrame())); - EXPECT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - - // Look for keywords on the page to check that the Superfish interstitial is - // showing. - const std::string expected_title = - l10n_util::GetStringUTF8(IDS_SSL_SUPERFISH_HEADING); - EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial_page, expected_title)); + ASSERT_NO_FATAL_FAILURE(ExpectSuperfishInterstitial(tab)); // Check that the correct histograms were recorded. histograms.ExpectTotalCount(kDecisionHistogram, 1); @@ -6440,7 +6445,7 @@ // Tests that the Superfish interstitial is not shown when the Finch feature is // disabled. -IN_PROC_BROWSER_TEST_F(SuperfishSSLUITest, SuperfishInterstitialDisabled) { +IN_PROC_BROWSER_TEST_P(SuperfishSSLUITest, SuperfishInterstitialDisabled) { base::test::ScopedFeatureList scoped_feature_list; scoped_feature_list.InitWithFeatures({} /* enabled */, {kSuperfishInterstitial} /* disabled */); @@ -6450,18 +6455,7 @@ content::WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents(); WaitForInterstitial(tab); - InterstitialPage* interstitial_page = tab->GetInterstitialPage(); - ASSERT_TRUE(interstitial_page); - EXPECT_TRUE(WaitForRenderFrameReady(interstitial_page->GetMainFrame())); - EXPECT_EQ(SSLBlockingPage::kTypeForTesting, - interstitial_page->GetDelegateForTesting()->GetTypeForTesting()); - - // Look for keywords on the page to check that the Superfish interstitial is - // not showing. - const std::string expected_title = - l10n_util::GetStringUTF8(IDS_SSL_V2_HEADING); - EXPECT_TRUE(chrome_browser_interstitials::IsInterstitialDisplayingText( - interstitial_page, expected_title)); + ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab)); } // Allows tests to effectively turn off CT requirements. Used by
diff --git a/chrome/browser/ssl/ssl_error_tab_helper.cc b/chrome/browser/ssl/ssl_error_tab_helper.cc index 0c1eed5d..2ae03e36 100644 --- a/chrome/browser/ssl/ssl_error_tab_helper.cc +++ b/chrome/browser/ssl/ssl_error_tab_helper.cc
@@ -20,17 +20,17 @@ auto it = blocking_pages_for_navigations_.find( navigation_handle->GetNavigationId()); - if (it == blocking_pages_for_navigations_.end()) { - blocking_page_for_currently_committed_navigation_.reset(); - return; + if (navigation_handle->HasCommitted()) { + if (it == blocking_pages_for_navigations_.end()) { + blocking_page_for_currently_committed_navigation_.reset(); + } else { + blocking_page_for_currently_committed_navigation_ = std::move(it->second); + } } - if (navigation_handle->HasCommitted()) { - blocking_page_for_currently_committed_navigation_ = std::move(it->second); - } else { - blocking_page_for_currently_committed_navigation_.reset(); + if (it != blocking_pages_for_navigations_.end()) { + blocking_pages_for_navigations_.erase(it); } - blocking_pages_for_navigations_.erase(it); } // static
diff --git a/chrome/browser/ssl/ssl_error_tab_helper_unittest.cc b/chrome/browser/ssl/ssl_error_tab_helper_unittest.cc index 8cbf2acc..d44ed71c 100644 --- a/chrome/browser/ssl/ssl_error_tab_helper_unittest.cc +++ b/chrome/browser/ssl/ssl_error_tab_helper_unittest.cc
@@ -161,4 +161,38 @@ EXPECT_TRUE(blocking_page3_destroyed); } +// Tests that the helper properly handles a navigation that finishes without +// committing. +TEST_F(SSLErrorTabHelperTest, NavigationDoesNotCommit) { + std::unique_ptr<content::NavigationHandle> committed_handle = + CreateHandle(true, false); + bool committed_blocking_page_destroyed = false; + CreateAssociatedBlockingPage(committed_handle.get(), + &committed_blocking_page_destroyed); + SSLErrorTabHelper* helper = + SSLErrorTabHelper::FromWebContents(web_contents()); + helper->DidFinishNavigation(committed_handle.get()); + EXPECT_FALSE(committed_blocking_page_destroyed); + + // Simulate a navigation that does not commit. + std::unique_ptr<content::NavigationHandle> non_committed_handle = + CreateHandle(false, false); + bool non_committed_blocking_page_destroyed = false; + CreateAssociatedBlockingPage(non_committed_handle.get(), + &non_committed_blocking_page_destroyed); + helper->DidFinishNavigation(non_committed_handle.get()); + + // The blocking page for the non-committed navigation should have been cleaned + // up, but the one for the previous committed navigation should still be + // around. + EXPECT_TRUE(non_committed_blocking_page_destroyed); + EXPECT_FALSE(committed_blocking_page_destroyed); + + // When a navigation does commit, the previous one should be cleaned up. + std::unique_ptr<content::NavigationHandle> next_committed_handle = + CreateHandle(true, false); + helper->DidFinishNavigation(next_committed_handle.get()); + EXPECT_TRUE(committed_blocking_page_destroyed); +} + } // namespace
diff --git a/chromeos/network/managed_network_configuration_handler_unittest.cc b/chromeos/network/managed_network_configuration_handler_unittest.cc index d8cb7a1..890c030 100644 --- a/chromeos/network/managed_network_configuration_handler_unittest.cc +++ b/chromeos/network/managed_network_configuration_handler_unittest.cc
@@ -249,8 +249,7 @@ ~ManagedNetworkConfigurationHandlerTest() override { network_state_handler_->Shutdown(); - managed_network_configuration_handler_->RemoveObserver(&policy_observer_); - managed_network_configuration_handler_.reset(); + ResetManagedNetworkConfigurationHandler(); network_configuration_handler_.reset(); network_profile_handler_.reset(); network_state_handler_.reset(); @@ -318,6 +317,13 @@ GetShillProfileClient()->AddEntry(profile_path, entry_path, *entry); } + void ResetManagedNetworkConfigurationHandler() { + if (!managed_network_configuration_handler_) + return; + managed_network_configuration_handler_->RemoveObserver(&policy_observer_); + managed_network_configuration_handler_.reset(); + } + private: base::MessageLoop message_loop_; @@ -930,66 +936,62 @@ EXPECT_EQ(*expected_shill_properties, *properties); } -TEST_F(ManagedNetworkConfigurationHandlerMockTest, AutoConnectDisallowed) { +TEST_F(ManagedNetworkConfigurationHandlerTest, AutoConnectDisallowed) { InitializeStandardProfiles(); + // Setup an unmanaged network. SetUpEntry("policy/shill_unmanaged_wifi2.json", kUser1ProfilePath, "wifi2_entry_path"); - // Apply the user policy with global autoconnect config and expect that - // autoconnect is disabled in the network's profile entry. - EXPECT_CALL(*mock_profile_client_, - GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _)); - - EXPECT_CALL( - *mock_profile_client_, - GetEntry(dbus::ObjectPath(kUser1ProfilePath), "wifi2_entry_path", _, _)); - std::unique_ptr<base::DictionaryValue> expected_shill_properties = test_utils::ReadTestDictionary( "policy/shill_disallow_autoconnect_on_unmanaged_wifi2.json"); - EXPECT_CALL(*mock_manager_client_, - ConfigureServiceForProfile( - dbus::ObjectPath(kUser1ProfilePath), - MatchesProperties(expected_shill_properties.get()), _, _)); - + // Apply the user policy with global autoconnect config and expect that + // autoconnect is disabled in the network's profile entry. SetPolicy(::onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_disallow_autoconnect.onc"); base::RunLoop().RunUntilIdle(); + const base::DictionaryValue* properties = + GetShillServiceClient()->GetServiceProperties("wifi2"); + ASSERT_TRUE(properties); + EXPECT_EQ(*expected_shill_properties, *properties); + // Verify that GetManagedProperties correctly augments the properties with the // global config from the user policy. - // GetManagedProperties requires the device policy to be set or explicitly // unset. - EXPECT_CALL(*mock_profile_client_, - GetProperties(dbus::ObjectPath( - NetworkProfileHandler::GetSharedProfilePath()), - _, - _)); managed_handler()->SetPolicy( ::onc::ONC_SOURCE_DEVICE_POLICY, std::string(), // no userhash base::ListValue(), // no device network policy base::DictionaryValue()); // no device global config - services_stub_.SetFakeProperties(*expected_shill_properties); - EXPECT_CALL(*mock_service_client_, - GetProperties(dbus::ObjectPath("wifi2"), _)); - - GetManagedProperties(kUser1, "wifi2"); + std::unique_ptr<base::DictionaryValue> dictionary; + managed_handler()->GetManagedProperties( + kUser1, "wifi2", + base::Bind( + [](std::unique_ptr<base::DictionaryValue>* dictionary_out, + const std::string& service_path, + const base::DictionaryValue& dictionary) { + *dictionary_out = base::DictionaryValue::From( + base::Value::ToUniquePtrValue(dictionary.Clone())); + }, + &dictionary), + base::Bind( + [](const std::string& error_name, + std::unique_ptr<base::DictionaryValue> error_data) { FAIL(); })); base::RunLoop().RunUntilIdle(); - EXPECT_EQ("wifi2", get_properties_service_path_); - + ASSERT_TRUE(dictionary.get()); std::unique_ptr<base::DictionaryValue> expected_managed_onc = test_utils::ReadTestDictionary( - "policy/managed_onc_disallow_autoconnect_on_unmanaged_wifi2.onc"); - EXPECT_TRUE(onc::test_utils::Equals(expected_managed_onc.get(), - &get_properties_result_)); + "policy/" + "managed_onc_disallow_autoconnect_on_unmanaged_wifi2.onc"); + EXPECT_EQ(*expected_managed_onc, *dictionary); } TEST_F(ManagedNetworkConfigurationHandlerTest, LateProfileLoading) { @@ -1009,41 +1011,15 @@ EXPECT_EQ(*expected_shill_properties, *properties); } -class ManagedNetworkConfigurationHandlerShutdownTest - : public ManagedNetworkConfigurationHandlerMockTest { - public: - void SetUp() override { - ManagedNetworkConfigurationHandlerMockTest::SetUp(); - ON_CALL(*mock_profile_client_, GetProperties(_, _, _)).WillByDefault( - Invoke(&ManagedNetworkConfigurationHandlerShutdownTest::GetProperties)); - } - - static void GetProperties( - const dbus::ObjectPath& profile_path, - const ShillClientHelper::DictionaryValueCallbackWithoutStatus& callback, - const ShillClientHelper::ErrorCallback& error_callback) { - base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(ManagedNetworkConfigurationHandlerShutdownTest:: - CallbackWithEmptyDictionary, - callback)); - } - - static void CallbackWithEmptyDictionary( - const ShillClientHelper::DictionaryValueCallbackWithoutStatus& callback) { - callback.Run(base::DictionaryValue()); - } -}; - -TEST_F(ManagedNetworkConfigurationHandlerShutdownTest, - DuringPolicyApplication) { +TEST_F(ManagedNetworkConfigurationHandlerTest, + ShutdownDuringPolicyApplication) { InitializeStandardProfiles(); - EXPECT_CALL(*mock_profile_client_, - GetProperties(dbus::ObjectPath(kUser1ProfilePath), _, _)); - SetPolicy(::onc::ONC_SOURCE_USER_POLICY, kUser1, "policy/policy_wifi1.onc"); - managed_network_configuration_handler_->RemoveObserver(&policy_observer_); - managed_network_configuration_handler_.reset(); + + // Reset the network configuration manager after setting policy and before + // calling RunUntilIdle to simulate shutdown during policy application. + ResetManagedNetworkConfigurationHandler(); base::RunLoop().RunUntilIdle(); }
diff --git a/chromeos/test/data/network/policy/managed_onc_disallow_autoconnect_on_unmanaged_wifi2.onc b/chromeos/test/data/network/policy/managed_onc_disallow_autoconnect_on_unmanaged_wifi2.onc index 8205bd5..af6cbd4 100644 --- a/chromeos/test/data/network/policy/managed_onc_disallow_autoconnect_on_unmanaged_wifi2.onc +++ b/chromeos/test/data/network/policy/managed_onc_disallow_autoconnect_on_unmanaged_wifi2.onc
@@ -1,23 +1,26 @@ { - "Name": { + "ConnectionState": "NotConnected", + "GUID": "wifi2", + "Name": { + "Active": "wifi2" + }, + "Source": "User", + "Type": "WiFi", + "WiFi": { + "AutoConnect": { + "Active": false, + "Effective": "UserPolicy", + "UserPolicy": false + }, + "HexSSID": { + "Active": "7769666932" // "wifi2" + }, + "SSID": { "Active": "wifi2" - }, - "Source": "User", - "Type": "WiFi", - "WiFi": { - "AutoConnect": { - "Active": false, - "Effective": "UserPolicy", - "UserPolicy": false - }, - "HexSSID": { - "Active": "7769666932" // "wifi2" - }, - "SSID": { - "Active": "wifi2" - }, - "Security": { - "Active": "WPA-PSK" - } - } + }, + "Security": { + "Active": "WPA-PSK" + } + } } +
diff --git a/chromeos/test/data/network/policy/shill_disallow_autoconnect_on_unmanaged_wifi2.json b/chromeos/test/data/network/policy/shill_disallow_autoconnect_on_unmanaged_wifi2.json index d03b86c1..0fca6bd 100644 --- a/chromeos/test/data/network/policy/shill_disallow_autoconnect_on_unmanaged_wifi2.json +++ b/chromeos/test/data/network/policy/shill_disallow_autoconnect_on_unmanaged_wifi2.json
@@ -1,8 +1,14 @@ { "AutoConnect": false, + "Device": "/device/wifi1", + "GUID": "wifi2", "Mode": "managed", + "Name": "wifi2", "Profile": "/profile/user1/shill", + "SSID": "wifi2", "SecurityClass": "psk", + "State": "idle", "Type": "wifi", + "Visible": true, "WiFi.HexSSID": "7769666932" // "wifi2" }
diff --git a/chromeos/test/data/network/policy/shill_unmanaged_wifi2.json b/chromeos/test/data/network/policy/shill_unmanaged_wifi2.json index 4b26449..b86da65 100644 --- a/chromeos/test/data/network/policy/shill_unmanaged_wifi2.json +++ b/chromeos/test/data/network/policy/shill_unmanaged_wifi2.json
@@ -1,5 +1,6 @@ { "AutoConnect": true, + "GUID": "wifi2", "Mode": "managed", "Passphrase": "user's passphrase", "Profile": "/profile/user1/shill",
diff --git a/components/policy/proto/device_management_backend.proto b/components/policy/proto/device_management_backend.proto index a363c83..a3bc015 100644 --- a/components/policy/proto/device_management_backend.proto +++ b/components/policy/proto/device_management_backend.proto
@@ -170,12 +170,21 @@ message DeviceUnregisterResponse { } -// Request from device to server to upload device EMCert -// (enterprise machine cert used for remote attestation). +// Request from device to server to upload device certificate. // GoogleDMToken MUST be in HTTP Authorization header. message DeviceCertUploadRequest { - // EMCert in X.509 format. + enum CertificateType { + // Default value for when a type is not specified. + CERTIFICATE_TYPE_UNSPECIFIED = 0; + // Enterprise machine certificate used for remote attestation. + ENTERPRISE_MACHINE_CERTIFICATE = 1; + // Enrollment certificate used to obtain an enrollment identifier. + ENTERPRISE_ENROLLMENT_CERTIFICATE = 2; + } + // Certificate in X.509 format. optional bytes device_certificate = 1; + // Type of certificate. If omitted, will be guessed from the certificate. + optional CertificateType certificate_type = 2; } // Response from server to device for cert upload request. @@ -467,6 +476,9 @@ // this topic in order to receive notifications that one or more commands are // available for execution. optional string command_invalidation_topic = 27; + + // Whether the device needs to upload an enrollment identifier to the cloud. + optional bool enrollment_id_needed = 28; } message PolicyFetchResponse {
diff --git a/components/security_interstitials/core/bad_clock_ui.cc b/components/security_interstitials/core/bad_clock_ui.cc index 27d845f..87933c8b 100644 --- a/components/security_interstitials/core/bad_clock_ui.cc +++ b/components/security_interstitials/core/bad_clock_ui.cc
@@ -121,8 +121,8 @@ case CMD_ERROR: case CMD_TEXT_FOUND: case CMD_TEXT_NOT_FOUND: - // Commands are only for testing. - NOTREACHED() << "Unexpected command: " << command; + // Commands are for testing. + break; } }
diff --git a/components/ukm/ukm_recorder_impl.cc b/components/ukm/ukm_recorder_impl.cc index 6f7a81a2..ceeee432 100644 --- a/components/ukm/ukm_recorder_impl.cc +++ b/components/ukm/ukm_recorder_impl.cc
@@ -18,6 +18,7 @@ #include "third_party/metrics_proto/ukm/entry.pb.h" #include "third_party/metrics_proto/ukm/report.pb.h" #include "third_party/metrics_proto/ukm/source.pb.h" +#include "url/gurl.h" namespace ukm { @@ -112,6 +113,13 @@ } } +GURL SanitizeURL(const GURL& url) { + GURL::Replacements remove_params; + remove_params.ClearUsername(); + remove_params.ClearPassword(); + return url.ReplaceComponents(remove_params); +} + } // namespace UkmRecorderImpl::UkmRecorderImpl() : recording_enabled_(false) {} @@ -193,7 +201,8 @@ kUkmFeature, "RestrictToWhitelistedSourceIds", true); } -void UkmRecorderImpl::UpdateSourceURL(SourceId source_id, const GURL& url) { +void UkmRecorderImpl::UpdateSourceURL(SourceId source_id, + const GURL& unsanitized_url) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (!recording_enabled_) { @@ -207,6 +216,8 @@ return; } + GURL url = SanitizeURL(unsanitized_url); + if (!HasSupportedScheme(url)) { RecordDroppedSource(DroppedDataReason::UNSUPPORTED_URL_SCHEME); return;
diff --git a/components/ukm/ukm_service_unittest.cc b/components/ukm/ukm_service_unittest.cc index e4bb0e7..8705687 100644 --- a/components/ukm/ukm_service_unittest.cc +++ b/components/ukm/ukm_service_unittest.cc
@@ -776,4 +776,25 @@ } } +TEST_F(UkmServiceTest, SanitizeUrlAuthParams) { + UkmService service(&prefs_, &client_); + TestRecordingHelper recorder(&service); + EXPECT_EQ(0, GetPersistedLogCount()); + service.Initialize(); + task_runner_->RunUntilIdle(); + service.EnableRecording(); + service.EnableReporting(); + + auto id = GetWhitelistedSourceId(0); + recorder.UpdateSourceURL(id, GURL("https://username:password@example.com/")); + + service.Flush(); + EXPECT_EQ(1, GetPersistedLogCount()); + + auto proto_report = GetPersistedReport(); + ASSERT_EQ(1, proto_report.sources_size()); + const Source& proto_source = proto_report.sources(0); + EXPECT_EQ("https://example.com/", proto_source.url()); +} + } // namespace ukm
diff --git a/ios/chrome/browser/download/BUILD.gn b/ios/chrome/browser/download/BUILD.gn index b004d8e..364a14c 100644 --- a/ios/chrome/browser/download/BUILD.gn +++ b/ios/chrome/browser/download/BUILD.gn
@@ -8,6 +8,8 @@ "browser_download_service.mm", "browser_download_service_factory.h", "browser_download_service_factory.mm", + "pass_kit_mime_type.cc", + "pass_kit_mime_type.h", "pass_kit_tab_helper.h", "pass_kit_tab_helper.mm", "pass_kit_tab_helper_delegate.h",
diff --git a/ios/chrome/browser/download/browser_download_service.mm b/ios/chrome/browser/download/browser_download_service.mm index 080c650..43b97e0 100644 --- a/ios/chrome/browser/download/browser_download_service.mm +++ b/ios/chrome/browser/download/browser_download_service.mm
@@ -5,6 +5,7 @@ #include "ios/chrome/browser/download/browser_download_service.h" #include "base/feature_list.h" +#include "ios/chrome/browser/download/pass_kit_mime_type.h" #import "ios/chrome/browser/download/pass_kit_tab_helper.h" #import "ios/web/public/download/download_controller.h" #import "ios/web/public/download/download_task.h" @@ -32,7 +33,7 @@ web::DownloadController* download_controller, web::WebState* web_state, std::unique_ptr<web::DownloadTask> task) { - if (task->GetMimeType() == "application/vnd.apple.pkpass") { + if (task->GetMimeType() == kPkPassMimeType) { if (base::FeatureList::IsEnabled(web::features::kNewPassKitDownload)) { PassKitTabHelper* tab_helper = PassKitTabHelper::FromWebState(web_state); if (tab_helper) {
diff --git a/ios/chrome/browser/download/browser_download_service_unittest.mm b/ios/chrome/browser/download/browser_download_service_unittest.mm index ccef526..2c9922d 100644 --- a/ios/chrome/browser/download/browser_download_service_unittest.mm +++ b/ios/chrome/browser/download/browser_download_service_unittest.mm
@@ -9,6 +9,7 @@ #include "base/macros.h" #include "base/test/scoped_feature_list.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" +#include "ios/chrome/browser/download/pass_kit_mime_type.h" #import "ios/chrome/browser/download/pass_kit_tab_helper.h" #import "ios/web/public/download/download_controller.h" #import "ios/web/public/download/download_task.h" @@ -25,7 +26,6 @@ namespace { char kUrl[] = "https://test.test/"; -char kMimeType[] = "application/vnd.apple.pkpass"; // Fake Tab Helper that substitutes real PassKitTabHelper for testing. class StubPassKitTabHelper : public PassKitTabHelper { @@ -101,7 +101,8 @@ // PassKitTabHelper. TEST_F(BrowserDownloadServiceTest, PkPassMimeType) { ASSERT_TRUE(download_controller()->GetDelegate()); - auto task = std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kMimeType); + auto task = + std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kPkPassMimeType); web::DownloadTask* task_ptr = task.get(); download_controller()->GetDelegate()->OnDownloadCreated( download_controller(), &web_state_, std::move(task));
diff --git a/ios/chrome/browser/download/pass_kit_mime_type.cc b/ios/chrome/browser/download/pass_kit_mime_type.cc new file mode 100644 index 0000000..b6e65c0 --- /dev/null +++ b/ios/chrome/browser/download/pass_kit_mime_type.cc
@@ -0,0 +1,7 @@ +// Copyright 2017 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. + +#include "ios/chrome/browser/download/pass_kit_mime_type.h" + +char kPkPassMimeType[] = "application/vnd.apple.pkpass";
diff --git a/ios/chrome/browser/download/pass_kit_mime_type.h b/ios/chrome/browser/download/pass_kit_mime_type.h new file mode 100644 index 0000000..f7e107a --- /dev/null +++ b/ios/chrome/browser/download/pass_kit_mime_type.h
@@ -0,0 +1,10 @@ +// Copyright 2017 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. + +#ifndef IOS_CHROME_BROWSER_DOWNLOAD_PASS_KIT_MIME_TYPE_H_ +#define IOS_CHROME_BROWSER_DOWNLOAD_PASS_KIT_MIME_TYPE_H_ + +extern char kPkPassMimeType[]; + +#endif // IOS_CHROME_BROWSER_DOWNLOAD_PASS_KIT_MIME_TYPE_H_
diff --git a/ios/chrome/browser/download/pass_kit_tab_helper.mm b/ios/chrome/browser/download/pass_kit_tab_helper.mm index 28ff215..88f5b7c 100644 --- a/ios/chrome/browser/download/pass_kit_tab_helper.mm +++ b/ios/chrome/browser/download/pass_kit_tab_helper.mm
@@ -10,6 +10,7 @@ #import <PassKit/PassKit.h> #include "base/memory/ptr_util.h" +#include "ios/chrome/browser/download/pass_kit_mime_type.h" #import "ios/chrome/browser/download/pass_kit_tab_helper_delegate.h" #import "ios/web/public/download/download_task.h" #include "net/url_request/url_fetcher_response_writer.h" @@ -43,7 +44,7 @@ } void PassKitTabHelper::Download(std::unique_ptr<web::DownloadTask> task) { - DCHECK_EQ(task->GetMimeType(), "application/vnd.apple.pkpass"); + DCHECK_EQ(task->GetMimeType(), kPkPassMimeType); web::DownloadTask* task_ptr = task.get(); // Start may call OnDownloadUpdated immediately, so add the task to the set of // unfinished tasks.
diff --git a/ios/chrome/browser/download/pass_kit_tab_helper_unittest.mm b/ios/chrome/browser/download/pass_kit_tab_helper_unittest.mm index 507a218..203141c 100644 --- a/ios/chrome/browser/download/pass_kit_tab_helper_unittest.mm +++ b/ios/chrome/browser/download/pass_kit_tab_helper_unittest.mm
@@ -8,6 +8,7 @@ #import <PassKit/PassKit.h> +#include "ios/chrome/browser/download/pass_kit_mime_type.h" #include "ios/chrome/browser/download/pass_kit_test_util.h" #import "ios/chrome/test/fakes/fake_pass_kit_tab_helper_delegate.h" #import "ios/web/public/test/fakes/fake_download_task.h" @@ -24,7 +25,6 @@ namespace { char kUrl[] = "https://test.test/"; -char kMimeType[] = "application/vnd.apple.pkpass"; // Used as no-op callback. void DoNothing(int) {} @@ -49,7 +49,8 @@ // Tests downloading empty pkpass file. TEST_F(PassKitTabHelperTest, EmptyFile) { - auto task = std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kMimeType); + auto task = + std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kPkPassMimeType); web::FakeDownloadTask* task_ptr = task.get(); tab_helper()->Download(std::move(task)); task_ptr->SetDone(true); @@ -59,11 +60,13 @@ // Tests downloading 2 empty pkpass files. TEST_F(PassKitTabHelperTest, MultipleEmptyFiles) { - auto task = std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kMimeType); + auto task = + std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kPkPassMimeType); web::FakeDownloadTask* task_ptr = task.get(); tab_helper()->Download(std::move(task)); - auto task2 = std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kMimeType); + auto task2 = + std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kPkPassMimeType); web::FakeDownloadTask* task_ptr2 = task2.get(); tab_helper()->Download(std::move(task2)); @@ -78,7 +81,8 @@ // Tests downloading a valid pkpass file. TEST_F(PassKitTabHelperTest, ValidPassKitFile) { - auto task = std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kMimeType); + auto task = + std::make_unique<web::FakeDownloadTask>(GURL(kUrl), kPkPassMimeType); web::FakeDownloadTask* task_ptr = task.get(); tab_helper()->Download(std::move(task));
diff --git a/ios/chrome/browser/feature_engagement/feature_engagement_egtest.mm b/ios/chrome/browser/feature_engagement/feature_engagement_egtest.mm index e8e344a..d85f578 100644 --- a/ios/chrome/browser/feature_engagement/feature_engagement_egtest.mm +++ b/ios/chrome/browser/feature_engagement/feature_engagement_egtest.mm
@@ -30,7 +30,7 @@ const int kMinChromeOpensRequired = 5; // The timeout for the load of the feature engagement tracker. -const NSTimeInterval kWaitForTrackerLoadTimeout = 5.0; +const NSTimeInterval kWaitForTrackerLoadTimeout = 10.0; // Matcher for the Reading List Text Badge. id<GREYMatcher> ReadingListTextBadge() {
diff --git a/ios/chrome/browser/ui/download/BUILD.gn b/ios/chrome/browser/ui/download/BUILD.gn index 67486cc..f58af832 100644 --- a/ios/chrome/browser/ui/download/BUILD.gn +++ b/ios/chrome/browser/ui/download/BUILD.gn
@@ -64,6 +64,7 @@ "//base", "//ios/chrome/app:app_internal", "//ios/chrome/app/strings", + "//ios/chrome/browser/download", "//ios/chrome/browser/download:test_support", "//ios/chrome/browser/ui:ui_internal", "//ios/chrome/browser/ui:ui_util",
diff --git a/ios/chrome/browser/ui/download/pass_kit_egtest.mm b/ios/chrome/browser/ui/download/pass_kit_egtest.mm index d40181c..5d57d864 100644 --- a/ios/chrome/browser/ui/download/pass_kit_egtest.mm +++ b/ios/chrome/browser/ui/download/pass_kit_egtest.mm
@@ -7,6 +7,7 @@ #include "base/memory/ptr_util.h" #import "ios/chrome/app/main_controller.h" +#include "ios/chrome/browser/download/pass_kit_mime_type.h" #include "ios/chrome/browser/download/pass_kit_test_util.h" #import "ios/chrome/browser/ui/browser_view_controller.h" #include "ios/chrome/browser/ui/ui_util.h" @@ -47,10 +48,10 @@ "<a id='bad' href='/bad'>Bad</a>" "<a id='good' href='/good'>Good</a>"); } else if (request.GetURL().path() == "/bad") { - result->AddCustomHeader("Content-Type", "application/vnd.apple.pkpass"); + result->AddCustomHeader("Content-Type", kPkPassMimeType); result->set_content("corrupted"); } else if (request.GetURL().path() == "/good") { - result->AddCustomHeader("Content-Type", "application/vnd.apple.pkpass"); + result->AddCustomHeader("Content-Type", kPkPassMimeType); result->set_content(testing::GetTestPass()); }
diff --git a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter index 50b382d..6eb2ad4c 100644 --- a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter +++ b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
@@ -162,33 +162,55 @@ -ShowPageActionWithoutPageActionTest/ShowPageActionWithoutPageActionTest.Test/0 -ShowPageActionWithoutPageActionTest/ShowPageActionWithoutPageActionTest.Test/1 -SiteDetailsBrowserTest.PlatformAppsNotIsolated --SSLUICaptivePortalListResourceBundleTest.Enabled --SSLUICaptivePortalListResourceBundleTest.Enabled_AuthorityInvalid --SSLUICaptivePortalListResourceBundleTest.Enabled_DynamicUpdate --SSLUICaptivePortalListResourceBundleTest.Enabled_NameMismatchAndWeakKey --SSLUIMITMSoftwareDisabledTest.DisabledWithFinch --SSLUIMITMSoftwareEnabledTest.CertificateCommonNameMatchOnly_NoMITMSoftwareInterstitial --SSLUIMITMSoftwareEnabledTest.CertificateOrganizationMatchOnly_NoMITMSoftwareInterstitial --SSLUIMITMSoftwareEnabledTest.EnabledWithFinch --SSLUIMITMSoftwareEnabledTest.EnterpriseManaged --SSLUIMITMSoftwareEnabledTest.IgnoreDynamicUpdateWithSmallVersionId --SSLUIMITMSoftwareEnabledTest.NonMatchingCertificate_NoMITMSoftwareInterstitial --SSLUIMITMSoftwareEnabledTest.NotEnterpriseManaged --SSLUIMITMSoftwareEnabledTest.OverridableError_NoMITMSoftwareInterstitial --SSLUIMITMSoftwareEnabledTest.TwoCertErrors_NoMITMSoftwareInterstitial --SSLUIMITMSoftwareEnabledTest.WrongCertError_NoMITMSoftwareInterstitial --SSLUITest.TestBadHTTPSDownload --SSLUITest.TestHTTPSOCSPOk --SSLUITest.TestHTTPSOCSPRevoked +-SSLUICaptivePortalListResourceBundleTest.Enabled/0 +-SSLUICaptivePortalListResourceBundleTest.Enabled/1 +-SSLUICaptivePortalListResourceBundleTest.Enabled_AuthorityInvalid/0 +-SSLUICaptivePortalListResourceBundleTest.Enabled_AuthorityInvalid/1 +-SSLUICaptivePortalListResourceBundleTest.Enabled_DynamicUpdate/0 +-SSLUICaptivePortalListResourceBundleTest.Enabled_DynamicUpdate/1 +-SSLUICaptivePortalListResourceBundleTest.Enabled_NameMismatchAndWeakKey/0 +-SSLUICaptivePortalListResourceBundleTest.Enabled_NameMismatchAndWeakKey/1 +-SSLUIMITMSoftwareDisabledTest.DisabledWithFinch/0 +-SSLUIMITMSoftwareDisabledTest.DisabledWithFinch/1 +-SSLUIMITMSoftwareEnabledTest.CertificateCommonNameMatchOnly_NoMITMSoftwareInterstitial/0 +-SSLUIMITMSoftwareEnabledTest.CertificateCommonNameMatchOnly_NoMITMSoftwareInterstitial/1 +-SSLUIMITMSoftwareEnabledTest.CertificateOrganizationMatchOnly_NoMITMSoftwareInterstitial/0 +-SSLUIMITMSoftwareEnabledTest.CertificateOrganizationMatchOnly_NoMITMSoftwareInterstitial/1 +-SSLUIMITMSoftwareEnabledTest.EnabledWithFinch/0 +-SSLUIMITMSoftwareEnabledTest.EnabledWithFinch/1 +-SSLUIMITMSoftwareEnabledTest.EnterpriseManaged/0 +-SSLUIMITMSoftwareEnabledTest.EnterpriseManaged/1 +-SSLUIMITMSoftwareEnabledTest.IgnoreDynamicUpdateWithSmallVersionId/0 +-SSLUIMITMSoftwareEnabledTest.IgnoreDynamicUpdateWithSmallVersionId/1 +-SSLUIMITMSoftwareEnabledTest.NonMatchingCertificate_NoMITMSoftwareInterstitial/0 +-SSLUIMITMSoftwareEnabledTest.NonMatchingCertificate_NoMITMSoftwareInterstitial/1 +-SSLUIMITMSoftwareEnabledTest.NotEnterpriseManaged/0 +-SSLUIMITMSoftwareEnabledTest.NotEnterpriseManaged/1 +-SSLUIMITMSoftwareEnabledTest.OverridableError_NoMITMSoftwareInterstitial/0 +-SSLUIMITMSoftwareEnabledTest.OverridableError_NoMITMSoftwareInterstitial/1 +-SSLUIMITMSoftwareEnabledTest.TwoCertErrors_NoMITMSoftwareInterstitial/0 +-SSLUIMITMSoftwareEnabledTest.TwoCertErrors_NoMITMSoftwareInterstitial/1 +-SSLUIMITMSoftwareEnabledTest.WrongCertError_NoMITMSoftwareInterstitial/0 +-SSLUIMITMSoftwareEnabledTest.WrongCertError_NoMITMSoftwareInterstitial/1 +-SSLUITest.TestBadHTTPSDownload/0 +-SSLUITest.TestBadHTTPSDownload/1 +-SSLUITest.TestHTTPSOCSPOk/0 +-SSLUITest.TestHTTPSOCSPOk/1 +-SSLUITest.TestHTTPSOCSPRevoked/0 +-SSLUITest.TestHTTPSOCSPRevoked/1 -SSLUITestTransientAndCommitted.TestHTTPSOCSPOk/0 -SSLUITestTransientAndCommitted.TestHTTPSOCSPOk/1 -SSLUITestTransientAndCommitted.TestHTTPSOCSPRevoked/1 -SSLUITestTransientAndCommitted.TestHTTPSOCSPRevoked/0 -SubresourceFilterBrowserTest.FailedProvisionalLoadInMainframe --SuperfishSSLUITest.NoSuperfishRecorded --SuperfishSSLUITest.SuperfishInterstitial --SuperfishSSLUITest.SuperfishInterstitialDisabled --SuperfishSSLUITest.SuperfishRecorded +-SuperfishSSLUITest.NoSuperfishRecorded/0 +-SuperfishSSLUITest.NoSuperfishRecorded/1 +-SuperfishSSLUITest.SuperfishInterstitial/0 +-SuperfishSSLUITest.SuperfishInterstitial/1 +-SuperfishSSLUITest.SuperfishInterstitialDisabled/0 +-SuperfishSSLUITest.SuperfishInterstitialDisabled/1 +-SuperfishSSLUITest.SuperfishRecorded/0 +-SuperfishSSLUITest.SuperfishRecorded/1 -SymantecMessageSSLUITest.ManySubresources -SymantecMessageSSLUITest.PostJune2016 -SymantecMessageSSLUITest.PreJune2016 @@ -267,8 +289,10 @@ -CaptivePortalBrowserTest.Status511 -CaptivePortalBrowserTest.TwoBrokenTabs -ClientHintsBrowserTest.ClientHintsLifetimeNotAttachedCookiesBlocked --CommonNameMismatchBrowserTest.CheckWWWSubdomainMismatchInverse --CommonNameMismatchBrowserTest.ShouldShowWWWSubdomainMismatchInterstitial +-CommonNameMismatchBrowserTest.CheckWWWSubdomainMismatchInverse/0 +-CommonNameMismatchBrowserTest.CheckWWWSubdomainMismatchInverse/1 +-CommonNameMismatchBrowserTest.ShouldShowWWWSubdomainMismatchInterstitial/0 +-CommonNameMismatchBrowserTest.ShouldShowWWWSubdomainMismatchInterstitial/1 -ContentSettingsTest.AllowCookiesForASessionUsingExceptions -ContentSettingsTest.BasicCookies -ContentSettingsTest.BasicCookiesHttps @@ -337,8 +361,10 @@ -RestartTest.PostWithPassword -RestartTest.SessionCookies -RestartTest.SessionStorage --SSLUITest.MarkBlobAsNonSecure --SSLUITestHSTS.TestInterstitialOptionsNonOverridable +-SSLUITest.MarkBlobAsNonSecure/0 +-SSLUITest.MarkBlobAsNonSecure/1 +-SSLUITestHSTS.TestInterstitialOptionsNonOverridable/0 +-SSLUITestHSTS.TestInterstitialOptionsNonOverridable/1 # crbug.com/778793 -PreviewsBrowserTest.NoScriptPreviewsEnabled
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 3c258a3..2e2cb51 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2520,7 +2520,6 @@ crbug.com/788337 external/wpt/css/css-multicol/multicol-span-all-child-002.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-span-all-margin-nested-002.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-span-float-001.xht [ Failure ] -crbug.com/788337 external/wpt/css/css-multicol/multicol-width-ems-001.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-width-small-001.xht [ Failure ] crbug.com/788337 external/wpt/css/css-multicol/multicol-zero-height-001.xht [ Failure ]
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ch-001.xht b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ch-001.xht new file mode 100644 index 0000000..c3e3b353 --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ch-001.xht
@@ -0,0 +1,34 @@ +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" +"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> +<head> +<title>multicolumn | column-width</title> +<link rel="author" title="Opera Software ASA" href="http://www.opera.com/"/> +<link rel="help" href="http://www.w3.org/TR/css3-multicol/#the-number-and-width-of-columns"/> +<link rel="match" href="multicol-width-ch-ref.xht"/> +<meta name="flags" content=""/> +<style type="text/css"><![CDATA[ +.multicol { + font: 1em monospace; + width: 69ch; + column-width: 13ch; + column-gap: 1ch; + orphans: 1; + widows: 1; + background: yellow; +} +]]></style> +</head> + +<body> + <div class="multicol"> + one two three four + five six seven eight + nineten eleven twelve + thirtn fourtnfiftn sixtn + seventn eightn ninetn twenty + hundred thousand million billion + trillion + </div> +</body> +</html>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ch-ref.xht b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ch-ref.xht new file mode 100644 index 0000000..133ad3e --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ch-ref.xht
@@ -0,0 +1,49 @@ +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" +"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> +<head> +<title>multicolumn | column-width</title> +<link rel="author" title="Opera Software ASA" href="http://www.opera.com/"/> +<style type="text/css"><![CDATA[ +.multicol-ref { + display: flow-root; + width: 69ch; + font: 1em monospace; + background: yellow; +} +.multicol-ref > span { + float: left; + width: 13ch; + margin-right: 1ch; +} +.multicol-ref > span:last-child { + margin: 0; +} +]]></style> +</head> + +<body> + <div class="multicol-ref"> + <span> + one two three four + five six seven eight + </span> + <span> + nineten eleven twelve + thirtn + </span> + <span> + fourtnfiftn sixtn + seventn eightn ninetn + </span> + <span> + twenty + hundred thousand + </span> + <span> + million billion + trillion + </span> + </div> +</body> +</html>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ems-001.xht b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ems-001.xht deleted file mode 100644 index c227ab4..0000000 --- a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ems-001.xht +++ /dev/null
@@ -1,42 +0,0 @@ -<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" -"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> -<html xmlns="http://www.w3.org/1999/xhtml"> -<head> -<title>multicolumn | column-width</title> -<link rel="author" title="Opera Software ASA" href="http://www.opera.com/"/> -<link rel="help" href="http://www.w3.org/TR/css3-multicol/#the-number-and-width-of-columns"/> -<link rel="match" href="multicol-width-ems-ref.xht"/> -<meta name="flags" content=""/> -<style type="text/css"><![CDATA[ -body { - margin: 0; - width: 40em; -} -div { - font-family: monospace; - font-size: 1em; - line-height: 1em; - color: black; - background: yellow; - orphans: 1; - widows: 1; - - column-width: 8em; - column-gap: 0; -} -]]></style> -</head> - -<body> -<div> - one two three four - five six seven eight - nineten eleven twelve - thirtn fourtnfiftn sixtn - seventn eightn ninetn twenty - hundred thousand million billion - trillion -</div> - -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ems-ref.xht b/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ems-ref.xht deleted file mode 100644 index b3ca468..0000000 --- a/third_party/WebKit/LayoutTests/external/wpt/css/css-multicol/multicol-width-ems-ref.xht +++ /dev/null
@@ -1,59 +0,0 @@ -<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" -"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> -<html xmlns="http://www.w3.org/1999/xhtml"> -<head> -<title>multicolumn | column-width</title> -<link rel="author" title="Opera Software ASA" href="http://www.opera.com/"/> -<style type="text/css"><![CDATA[ -body { - margin: 0; - width: 40em; -} -div { - font-family: monospace; - font-size: 1em; - line-height: 1em; - color: black; - background: yellow; - orphans: 1; - widows: 1; -} -span { - float: left; - width: 8em; -} -div::after { - content: ""; - clear: both; - display: block; -} -]]></style> -</head> - -<body> - -<div> -<span> - one two three four - five six seven eight -</span> -<span> - nineten eleven twelve - thirtn -</span> -<span> - fourtnfiftn sixtn - seventn eightn -</span> -<span> - ninetn twenty - hundred thousand -</span> -<span> - million billion - trillion -</span> -</div> - -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/reporting-observer/reporting-api.html b/third_party/WebKit/LayoutTests/reporting-observer/reporting-api.html index 1db9329..60b9985 100644 --- a/third_party/WebKit/LayoutTests/reporting-observer/reporting-api.html +++ b/third_party/WebKit/LayoutTests/reporting-observer/reporting-api.html
@@ -42,7 +42,7 @@ // Make an instance and have it receive the request. var proxy = new MockReportingServiceProxy(); -var interceptor = new MojoInterfaceInterceptor(blink.mojom.ReportingServiceProxy.name); +var interceptor = new MojoInterfaceInterceptor(blink.mojom.ReportingServiceProxy.name, "process"); interceptor.oninterfacerequest = e => proxy.bind(e.handle); interceptor.start();
diff --git a/third_party/WebKit/Source/core/frame/Deprecation.cpp b/third_party/WebKit/Source/core/frame/Deprecation.cpp index 11f1b0a..97c748c 100644 --- a/third_party/WebKit/Source/core/frame/Deprecation.cpp +++ b/third_party/WebKit/Source/core/frame/Deprecation.cpp
@@ -16,8 +16,9 @@ #include "core/page/Page.h" #include "core/workers/WorkerOrWorkletGlobalScope.h" #include "platform/runtime_enabled_features.h" +#include "public/platform/Platform.h" #include "public/platform/reporting.mojom-blink.h" -#include "services/service_manager/public/cpp/interface_provider.h" +#include "services/service_manager/public/cpp/connector.h" #include "third_party/WebKit/common/feature_policy/feature_policy_feature.h" using blink::WebFeature; @@ -795,7 +796,9 @@ // Send the deprecation report to the Reporting API. mojom::blink::ReportingServiceProxyPtr service; - frame->Client()->GetInterfaceProvider()->GetInterface(&service); + Platform* platform = Platform::Current(); + platform->GetConnector()->BindInterface(platform->GetBrowserServiceName(), + &service); service->QueueDeprecationReport(document->Url(), info.id, WTF::Time::FromDoubleT(removalDate), info.message, body->sourceFile(),
diff --git a/third_party/WebKit/Source/core/frame/Intervention.cpp b/third_party/WebKit/Source/core/frame/Intervention.cpp index fb342ba..089eb6a 100644 --- a/third_party/WebKit/Source/core/frame/Intervention.cpp +++ b/third_party/WebKit/Source/core/frame/Intervention.cpp
@@ -11,8 +11,9 @@ #include "core/frame/Report.h" #include "core/frame/ReportingContext.h" #include "core/inspector/ConsoleMessage.h" +#include "public/platform/Platform.h" #include "public/platform/reporting.mojom-blink.h" -#include "services/service_manager/public/cpp/interface_provider.h" +#include "services/service_manager/public/cpp/connector.h" namespace blink { @@ -44,7 +45,9 @@ // Send the intervention report to the Reporting API. mojom::blink::ReportingServiceProxyPtr service; - frame->Client()->GetInterfaceProvider()->GetInterface(&service); + Platform* platform = Platform::Current(); + platform->GetConnector()->BindInterface(platform->GetBrowserServiceName(), + &service); service->QueueInterventionReport(document->Url(), message, body->sourceFile(), body->lineNumber(), body->columnNumber()); }
diff --git a/tools/cygprofile/compare_orderfiles.py b/tools/cygprofile/compare_orderfiles.py new file mode 100755 index 0000000..3cf2934 --- /dev/null +++ b/tools/cygprofile/compare_orderfiles.py
@@ -0,0 +1,156 @@ +#!/usr/bin/python +# Copyright 2017 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. + +"""Compares two orderfiles, from filenames or a commit. + +This shows some statistics about two orderfiles, possibly extracted from an +updating commit made by the orderfile bot. +""" + +import argparse +import logging +import os +import subprocess +import sys + + +def ParseOrderfile(filename): + """Parses an orderfile into a list of symbols. + + Args: + filename: (str) Path to the orderfile. + + Returns: + [str] List of symbols. + """ + symbols = [] + lines = [] + already_seen = set() + with open(filename, 'r') as f: + lines = [line.strip() for line in f] + + for entry in lines: + # Example: .text.startup.BLA + symbol_name = entry[entry.rindex('.'):] + if symbol_name in already_seen or symbol_name == '*' or entry == '.text': + continue + already_seen.add(symbol_name) + symbols.append(symbol_name) + return symbols + + +def Compare(first_filename, second_filename): + """Outputs a comparison of two orderfiles to stdout. + + Args: + first_filename: (str) First orderfile. + second_filename: (str) Second orderfile. + """ + first_symbols = ParseOrderfile(first_filename) + second_symbols = ParseOrderfile(second_filename) + print 'Symbols count:\n\tfirst:\t%d\n\tsecond:\t%d' % ( + len(first_symbols), len(second_symbols)) + first_symbols = set(first_symbols) + second_symbols = set(second_symbols) + new_symbols = second_symbols - first_symbols + removed_symbols = first_symbols - second_symbols + print 'New symbols = %d' % len(new_symbols) + print 'Removed symbols = %d' % len(removed_symbols) + + +def CheckOrderfileCommit(commit_hash, clank_path): + """Asserts that a commit is an orderfile update from the bot. + + Args: + commit_hash: (str) Git hash of the orderfile roll commit. + clank_path: (str) Path to the clank repository. + """ + output = subprocess.check_output( + ['git', 'show', r'--format=%an %s', commit_hash], cwd=clank_path) + first_line = output.split('\n')[0] + assert first_line == 'clank-autoroller Update Orderfile.', ( + 'Not an orderfile commit') + + +def GetBeforeAfterOrderfileHashes(commit_hash, clank_path): + """Downloads the orderfiles before and afer an orderfile roll. + + Args: + commit_hash: (str) Git hash of the orderfile roll commit. + clank_path: (str) Path to the clank repository. + + Returns: + (str, str) Path to the before and after commit orderfiles. + """ + orderfile_hash_relative_path = 'orderfiles/orderfile.arm.out.sha1' + before_output = subprocess.check_output( + ['git', 'show', '%s^:%s' % (commit_hash, orderfile_hash_relative_path)], + cwd=clank_path) + before_hash = before_output.split('\n')[0] + after_output = subprocess.check_output( + ['git', 'show', '%s:%s' % (commit_hash, orderfile_hash_relative_path)], + cwd=clank_path) + after_hash = after_output.split('\n')[0] + assert before_hash != after_hash + return (before_hash, after_hash) + + +def DownloadOrderfile(orderfile_hash, output_filename): + """Downloads an orderfile with a given hash to a given destination.""" + cloud_storage_path = ( + 'gs://clank-archive/orderfile-clankium/%s' % orderfile_hash) + subprocess.check_call( + ['gsutil.py', 'cp', cloud_storage_path, output_filename]) + + +def GetOrderfilesFromCommit(commit_hash): + """Returns paths to the before and after orderfiles for a commit.""" + clank_path = os.path.join(os.path.dirname(__file__), os.pardir, os.pardir, + 'clank') + logging.info('Checking that the commit is an orderfile') + CheckOrderfileCommit(commit_hash, clank_path) + (before_hash, after_hash) = GetBeforeAfterOrderfileHashes( + commit_hash, clank_path) + logging.info('Before / after hashes: %s %s', before_hash, after_hash) + before_filename = os.path.join('/tmp/', before_hash) + after_filename = os.path.join('/tmp/', after_hash) + logging.info('Downloading files') + DownloadOrderfile(before_hash, before_filename) + DownloadOrderfile(after_hash, after_filename) + return (before_filename, after_filename) + + +def CreateArgumentParser(): + """Returns the argumeng parser.""" + parser = argparse.ArgumentParser() + parser.add_argument('--first', help='First orderfile') + parser.add_argument('--second', help='Second orderfile') + parser.add_argument('--from-commit', help='Analyze the difference in the ' + 'orderfile from an orderfile bot commit.') + return parser + + +def main(): + logging.basicConfig(level=logging.INFO) + parser = CreateArgumentParser() + args = parser.parse_args() + if args.first or args.second: + assert args.first and args.second, 'Need both files.' + Compare(args.first, args.second) + elif args.from_commit: + first, second = GetOrderfilesFromCommit(args.from_commit) + try: + logging.info('Comparing the orderfiles') + Compare(first, second) + finally: + os.remove(first) + os.remove(second) + else: + return False + return True + + +if __name__ == '__main__': + sys.exit(0 if main() else 1)