Commit ba773380 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

Eliminate remaining usage of CAPTIVE_PORTAL_CHECK_RESULT notification

The sending of the CAPTIVE_PORTAL_CHECK_RESULT notification is a
blocker to the ongoing effort to componentize CaptivePortalService. A
previous CL has introduced the ability to register for a callback with
CaptivePortalService as an alternative mechanism of being informed on
the results of captive portal queries. This CL converts the remaining
consumers and eliminates the notification.

Bug: 1030692
Change-Id: I9a2c5f6190922eb6ddb10d572dcbec6b52ffc3ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012383
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734407}
parent bc867047
...@@ -10,13 +10,11 @@ ...@@ -10,13 +10,11 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/time/tick_clock.h" #include "base/time/tick_clock.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/chrome_notification_types.h"
#include "components/captive_portal/core/captive_portal_types.h" #include "components/captive_portal/core/captive_portal_types.h"
#include "components/embedder_support/pref_names.h" #include "components/embedder_support/pref_names.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
...@@ -363,11 +361,6 @@ void CaptivePortalService::OnResult(CaptivePortalResult result, ...@@ -363,11 +361,6 @@ void CaptivePortalService::OnResult(CaptivePortalResult result,
results.landing_url = landing_url; results.landing_url = landing_url;
last_detection_result_ = result; last_detection_result_ = result;
content::NotificationService::current()->Notify(
chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
content::Source<content::BrowserContext>(browser_context_),
content::Details<Results>(&results));
callback_list_.Notify(results); callback_list_.Notify(results);
} }
......
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include "chrome/browser/captive_portal/captive_portal_login_detector.h" #include "chrome/browser/captive_portal/captive_portal_login_detector.h"
#include "chrome/browser/captive_portal/captive_portal_service_factory.h" #include "chrome/browser/captive_portal/captive_portal_service_factory.h"
#include "chrome/browser/captive_portal/captive_portal_tab_reloader.h" #include "chrome/browser/captive_portal/captive_portal_tab_reloader.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/browser_finder.h"
...@@ -17,10 +16,6 @@ ...@@ -17,10 +16,6 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
...@@ -42,10 +37,12 @@ CaptivePortalTabHelper::CaptivePortalTabHelper( ...@@ -42,10 +37,12 @@ CaptivePortalTabHelper::CaptivePortalTabHelper(
web_contents, web_contents,
false))), false))),
login_detector_(new CaptivePortalLoginDetector(profile_)), login_detector_(new CaptivePortalLoginDetector(profile_)),
is_captive_portal_window_(false) { is_captive_portal_window_(false),
subscription_(
CaptivePortalServiceFactory::GetForProfile(profile_)
->RegisterCallback(base::Bind(&CaptivePortalTabHelper::Observe,
base::Unretained(this)))) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
registrar_.Add(this, chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
content::Source<content::BrowserContext>(profile_));
} }
CaptivePortalTabHelper::~CaptivePortalTabHelper() { CaptivePortalTabHelper::~CaptivePortalTabHelper() {
...@@ -130,17 +127,10 @@ void CaptivePortalTabHelper::DidStopLoading() { ...@@ -130,17 +127,10 @@ void CaptivePortalTabHelper::DidStopLoading() {
} }
void CaptivePortalTabHelper::Observe( void CaptivePortalTabHelper::Observe(
int type, const CaptivePortalService::Results& results) {
const content::NotificationSource& source,
const content::NotificationDetails& details) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK_EQ(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, type);
DCHECK_EQ(profile_, content::Source<content::BrowserContext>(source).ptr());
const CaptivePortalService::Results* results = OnCaptivePortalResults(results.previous_result, results.result);
content::Details<CaptivePortalService::Results>(details).ptr();
OnCaptivePortalResults(results->previous_result, results->result);
} }
void CaptivePortalTabHelper::OnSSLCertError(const net::SSLInfo& ssl_info) { void CaptivePortalTabHelper::OnSSLCertError(const net::SSLInfo& ssl_info) {
......
...@@ -10,8 +10,6 @@ ...@@ -10,8 +10,6 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/captive_portal/captive_portal_service.h" #include "chrome/browser/captive_portal/captive_portal_service.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h" #include "content/public/browser/web_contents_user_data.h"
#include "content/public/common/resource_type.h" #include "content/public/common/resource_type.h"
...@@ -54,7 +52,6 @@ class CaptivePortalTabReloader; ...@@ -54,7 +52,6 @@ class CaptivePortalTabReloader;
// https://docs.google.com/document/d/1k-gP2sswzYNvryu9NcgN7q5XrsMlUdlUdoW9WRaEmfM/edit // https://docs.google.com/document/d/1k-gP2sswzYNvryu9NcgN7q5XrsMlUdlUdoW9WRaEmfM/edit
class CaptivePortalTabHelper class CaptivePortalTabHelper
: public content::WebContentsObserver, : public content::WebContentsObserver,
public content::NotificationObserver,
public content::WebContentsUserData<CaptivePortalTabHelper> { public content::WebContentsUserData<CaptivePortalTabHelper> {
public: public:
~CaptivePortalTabHelper() override; ~CaptivePortalTabHelper() override;
...@@ -68,11 +65,6 @@ class CaptivePortalTabHelper ...@@ -68,11 +65,6 @@ class CaptivePortalTabHelper
content::NavigationHandle* navigation_handle) override; content::NavigationHandle* navigation_handle) override;
void DidStopLoading() override; void DidStopLoading() override;
// content::NotificationObserver:
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
// Called when a certificate interstitial error page is about to be shown. // Called when a certificate interstitial error page is about to be shown.
void OnSSLCertError(const net::SSLInfo& ssl_info); void OnSSLCertError(const net::SSLInfo& ssl_info);
...@@ -94,6 +86,8 @@ class CaptivePortalTabHelper ...@@ -94,6 +86,8 @@ class CaptivePortalTabHelper
friend class content::WebContentsUserData<CaptivePortalTabHelper>; friend class content::WebContentsUserData<CaptivePortalTabHelper>;
explicit CaptivePortalTabHelper(content::WebContents* web_contents); explicit CaptivePortalTabHelper(content::WebContents* web_contents);
void Observe(const CaptivePortalService::Results& results);
// Called by Observe in response to the corresponding event. // Called by Observe in response to the corresponding event.
void OnCaptivePortalResults( void OnCaptivePortalResults(
captive_portal::CaptivePortalResult previous_result, captive_portal::CaptivePortalResult previous_result,
...@@ -123,7 +117,7 @@ class CaptivePortalTabHelper ...@@ -123,7 +117,7 @@ class CaptivePortalTabHelper
// portal resolution. // portal resolution.
bool is_captive_portal_window_; bool is_captive_portal_window_;
content::NotificationRegistrar registrar_; std::unique_ptr<CaptivePortalService::Subscription> subscription_;
WEB_CONTENTS_USER_DATA_KEY_DECL(); WEB_CONTENTS_USER_DATA_KEY_DECL();
......
...@@ -12,10 +12,6 @@ ...@@ -12,10 +12,6 @@
#include "chrome/browser/captive_portal/captive_portal_tab_reloader.h" #include "chrome/browser/captive_portal/captive_portal_tab_reloader.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chrome_notification_types.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
...@@ -152,12 +148,10 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness { ...@@ -152,12 +148,10 @@ class CaptivePortalTabHelperTest : public ChromeRenderViewHostTestHarness {
CaptivePortalService::Results results; CaptivePortalService::Results results;
results.previous_result = previous_result; results.previous_result = previous_result;
results.result = result; results.result = result;
content::Details<CaptivePortalService::Results> details_results(&results);
EXPECT_CALL(mock_reloader(), OnCaptivePortalResults(previous_result, EXPECT_CALL(mock_reloader(), OnCaptivePortalResults(previous_result,
result)).Times(1); result)).Times(1);
tab_helper()->Observe(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, tab_helper()->Observe(results);
source_profile, details_results);
} }
MockCaptivePortalTabReloader& mock_reloader() { return *mock_reloader_; } MockCaptivePortalTabReloader& mock_reloader() { return *mock_reloader_; }
......
...@@ -197,15 +197,6 @@ enum NotificationType { ...@@ -197,15 +197,6 @@ enum NotificationType {
NOTIFICATION_TAB_DRAG_LOOP_DONE, NOTIFICATION_TAB_DRAG_LOOP_DONE,
#endif #endif
// DEPRECATED: Instead of listening this notification, use
// CaptivePortalService::RegisterCallback().
// TODO(blundell): Delete this notification as part of
// https://crbug.com/1030692.
// Sent when the CaptivePortalService checks if we're behind a captive portal.
// The Source is the Profile the CaptivePortalService belongs to, and the
// Details are a Details<CaptivePortalService::CheckResults>.
NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
// Sent when the applications in the NTP app launcher have been reordered. // Sent when the applications in the NTP app launcher have been reordered.
// The details, if not NoDetails, is the std::string ID of the extension that // The details, if not NoDetails, is the std::string ID of the extension that
// was moved. // was moved.
......
...@@ -8,11 +8,8 @@ ...@@ -8,11 +8,8 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/captive_portal/captive_portal_service.h"
#include "chrome/browser/captive_portal/captive_portal_service_factory.h" #include "chrome/browser/captive_portal/captive_portal_service_factory.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
namespace { namespace {
...@@ -50,8 +47,10 @@ CaptivePortalMetricsRecorder::CaptivePortalMetricsRecorder( ...@@ -50,8 +47,10 @@ CaptivePortalMetricsRecorder::CaptivePortalMetricsRecorder(
Profile::FromBrowserContext(web_contents->GetBrowserContext()); Profile::FromBrowserContext(web_contents->GetBrowserContext());
captive_portal_detection_enabled_ = captive_portal_detection_enabled_ =
CaptivePortalServiceFactory::GetForProfile(profile)->enabled(); CaptivePortalServiceFactory::GetForProfile(profile)->enabled();
registrar_.Add(this, chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, subscription_ =
content::Source<content::BrowserContext>(profile)); CaptivePortalServiceFactory::GetForProfile(profile)->RegisterCallback(
base::Bind(&CaptivePortalMetricsRecorder::Observe,
base::Unretained(this)));
} }
CaptivePortalMetricsRecorder::~CaptivePortalMetricsRecorder() = default; CaptivePortalMetricsRecorder::~CaptivePortalMetricsRecorder() = default;
...@@ -78,19 +77,13 @@ void CaptivePortalMetricsRecorder::RecordCaptivePortalUMAStatistics() const { ...@@ -78,19 +77,13 @@ void CaptivePortalMetricsRecorder::RecordCaptivePortalUMAStatistics() const {
} }
void CaptivePortalMetricsRecorder::Observe( void CaptivePortalMetricsRecorder::Observe(
int type, const CaptivePortalService::Results& results) {
const content::NotificationSource& source,
const content::NotificationDetails& details) {
DCHECK_EQ(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, type);
// When detection is disabled, captive portal service always sends // When detection is disabled, captive portal service always sends
// RESULT_INTERNET_CONNECTED. Ignore any probe results in that case. // RESULT_INTERNET_CONNECTED. Ignore any probe results in that case.
if (!captive_portal_detection_enabled_) if (!captive_portal_detection_enabled_)
return; return;
captive_portal_probe_completed_ = true; captive_portal_probe_completed_ = true;
CaptivePortalService::Results* results =
content::Details<CaptivePortalService::Results>(details).ptr();
// If a captive portal was detected at any point when the interstitial was // If a captive portal was detected at any point when the interstitial was
// displayed, assume that the interstitial was caused by a captive portal. // displayed, assume that the interstitial was caused by a captive portal.
// Example scenario: // Example scenario:
...@@ -102,9 +95,9 @@ void CaptivePortalMetricsRecorder::Observe( ...@@ -102,9 +95,9 @@ void CaptivePortalMetricsRecorder::Observe(
// potentially caused by the captive portal. // potentially caused by the captive portal.
captive_portal_detected_ = captive_portal_detected_ =
captive_portal_detected_ || captive_portal_detected_ ||
(results->result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL); (results.result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL);
// Also keep track of non-HTTP portals and error cases. // Also keep track of non-HTTP portals and error cases.
captive_portal_no_response_ = captive_portal_no_response_ =
captive_portal_no_response_ || captive_portal_no_response_ ||
(results->result == captive_portal::RESULT_NO_RESPONSE); (results.result == captive_portal::RESULT_NO_RESPONSE);
} }
...@@ -9,8 +9,7 @@ ...@@ -9,8 +9,7 @@
#include <vector> #include <vector>
#include "base/time/time.h" #include "base/time/time.h"
#include "content/public/browser/notification_observer.h" #include "chrome/browser/captive_portal/captive_portal_service.h"
#include "content/public/browser/notification_registrar.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -22,11 +21,11 @@ class WebContents; ...@@ -22,11 +21,11 @@ class WebContents;
// metrics. It should only be used on the UI thread because its implementation // metrics. It should only be used on the UI thread because its implementation
// uses captive_portal::CaptivePortalService which can only be accessed on the // uses captive_portal::CaptivePortalService which can only be accessed on the
// UI thread. // UI thread.
class CaptivePortalMetricsRecorder : public content::NotificationObserver { class CaptivePortalMetricsRecorder {
public: public:
CaptivePortalMetricsRecorder(content::WebContents* web_contents, CaptivePortalMetricsRecorder(content::WebContents* web_contents,
bool overridable); bool overridable);
~CaptivePortalMetricsRecorder() override; ~CaptivePortalMetricsRecorder();
// Should be called when the interstitial is closing. // Should be called when the interstitial is closing.
void RecordCaptivePortalUMAStatistics() const; void RecordCaptivePortalUMAStatistics() const;
...@@ -34,10 +33,7 @@ class CaptivePortalMetricsRecorder : public content::NotificationObserver { ...@@ -34,10 +33,7 @@ class CaptivePortalMetricsRecorder : public content::NotificationObserver {
private: private:
typedef std::vector<std::string> Tokens; typedef std::vector<std::string> Tokens;
// content::NotificationObserver: void Observe(const CaptivePortalService::Results& results);
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
bool overridable_; bool overridable_;
bool captive_portal_detection_enabled_; bool captive_portal_detection_enabled_;
...@@ -47,7 +43,7 @@ class CaptivePortalMetricsRecorder : public content::NotificationObserver { ...@@ -47,7 +43,7 @@ class CaptivePortalMetricsRecorder : public content::NotificationObserver {
bool captive_portal_no_response_; bool captive_portal_no_response_;
bool captive_portal_detected_; bool captive_portal_detected_;
content::NotificationRegistrar registrar_; std::unique_ptr<CaptivePortalService::Subscription> subscription_;
}; };
#endif // CHROME_BROWSER_SSL_CAPTIVE_PORTAL_METRICS_RECORDER_H_ #endif // CHROME_BROWSER_SSL_CAPTIVE_PORTAL_METRICS_RECORDER_H_
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
#include "base/time/clock.h" #include "base/time/clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/captive_portal/captive_portal_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ssl/captive_portal_helper.h" #include "chrome/browser/ssl/captive_portal_helper.h"
#include "chrome/browser/ssl/chrome_security_blocking_page_factory.h" #include "chrome/browser/ssl/chrome_security_blocking_page_factory.h"
...@@ -53,8 +53,6 @@ ...@@ -53,8 +53,6 @@
#include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h" #include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h"
#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION) #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
#include "chrome/browser/captive_portal/captive_portal_service.h"
#include "chrome/browser/captive_portal/captive_portal_service_factory.h"
#include "chrome/browser/captive_portal/captive_portal_tab_helper.h" #include "chrome/browser/captive_portal/captive_portal_tab_helper.h"
#endif #endif
...@@ -573,8 +571,7 @@ void SSLErrorHandler::HandleSSLError( ...@@ -573,8 +571,7 @@ void SSLErrorHandler::HandleSSLError(
request_url, std::move(ssl_cert_reporter), request_url, std::move(ssl_cert_reporter),
g_config.Pointer()->on_blocking_page_shown_callback(), g_config.Pointer()->on_blocking_page_shown_callback(),
std::move(blocking_page_ready_callback))), std::move(blocking_page_ready_callback))),
web_contents, profile, cert_error, ssl_info, network_time_tracker, web_contents, cert_error, ssl_info, network_time_tracker, request_url);
request_url);
web_contents->SetUserData(UserDataKey(), base::WrapUnique(error_handler)); web_contents->SetUserData(UserDataKey(), base::WrapUnique(error_handler));
error_handler->StartHandlingError(); error_handler->StartHandlingError();
} }
...@@ -644,7 +641,6 @@ void SSLErrorHandler::SetClientCallbackOnInterstitialsShown( ...@@ -644,7 +641,6 @@ void SSLErrorHandler::SetClientCallbackOnInterstitialsShown(
SSLErrorHandler::SSLErrorHandler( SSLErrorHandler::SSLErrorHandler(
std::unique_ptr<Delegate> delegate, std::unique_ptr<Delegate> delegate,
content::WebContents* web_contents, content::WebContents* web_contents,
Profile* profile,
int cert_error, int cert_error,
const net::SSLInfo& ssl_info, const net::SSLInfo& ssl_info,
network_time::NetworkTimeTracker* network_time_tracker, network_time::NetworkTimeTracker* network_time_tracker,
...@@ -652,7 +648,6 @@ SSLErrorHandler::SSLErrorHandler( ...@@ -652,7 +648,6 @@ SSLErrorHandler::SSLErrorHandler(
: content::WebContentsObserver(web_contents), : content::WebContentsObserver(web_contents),
delegate_(std::move(delegate)), delegate_(std::move(delegate)),
web_contents_(web_contents), web_contents_(web_contents),
profile_(profile),
cert_error_(cert_error), cert_error_(cert_error),
ssl_info_(ssl_info), ssl_info_(ssl_info),
request_url_(request_url), request_url_(request_url),
...@@ -764,13 +759,13 @@ void SSLErrorHandler::StartHandlingError() { ...@@ -764,13 +759,13 @@ void SSLErrorHandler::StartHandlingError() {
} }
} }
// Always listen to captive portal notifications, otherwise build fails
// because profile_ isn't used. This is a no-op on platforms where
// captive portal detection is disabled.
registrar_.Add(this, chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
content::Source<content::BrowserContext>(profile_));
#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION) #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
Profile* profile =
Profile::FromBrowserContext(web_contents_->GetBrowserContext());
subscription_ =
CaptivePortalServiceFactory::GetForProfile(profile)->RegisterCallback(
base::Bind(&SSLErrorHandler::Observe, base::Unretained(this)));
CaptivePortalTabHelper* captive_portal_tab_helper = CaptivePortalTabHelper* captive_portal_tab_helper =
CaptivePortalTabHelper::FromWebContents(web_contents_); CaptivePortalTabHelper::FromWebContents(web_contents_);
if (captive_portal_tab_helper) { if (captive_portal_tab_helper) {
...@@ -880,18 +875,11 @@ void SSLErrorHandler::CommonNameMismatchHandlerCallback( ...@@ -880,18 +875,11 @@ void SSLErrorHandler::CommonNameMismatchHandlerCallback(
} }
} }
void SSLErrorHandler::Observe( void SSLErrorHandler::Observe(const CaptivePortalService::Results& results) {
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION) #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
DCHECK_EQ(chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT, type);
timer_.Stop(); timer_.Stop();
CaptivePortalService::Results* results = if (results.result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL)
content::Details<CaptivePortalService::Results>(details).ptr(); ShowCaptivePortalInterstitial(results.landing_url);
if (results->result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL)
ShowCaptivePortalInterstitial(results->landing_url);
else else
ShowSSLInterstitial(); ShowSSLInterstitial();
#else #else
......
...@@ -12,14 +12,13 @@ ...@@ -12,14 +12,13 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "chrome/browser/captive_portal/captive_portal_service.h"
#include "components/security_interstitials/content/common_name_mismatch_handler.h" #include "components/security_interstitials/content/common_name_mismatch_handler.h"
#include "components/security_interstitials/content/security_interstitial_page.h" #include "components/security_interstitials/content/security_interstitial_page.h"
#include "components/security_interstitials/content/ssl_cert_reporter.h" #include "components/security_interstitials/content/ssl_cert_reporter.h"
#include "components/security_interstitials/content/ssl_error_assistant.pb.h" #include "components/security_interstitials/content/ssl_error_assistant.pb.h"
#include "components/ssl_errors/error_classification.h" #include "components/ssl_errors/error_classification.h"
#include "content/public/browser/certificate_request_result_type.h" #include "content/public/browser/certificate_request_result_type.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/restore_type.h" #include "content/public/browser/restore_type.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h" #include "content/public/browser/web_contents_user_data.h"
...@@ -27,7 +26,6 @@ ...@@ -27,7 +26,6 @@
#include "url/gurl.h" #include "url/gurl.h"
class CommonNameMismatchHandler; class CommonNameMismatchHandler;
class Profile;
struct DynamicInterstitialInfo; struct DynamicInterstitialInfo;
namespace base { namespace base {
...@@ -67,8 +65,7 @@ extern const base::Feature kCaptivePortalCertificateList; ...@@ -67,8 +65,7 @@ extern const base::Feature kCaptivePortalCertificateList;
// uses captive_portal::CaptivePortalService which can only be accessed on the // uses captive_portal::CaptivePortalService which can only be accessed on the
// UI thread. // UI thread.
class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>, class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
public content::WebContentsObserver, public content::WebContentsObserver {
public content::NotificationObserver {
public: public:
typedef base::Callback<void(content::WebContents*)> TimerStartedCallback; typedef base::Callback<void(content::WebContents*)> TimerStartedCallback;
typedef base::OnceCallback<void( typedef base::OnceCallback<void(
...@@ -184,7 +181,6 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>, ...@@ -184,7 +181,6 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
protected: protected:
SSLErrorHandler(std::unique_ptr<Delegate> delegate, SSLErrorHandler(std::unique_ptr<Delegate> delegate,
content::WebContents* web_contents, content::WebContents* web_contents,
Profile* profile,
int cert_error, int cert_error,
const net::SSLInfo& ssl_info, const net::SSLInfo& ssl_info,
network_time::NetworkTimeTracker* network_time_tracker, network_time::NetworkTimeTracker* network_time_tracker,
...@@ -198,6 +194,10 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>, ...@@ -198,6 +194,10 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
private: private:
friend class content::WebContentsUserData<SSLErrorHandler>; friend class content::WebContentsUserData<SSLErrorHandler>;
FRIEND_TEST_ALL_PREFIXES(SSLErrorHandlerTest, CalculateOptionsMask); FRIEND_TEST_ALL_PREFIXES(SSLErrorHandlerTest, CalculateOptionsMask);
FRIEND_TEST_ALL_PREFIXES(SSLErrorHandlerNameMismatchTest,
ShouldShowCustomInterstitialOnCaptivePortalResult);
FRIEND_TEST_ALL_PREFIXES(SSLErrorHandlerNameMismatchTest,
ShouldShowSSLInterstitialOnNoCaptivePortalResult);
void ShowCaptivePortalInterstitial(const GURL& landing_url); void ShowCaptivePortalInterstitial(const GURL& landing_url);
void ShowMITMSoftwareInterstitial(const std::string& mitm_software_name); void ShowMITMSoftwareInterstitial(const std::string& mitm_software_name);
...@@ -213,11 +213,8 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>, ...@@ -213,11 +213,8 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
CommonNameMismatchHandler::SuggestedUrlCheckResult result, CommonNameMismatchHandler::SuggestedUrlCheckResult result,
const GURL& suggested_url); const GURL& suggested_url);
// content::NotificationObserver: // Callback invoked with the result of a query for captive portal status.
void Observe( void Observe(const CaptivePortalService::Results& results);
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
// content::WebContentsObserver: // content::WebContentsObserver:
void DidStartNavigation( void DidStartNavigation(
...@@ -237,13 +234,13 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>, ...@@ -237,13 +234,13 @@ class SSLErrorHandler : public content::WebContentsUserData<SSLErrorHandler>,
std::unique_ptr<Delegate> delegate_; std::unique_ptr<Delegate> delegate_;
content::WebContents* const web_contents_; content::WebContents* const web_contents_;
Profile* const profile_;
const int cert_error_; const int cert_error_;
const net::SSLInfo ssl_info_; const net::SSLInfo ssl_info_;
const GURL request_url_; const GURL request_url_;
network_time::NetworkTimeTracker* network_time_tracker_; network_time::NetworkTimeTracker* network_time_tracker_;
content::NotificationRegistrar registrar_; std::unique_ptr<CaptivePortalService::Subscription> subscription_;
base::OneShotTimer timer_; base::OneShotTimer timer_;
std::unique_ptr<CommonNameMismatchHandler> common_name_mismatch_handler_; std::unique_ptr<CommonNameMismatchHandler> common_name_mismatch_handler_;
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/captive_portal/captive_portal_service.h" #include "chrome/browser/captive_portal/captive_portal_service.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ssl/ssl_error_assistant.h" #include "chrome/browser/ssl/ssl_error_assistant.h"
#include "chrome/browser/ssl/ssl_error_handler.h" #include "chrome/browser/ssl/ssl_error_handler.h"
...@@ -36,7 +35,6 @@ ...@@ -36,7 +35,6 @@
#include "components/security_interstitials/core/ssl_error_ui.h" #include "components/security_interstitials/core/ssl_error_ui.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_service.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/cert/cert_status_flags.h" #include "net/cert/cert_status_flags.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
...@@ -135,14 +133,12 @@ class TestSSLErrorHandler : public SSLErrorHandler { ...@@ -135,14 +133,12 @@ class TestSSLErrorHandler : public SSLErrorHandler {
public: public:
TestSSLErrorHandler(std::unique_ptr<Delegate> delegate, TestSSLErrorHandler(std::unique_ptr<Delegate> delegate,
content::WebContents* web_contents, content::WebContents* web_contents,
Profile* profile,
int cert_error, int cert_error,
const net::SSLInfo& ssl_info, const net::SSLInfo& ssl_info,
network_time::NetworkTimeTracker* network_time_tracker, network_time::NetworkTimeTracker* network_time_tracker,
const GURL& request_url) const GURL& request_url)
: SSLErrorHandler(std::move(delegate), : SSLErrorHandler(std::move(delegate),
web_contents, web_contents,
profile,
cert_error, cert_error,
ssl_info, ssl_info,
network_time_tracker, network_time_tracker,
...@@ -153,11 +149,9 @@ class TestSSLErrorHandler : public SSLErrorHandler { ...@@ -153,11 +149,9 @@ class TestSSLErrorHandler : public SSLErrorHandler {
class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate { class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate {
public: public:
TestSSLErrorHandlerDelegate(Profile* profile, TestSSLErrorHandlerDelegate(content::WebContents* web_contents,
content::WebContents* web_contents,
const net::SSLInfo& ssl_info) const net::SSLInfo& ssl_info)
: profile_(profile), : captive_portal_checked_(false),
captive_portal_checked_(false),
os_reports_captive_portal_(false), os_reports_captive_portal_(false),
suggested_url_exists_(false), suggested_url_exists_(false),
suggested_url_checked_(false), suggested_url_checked_(false),
...@@ -170,17 +164,6 @@ class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate { ...@@ -170,17 +164,6 @@ class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate {
is_overridable_error_(true), is_overridable_error_(true),
has_blocked_interception_(false) {} has_blocked_interception_(false) {}
void SendCaptivePortalNotification(
captive_portal::CaptivePortalResult result) {
CaptivePortalService::Results results;
results.previous_result = captive_portal::RESULT_INTERNET_CONNECTED;
results.result = result;
content::NotificationService::current()->Notify(
chrome::NOTIFICATION_CAPTIVE_PORTAL_CHECK_RESULT,
content::Source<content::BrowserContext>(profile_),
content::Details<CaptivePortalService::Results>(&results));
}
void SendSuggestedUrlCheckResult( void SendSuggestedUrlCheckResult(
const CommonNameMismatchHandler::SuggestedUrlCheckResult& result, const CommonNameMismatchHandler::SuggestedUrlCheckResult& result,
const GURL& suggested_url) { const GURL& suggested_url) {
...@@ -283,7 +266,6 @@ class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate { ...@@ -283,7 +266,6 @@ class TestSSLErrorHandlerDelegate : public SSLErrorHandler::Delegate {
return has_blocked_interception_; return has_blocked_interception_;
} }
Profile* profile_;
bool captive_portal_checked_; bool captive_portal_checked_;
bool os_reports_captive_portal_; bool os_reports_captive_portal_;
bool suggested_url_exists_; bool suggested_url_exists_;
...@@ -319,12 +301,11 @@ class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness { ...@@ -319,12 +301,11 @@ class SSLErrorHandlerNameMismatchTest : public ChromeRenderViewHostTestHarness {
ssl_info_.public_key_hashes.push_back( ssl_info_.public_key_hashes.push_back(
net::HashValue(kCertPublicKeyHashValue)); net::HashValue(kCertPublicKeyHashValue));
delegate_ = delegate_ = new TestSSLErrorHandlerDelegate(web_contents(), ssl_info_);
new TestSSLErrorHandlerDelegate(profile(), web_contents(), ssl_info_);
error_handler_.reset(new TestSSLErrorHandler( error_handler_.reset(new TestSSLErrorHandler(
std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(), std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(),
profile(), net::MapCertStatusToNetError(ssl_info_.cert_status), net::MapCertStatusToNetError(ssl_info_.cert_status), ssl_info_,
ssl_info_, /*network_time_tracker=*/nullptr, GURL() /*request_url*/)); /*network_time_tracker=*/nullptr, GURL() /*request_url*/));
} }
void TearDown() override { void TearDown() override {
...@@ -581,12 +562,11 @@ class SSLErrorAssistantProtoTest : public ChromeRenderViewHostTestHarness { ...@@ -581,12 +562,11 @@ class SSLErrorAssistantProtoTest : public ChromeRenderViewHostTestHarness {
ssl_info_.public_key_hashes.push_back( ssl_info_.public_key_hashes.push_back(
net::HashValue(kCertPublicKeyHashValue)); net::HashValue(kCertPublicKeyHashValue));
delegate_ = delegate_ = new TestSSLErrorHandlerDelegate(web_contents(), ssl_info_);
new TestSSLErrorHandlerDelegate(profile(), web_contents(), ssl_info_);
error_handler_.reset(new TestSSLErrorHandler( error_handler_.reset(new TestSSLErrorHandler(
std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(), std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(),
profile(), net::MapCertStatusToNetError(ssl_info_.cert_status), net::MapCertStatusToNetError(ssl_info_.cert_status), ssl_info_,
ssl_info_, /*network_time_tracker=*/nullptr, GURL() /*request_url*/)); /*network_time_tracker=*/nullptr, GURL() /*request_url*/));
} }
net::SSLInfo ssl_info_; net::SSLInfo ssl_info_;
...@@ -642,12 +622,11 @@ class SSLErrorHandlerDateInvalidTest : public ChromeRenderViewHostTestHarness { ...@@ -642,12 +622,11 @@ class SSLErrorHandlerDateInvalidTest : public ChromeRenderViewHostTestHarness {
net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem"); net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
ssl_info_.cert_status = net::CERT_STATUS_DATE_INVALID; ssl_info_.cert_status = net::CERT_STATUS_DATE_INVALID;
delegate_ = delegate_ = new TestSSLErrorHandlerDelegate(web_contents(), ssl_info_);
new TestSSLErrorHandlerDelegate(profile(), web_contents(), ssl_info_);
error_handler_.reset(new TestSSLErrorHandler( error_handler_.reset(new TestSSLErrorHandler(
std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(), std::unique_ptr<SSLErrorHandler::Delegate>(delegate_), web_contents(),
profile(), net::MapCertStatusToNetError(ssl_info_.cert_status), net::MapCertStatusToNetError(ssl_info_.cert_status), ssl_info_,
ssl_info_, tracker_.get(), GURL() /*request_url*/)); tracker_.get(), GURL() /*request_url*/));
// Fix flakiness in case system time is off and triggers a bad clock // Fix flakiness in case system time is off and triggers a bad clock
// interstitial. https://crbug.com/666821#c50 // interstitial. https://crbug.com/666821#c50
...@@ -753,8 +732,12 @@ TEST_F(SSLErrorHandlerNameMismatchTest, ...@@ -753,8 +732,12 @@ TEST_F(SSLErrorHandlerNameMismatchTest,
EXPECT_FALSE(delegate()->captive_portal_interstitial_shown()); EXPECT_FALSE(delegate()->captive_portal_interstitial_shown());
// Fake a captive portal result. // Fake a captive portal result.
delegate()->ClearSeenOperations(); delegate()->ClearSeenOperations();
delegate()->SendCaptivePortalNotification(
captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL); CaptivePortalService::Results results;
results.previous_result = captive_portal::RESULT_INTERNET_CONNECTED;
results.result = captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL;
error_handler()->Observe(results);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE(error_handler()->IsTimerRunningForTesting()); EXPECT_FALSE(error_handler()->IsTimerRunningForTesting());
...@@ -784,8 +767,12 @@ TEST_F(SSLErrorHandlerNameMismatchTest, ...@@ -784,8 +767,12 @@ TEST_F(SSLErrorHandlerNameMismatchTest,
// This should immediately trigger an SSL interstitial without waiting for // This should immediately trigger an SSL interstitial without waiting for
// the timer to expire. // the timer to expire.
delegate()->ClearSeenOperations(); delegate()->ClearSeenOperations();
delegate()->SendCaptivePortalNotification(
captive_portal::RESULT_INTERNET_CONNECTED); CaptivePortalService::Results results;
results.previous_result = captive_portal::RESULT_INTERNET_CONNECTED;
results.result = captive_portal::RESULT_INTERNET_CONNECTED;
error_handler()->Observe(results);
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_FALSE(error_handler()->IsTimerRunningForTesting()); EXPECT_FALSE(error_handler()->IsTimerRunningForTesting());
...@@ -1507,11 +1494,11 @@ TEST_F(SSLErrorHandlerTest, BlockedInterceptionInterstitial) { ...@@ -1507,11 +1494,11 @@ TEST_F(SSLErrorHandlerTest, BlockedInterceptionInterstitial) {
ssl_info.public_key_hashes.push_back(net::HashValue(kCertPublicKeyHashValue)); ssl_info.public_key_hashes.push_back(net::HashValue(kCertPublicKeyHashValue));
std::unique_ptr<TestSSLErrorHandlerDelegate> delegate( std::unique_ptr<TestSSLErrorHandlerDelegate> delegate(
new TestSSLErrorHandlerDelegate(profile(), web_contents(), ssl_info)); new TestSSLErrorHandlerDelegate(web_contents(), ssl_info));
TestSSLErrorHandlerDelegate* delegate_ptr = delegate.get(); TestSSLErrorHandlerDelegate* delegate_ptr = delegate.get();
TestSSLErrorHandler error_handler( TestSSLErrorHandler error_handler(
std::move(delegate), web_contents(), profile(), std::move(delegate), web_contents(),
net::MapCertStatusToNetError(ssl_info.cert_status), ssl_info, net::MapCertStatusToNetError(ssl_info.cert_status), ssl_info,
/*network_time_tracker=*/nullptr, GURL() /*request_url*/); /*network_time_tracker=*/nullptr, GURL() /*request_url*/);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment