Skip to content

Commit

Permalink
[Captive Portal] Put componentized files in captive_portal namespace
Browse files Browse the repository at this point in the history
This CL finishes off the componentization of
//chrome/browser/captive_portal by putting the componentized files in
the captive_portal namespace. The CL additionally removes all
now-unnecessary references to the captive_portal namespace from within
these files.

Bug: 1030692
Change-Id: Ia9bb3498413aa95cdd6eeb2a652061e6dbca6b11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2038778
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738672}
  • Loading branch information
colinblundell authored and Commit Bot committed Feb 5, 2020
1 parent cca3af7 commit 27331f9
Show file tree
Hide file tree
Showing 34 changed files with 424 additions and 411 deletions.
254 changes: 134 additions & 120 deletions chrome/browser/captive_portal/captive_portal_browsertest.cc

Large diffs are not rendered by default.

15 changes: 7 additions & 8 deletions chrome/browser/captive_portal/captive_portal_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
#include "components/keyed_service/content/browser_context_dependency_manager.h"

// static
CaptivePortalService* CaptivePortalServiceFactory::GetForProfile(
Profile* profile) {
return static_cast<CaptivePortalService*>(
captive_portal::CaptivePortalService*
CaptivePortalServiceFactory::GetForProfile(Profile* profile) {
return static_cast<captive_portal::CaptivePortalService*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
}

Expand All @@ -23,17 +23,16 @@ CaptivePortalServiceFactory* CaptivePortalServiceFactory::GetInstance() {

CaptivePortalServiceFactory::CaptivePortalServiceFactory()
: BrowserContextKeyedServiceFactory(
"CaptivePortalService",
BrowserContextDependencyManager::GetInstance()) {
}
"captive_portal::CaptivePortalService",
BrowserContextDependencyManager::GetInstance()) {}

CaptivePortalServiceFactory::~CaptivePortalServiceFactory() {
}

KeyedService* CaptivePortalServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* profile) const {
return new CaptivePortalService(profile,
static_cast<Profile*>(profile)->GetPrefs());
return new captive_portal::CaptivePortalService(
profile, static_cast<Profile*>(profile)->GetPrefs());
}

content::BrowserContext* CaptivePortalServiceFactory::GetBrowserContextToUse(
Expand Down
14 changes: 8 additions & 6 deletions chrome/browser/captive_portal/captive_portal_service_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@

class Profile;

namespace captive_portal {
class CaptivePortalService;
}

// Singleton that owns all CaptivePortalServices and associates them with
// Profiles. Listens for the Profile's destruction notification and cleans up
// the associated CaptivePortalService. Incognito profiles have their own
// CaptivePortalService.
// Singleton that owns all captive_portal::CaptivePortalServices and associates
// them with Profiles. Listens for the Profile's destruction notification and
// cleans up the associated captive_portal::CaptivePortalService. Incognito
// profiles have their own captive_portal::CaptivePortalService.
class CaptivePortalServiceFactory : public BrowserContextKeyedServiceFactory {
public:
// Returns the CaptivePortalService for |profile|.
static CaptivePortalService* GetForProfile(Profile* profile);
// Returns the captive_portal::CaptivePortalService for |profile|.
static captive_portal::CaptivePortalService* GetForProfile(Profile* profile);

static CaptivePortalServiceFactory* GetInstance();

Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ void HandleSSLErrorWrapper(
if (!profile)
return;

CaptivePortalService* captive_portal_service = nullptr;
captive_portal::CaptivePortalService* captive_portal_service = nullptr;

#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
captive_portal_service = CaptivePortalServiceFactory::GetForProfile(profile);
Expand Down Expand Up @@ -4306,7 +4306,8 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(

#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
result.push_back(
std::make_unique<CaptivePortalURLLoaderThrottle>(wc_getter.Run()));
std::make_unique<captive_portal::CaptivePortalURLLoaderThrottle>(
wc_getter.Run()));
#endif

if (chrome_navigation_ui_data &&
Expand Down Expand Up @@ -4583,8 +4584,9 @@ bool ChromeContentBrowserClient::WillCreateURLLoaderFactory(
if (disable_secure_dns) {
WebContents* web_contents = WebContents::FromRenderFrameHost(frame);
*disable_secure_dns =
web_contents && CaptivePortalTabHelper::FromWebContents(web_contents) &&
CaptivePortalTabHelper::FromWebContents(web_contents)
web_contents &&
captive_portal::CaptivePortalTabHelper::FromWebContents(web_contents) &&
captive_portal::CaptivePortalTabHelper::FromWebContents(web_contents)
->is_captive_portal_window();
}
#endif
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/chrome_content_browser_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -573,10 +573,10 @@ TEST_F(ChromeContentBrowserClientCaptivePortalBrowserTest,
bool invoked_url_factory = false;
cp_rph_factory_.SetupForTracking(&invoked_url_factory,
true /* expected_disable_secure_dns */);
CaptivePortalTabHelper::CreateForWebContents(
captive_portal::CaptivePortalTabHelper::CreateForWebContents(
web_contents(), CaptivePortalServiceFactory::GetForProfile(profile()),
base::Callback<void(void)>());
CaptivePortalTabHelper::FromWebContents(web_contents())
captive_portal::CaptivePortalTabHelper::FromWebContents(web_contents())
->set_is_captive_portal_window();
NavigateAndCommit(GURL("https://www.google.com"), ui::PAGE_TRANSITION_LINK);
EXPECT_TRUE(invoked_url_factory);
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/net/network_portal_detector_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class NetworkState;

// This class handles all notifications about network changes from
// NetworkStateHandler and delegates portal detection for the default
// network to CaptivePortalService.
// network to captive_portal::CaptivePortalService.
class NetworkPortalDetectorImpl : public NetworkPortalDetector,
public chromeos::NetworkStateHandlerObserver,
public content::NotificationObserver,
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ssl/captive_portal_metrics_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void CaptivePortalMetricsRecorder::RecordCaptivePortalUMAStatistics() const {
}

void CaptivePortalMetricsRecorder::Observe(
const CaptivePortalService::Results& results) {
const captive_portal::CaptivePortalService::Results& results) {
// When detection is disabled, captive portal service always sends
// RESULT_INTERNET_CONNECTED. Ignore any probe results in that case.
if (!captive_portal_detection_enabled_)
Expand Down
9 changes: 5 additions & 4 deletions chrome/browser/ssl/captive_portal_metrics_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class WebContents;

// This class helps the SSL interstitial record captive portal-specific
// metrics. It should only be used on the UI thread because its implementation
// uses captive_portal::CaptivePortalService which can only be accessed on the
// UI thread.
// uses captive_portal::CaptivePortalService which can only be
// accessed on the UI thread.
class CaptivePortalMetricsRecorder {
public:
CaptivePortalMetricsRecorder(content::WebContents* web_contents,
Expand All @@ -33,7 +33,7 @@ class CaptivePortalMetricsRecorder {
private:
typedef std::vector<std::string> Tokens;

void Observe(const CaptivePortalService::Results& results);
void Observe(const captive_portal::CaptivePortalService::Results& results);

bool overridable_;
bool captive_portal_detection_enabled_;
Expand All @@ -43,7 +43,8 @@ class CaptivePortalMetricsRecorder {
bool captive_portal_no_response_;
bool captive_portal_detected_;

std::unique_ptr<CaptivePortalService::Subscription> subscription_;
std::unique_ptr<captive_portal::CaptivePortalService::Subscription>
subscription_;
};

#endif // CHROME_BROWSER_SSL_CAPTIVE_PORTAL_METRICS_RECORDER_H_
19 changes: 10 additions & 9 deletions chrome/browser/ssl/chrome_security_blocking_page_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,8 @@ void ChromeSecurityBlockingPageFactory::OpenLoginTabForWebContents(
if (secure_dns_mode == net::DnsConfig::SecureDnsMode::SECURE) {
// If there is already a captive portal popup window, do not create another.
for (auto* contents : AllTabContentses()) {
CaptivePortalTabHelper* captive_portal_tab_helper =
CaptivePortalTabHelper::FromWebContents(contents);
captive_portal::CaptivePortalTabHelper* captive_portal_tab_helper =
captive_portal::CaptivePortalTabHelper::FromWebContents(contents);
if (captive_portal_tab_helper->IsLoginTab()) {
Browser* browser_with_login_tab =
chrome::FindBrowserWithWebContents(contents);
Expand All @@ -418,8 +418,8 @@ void ChromeSecurityBlockingPageFactory::OpenLoginTabForWebContents(
params.is_captive_portal_popup = true;
Navigate(&params);
content::WebContents* new_contents = params.navigated_or_inserted_contents;
CaptivePortalTabHelper* captive_portal_tab_helper =
CaptivePortalTabHelper::FromWebContents(new_contents);
captive_portal::CaptivePortalTabHelper* captive_portal_tab_helper =
captive_portal::CaptivePortalTabHelper::FromWebContents(new_contents);
captive_portal_tab_helper->SetIsLoginTab();
return;
}
Expand All @@ -431,8 +431,8 @@ void ChromeSecurityBlockingPageFactory::OpenLoginTabForWebContents(
for (int i = 0; i < browser->tab_strip_model()->count(); ++i) {
content::WebContents* contents =
browser->tab_strip_model()->GetWebContentsAt(i);
CaptivePortalTabHelper* captive_portal_tab_helper =
CaptivePortalTabHelper::FromWebContents(contents);
captive_portal::CaptivePortalTabHelper* captive_portal_tab_helper =
captive_portal::CaptivePortalTabHelper::FromWebContents(contents);
if (captive_portal_tab_helper->IsLoginTab()) {
if (focus)
browser->tab_strip_model()->ActivateTabAt(i);
Expand All @@ -441,14 +441,15 @@ void ChromeSecurityBlockingPageFactory::OpenLoginTabForWebContents(
}

// Otherwise, open a login tab. Only end up here when a captive portal result
// was received, so it's safe to assume profile has a CaptivePortalService.
// was received, so it's safe to assume profile has a
// captive_portal::CaptivePortalService.
content::WebContents* new_contents = chrome::AddSelectedTabWithURL(
browser,
CaptivePortalServiceFactory::GetForProfile(browser->profile())
->test_url(),
ui::PAGE_TRANSITION_TYPED);
CaptivePortalTabHelper* captive_portal_tab_helper =
CaptivePortalTabHelper::FromWebContents(new_contents);
captive_portal::CaptivePortalTabHelper* captive_portal_tab_helper =
captive_portal::CaptivePortalTabHelper::FromWebContents(new_contents);
captive_portal_tab_helper->SetIsLoginTab();
}
#endif // BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/browser_navigator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,8 @@ std::unique_ptr<content::WebContents> CreateTargetContents(
#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
if (params.is_captive_portal_popup) {
DCHECK_EQ(WindowOpenDisposition::NEW_POPUP, params.disposition);
CaptivePortalTabHelper::FromWebContents(target_contents.get())
captive_portal::CaptivePortalTabHelper::FromWebContents(
target_contents.get())
->set_is_captive_portal_window();
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/browser_navigator_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
// Navigate() should have opened a new popup window of TYPE_TRUSTED_POPUP.
EXPECT_NE(browser(), params.browser);
EXPECT_TRUE(params.browser->is_type_popup());
EXPECT_TRUE(CaptivePortalTabHelper::FromWebContents(
EXPECT_TRUE(captive_portal::CaptivePortalTabHelper::FromWebContents(
params.navigated_or_inserted_contents)
->is_captive_portal_window());
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/tab_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
// --- Feature tab helpers behind flags ---

#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
CaptivePortalTabHelper::CreateForWebContents(
captive_portal::CaptivePortalTabHelper::CreateForWebContents(
web_contents, CaptivePortalServiceFactory::GetForProfile(profile),
base::Bind(&ChromeSecurityBlockingPageFactory::OpenLoginTabForWebContents,
web_contents, false));
Expand Down
4 changes: 2 additions & 2 deletions chrome/test/base/in_process_browser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ void InProcessBrowserTest::SetUp() {
#endif

#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
CaptivePortalService::set_state_for_testing(
CaptivePortalService::DISABLED_FOR_TESTING);
captive_portal::CaptivePortalService::set_state_for_testing(
captive_portal::CaptivePortalService::DISABLED_FOR_TESTING);
#endif

chrome_browser_net::NetErrorTabHelper::set_state_for_testing(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

#include "components/captive_portal/core/captive_portal_types.h"

using captive_portal::CaptivePortalResult;
namespace captive_portal {

CaptivePortalLoginDetector::CaptivePortalLoginDetector(
CaptivePortalService* captive_portal_service)
Expand All @@ -30,11 +30,13 @@ void CaptivePortalLoginDetector::OnStoppedLoading() {
void CaptivePortalLoginDetector::OnCaptivePortalResults(
CaptivePortalResult previous_result,
CaptivePortalResult result) {
if (result != captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL)
if (result != RESULT_BEHIND_CAPTIVE_PORTAL)
is_login_tab_ = false;
}

void CaptivePortalLoginDetector::SetIsLoginTab() {
is_login_tab_ = true;
first_login_tab_load_ = true;
}

} // namespace captive_portal
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include "base/macros.h"
#include "components/captive_portal/content/captive_portal_service.h"

namespace captive_portal {

// Triggers a captive portal test on navigations that may indicate a captive
// portal has been logged into. Currently only tracks if a page was opened
// at a captive portal tab's login page, and triggers checks every navigation
Expand All @@ -24,9 +26,8 @@ class CaptivePortalLoginDetector {
~CaptivePortalLoginDetector();

void OnStoppedLoading();
void OnCaptivePortalResults(
captive_portal::CaptivePortalResult previous_result,
captive_portal::CaptivePortalResult result);
void OnCaptivePortalResults(CaptivePortalResult previous_result,
CaptivePortalResult result);

bool is_login_tab() const { return is_login_tab_; }
void SetIsLoginTab();
Expand All @@ -45,4 +46,6 @@ class CaptivePortalLoginDetector {
DISALLOW_COPY_AND_ASSIGN(CaptivePortalLoginDetector);
};

} // namespace captive_portal

#endif // COMPONENTS_CAPTIVE_PORTAL_CONTENT_CAPTIVE_PORTAL_LOGIN_DETECTOR_H_
24 changes: 13 additions & 11 deletions components/captive_portal/content/captive_portal_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include "base/win/windows_version.h"
#endif

using captive_portal::CaptivePortalResult;
namespace captive_portal {

namespace {

Expand Down Expand Up @@ -90,21 +90,21 @@ void RecordRepeatHistograms(CaptivePortalResult result,
}

CaptivePortalDetectionResult GetHistogramEntryForDetectionResult(
const captive_portal::CaptivePortalDetector::Results& results) {
const CaptivePortalDetector::Results& results) {
bool is_https = results.landing_url.SchemeIs("https");
bool is_ip = results.landing_url.HostIsIPAddress();
switch (results.result) {
case captive_portal::RESULT_INTERNET_CONNECTED:
case RESULT_INTERNET_CONNECTED:
return DETECTION_RESULT_INTERNET_CONNECTED;
case captive_portal::RESULT_NO_RESPONSE:
case RESULT_NO_RESPONSE:
if (is_ip) {
return is_https
? DETECTION_RESULT_NO_RESPONSE_HTTPS_LANDING_URL_IP_ADDRESS
: DETECTION_RESULT_NO_RESPONSE_IP_ADDRESS;
}
return is_https ? DETECTION_RESULT_NO_RESPONSE_HTTPS_LANDING_URL
: DETECTION_RESULT_NO_RESPONSE;
case captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL:
case RESULT_BEHIND_CAPTIVE_PORTAL:
if (is_ip) {
return is_https
? DETECTION_RESULT_BEHIND_CAPTIVE_PORTAL_HTTPS_LANDING_URL_IP_ADDRESS
Expand Down Expand Up @@ -172,9 +172,9 @@ CaptivePortalService::CaptivePortalService(
: browser_context_(browser_context),
state_(STATE_IDLE),
enabled_(false),
last_detection_result_(captive_portal::RESULT_INTERNET_CONNECTED),
last_detection_result_(RESULT_INTERNET_CONNECTED),
num_checks_with_same_result_(0),
test_url_(captive_portal::CaptivePortalDetector::kDefaultURL),
test_url_(CaptivePortalDetector::kDefaultURL),
tick_clock_for_testing_(clock_for_testing) {
network::mojom::URLLoaderFactory* loader_factory;
if (loader_factory_for_testing) {
Expand All @@ -186,7 +186,7 @@ CaptivePortalService::CaptivePortalService(
loader_factory = shared_url_loader_factory_.get();
}
captive_portal_detector_ =
std::make_unique<captive_portal::CaptivePortalDetector>(loader_factory);
std::make_unique<CaptivePortalDetector>(loader_factory);

DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// The order matters here:
Expand Down Expand Up @@ -237,7 +237,7 @@ void CaptivePortalService::DetectCaptivePortalInternal() {
// Count this as a success, so the backoff entry won't apply exponential
// backoff, but will apply the standard delay.
backoff_entry_->InformOfRequest(true);
OnResult(captive_portal::RESULT_INTERNET_CONNECTED, GURL());
OnResult(RESULT_INTERNET_CONNECTED, GURL());
return;
}

Expand Down Expand Up @@ -277,7 +277,7 @@ void CaptivePortalService::DetectCaptivePortalInternal() {
}

void CaptivePortalService::OnPortalDetectionCompleted(
const captive_portal::CaptivePortalDetector::Results& results) {
const CaptivePortalDetector::Results& results) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK_EQ(STATE_CHECKING_FOR_PORTAL, state_);
DCHECK(!TimerRunning());
Expand Down Expand Up @@ -359,7 +359,7 @@ void CaptivePortalService::OnResult(CaptivePortalResult result,
}

void CaptivePortalService::ResetBackoffEntry(CaptivePortalResult result) {
if (!enabled_ || result == captive_portal::RESULT_BEHIND_CAPTIVE_PORTAL) {
if (!enabled_ || result == RESULT_BEHIND_CAPTIVE_PORTAL) {
// Use the shorter time when the captive portal service is not enabled, or
// behind a captive portal.
recheck_policy_.backoff_policy.initial_delay_ms =
Expand Down Expand Up @@ -421,3 +421,5 @@ bool CaptivePortalService::DetectionInProgress() const {
bool CaptivePortalService::TimerRunning() const {
return check_captive_portal_timer_.IsRunning();
}

} // namespace captive_portal
Loading

0 comments on commit 27331f9

Please sign in to comment.