Refactor NetworkPortalDetector and NetworkPortalNotificationController.

* NetworkPortalNotificationController directly observes network and portal changes.
* conditionally create NetworkPortalNotificationController
* move NetworkPortalDetectorStubImpl to stub files.
* make interface implementations private
* initialize data members in header

BUG=530485
TEST=unit tests and browser tests.

Review URL: https://codereview.chromium.org/1346843003

Cr-Commit-Position: refs/heads/master@{#350202}

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -553,7 +553,6 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
   NetworkPortalDetectorImpl::Initialize(
       g_browser_process->system_request_context());
   {
-    NetworkPortalDetector* detector = NetworkPortalDetector::Get();
 #if defined(GOOGLE_CHROME_BUILD)
     bool is_official_build = true;
 #else
@@ -562,7 +561,7 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() {
     // Enable portal detector if EULA was previously accepted or if
     // this is an unofficial build.
     if (!is_official_build || StartupUtils::IsEulaAccepted())
-      detector->Enable(true);
+      NetworkPortalDetector::Get()->Enable(true);
   }
 
   // Initialize input methods.

chrome/browser/chromeos/net/network_portal_detector_impl.cc

@@ -13,11 +13,13 @@
 #include "base/metrics/histogram.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/chromeos/net/network_portal_notification_controller.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_profile_client.h"
 #include "chromeos/login/login_state.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/portal_detector/network_portal_detector_stub.h"
 #include "components/device_event_log/device_event_log.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/content_switches.h"
@@ -209,34 +211,38 @@ const char NetworkPortalDetectorImpl::kSessionPortalToOnlineHistogram[] =
 // static
 void NetworkPortalDetectorImpl::Initialize(
     net::URLRequestContextGetter* url_context) {
-  if (NetworkPortalDetector::set_for_testing())
+  if (set_for_testing())
     return;
-  CHECK(!NetworkPortalDetector::network_portal_detector())
+  CHECK(!network_portal_detector())
       << "NetworkPortalDetector was initialized twice.";
-  NET_LOG(EVENT) << "NetworkPortalDetectorImpl::Initialize()";
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(::switches::kTestType))
-    set_network_portal_detector(new NetworkPortalDetectorStubImpl());
-  else
-    set_network_portal_detector(new NetworkPortalDetectorImpl(url_context));
+  NET_LOG(EVENT) << "NetworkPortalDetectorImpl::Initialize";
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ::switches::kTestType)) {
+    set_network_portal_detector(new NetworkPortalDetectorStub());
+  } else {
+    set_network_portal_detector(
+        new NetworkPortalDetectorImpl(url_context, true));
+  }
 }
 
 NetworkPortalDetectorImpl::NetworkPortalDetectorImpl(
-    const scoped_refptr<net::URLRequestContextGetter>& request_context)
-    : state_(STATE_IDLE),
-      test_url_(CaptivePortalDetector::kDefaultURL),
-      enabled_(false),
+    const scoped_refptr<net::URLRequestContextGetter>& request_context,
+    bool create_notification_controller)
+    : test_url_(CaptivePortalDetector::kDefaultURL),
       strategy_(PortalDetectorStrategy::CreateById(
           PortalDetectorStrategy::STRATEGY_ID_LOGIN_SCREEN,
           this)),
-      last_detection_result_(CAPTIVE_PORTAL_STATUS_UNKNOWN),
-      same_detection_result_count_(0),
-      no_response_result_count_(0),
       weak_factory_(this) {
   NET_LOG(EVENT) << "NetworkPortalDetectorImpl::NetworkPortalDetectorImpl()";
   captive_portal_detector_.reset(new CaptivePortalDetector(request_context));
 
-  notification_controller_.set_retry_detection_callback(base::Bind(
-      &NetworkPortalDetectorImpl::RetryDetection, base::Unretained(this)));
+  if (create_notification_controller) {
+    notification_controller_.reset(
+        new NetworkPortalNotificationController(this));
+    notification_controller_->set_retry_detection_callback(
+        base::Bind(&NetworkPortalDetectorImpl::RetryDetection,
+                   weak_factory_.GetWeakPtr()));
+  }
 
   registrar_.Add(this,
                  chrome::NOTIFICATION_LOGIN_PROXY_CHANGED,
@@ -337,14 +343,14 @@ void NetworkPortalDetectorImpl::SetStrategy(
 }
 
 void NetworkPortalDetectorImpl::OnLockScreenRequest() {
-  notification_controller_.CloseDialog();
+  if (notification_controller_)
+    notification_controller_->CloseDialog();
 }
 
 void NetworkPortalDetectorImpl::DefaultNetworkChanged(
     const NetworkState* default_network) {
   DCHECK(CalledOnValidThread());
 
-  notification_controller_.DefaultNetworkChanged(default_network);
   if (!default_network) {
     NET_LOG(EVENT) << "Default network changed: None";
 
@@ -354,7 +360,7 @@ void NetworkPortalDetectorImpl::DefaultNetworkChanged(
 
     CaptivePortalState state;
     state.status = CAPTIVE_PORTAL_STATUS_OFFLINE;
-    OnDetectionCompleted(NULL, state);
+    OnDetectionCompleted(nullptr, state);
     return;
   }
 
@@ -608,7 +614,6 @@ void NetworkPortalDetectorImpl::NotifyDetectionCompleted(
     const CaptivePortalState& state) {
   FOR_EACH_OBSERVER(
       Observer, observers_, OnPortalDetectionCompleted(network, state));
-  notification_controller_.OnPortalDetectionCompleted(network, state);
 }
 
 bool NetworkPortalDetectorImpl::AttemptTimeoutIsCancelledForTesting() const {

chrome/browser/chromeos/net/network_portal_detector_impl.h

@@ -17,7 +17,6 @@
 #include "base/observer_list.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/time/time.h"
-#include "chrome/browser/chromeos/net/network_portal_notification_controller.h"
 #include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/network/portal_detector/network_portal_detector.h"
 #include "chromeos/network/portal_detector/network_portal_detector_strategy.h"
@@ -36,6 +35,7 @@ class URLRequestContextGetter;
 
 namespace chromeos {
 
+class NetworkPortalNotificationController;
 class NetworkState;
 
 // This class handles all notifications about network changes from
@@ -62,37 +62,30 @@ class NetworkPortalDetectorImpl
   static const char kSessionShillOfflineHistogram[];
   static const char kSessionPortalToOnlineHistogram[];
 
-  // Creates an instance of NetworkPortalDetectorImpl.
+  // Creates an instance of the implementation or a stub.
   static void Initialize(net::URLRequestContextGetter* url_context);
 
-  explicit NetworkPortalDetectorImpl(
-      const scoped_refptr<net::URLRequestContextGetter>& request_context);
+  NetworkPortalDetectorImpl(
+      const scoped_refptr<net::URLRequestContextGetter>& request_context,
+      bool create_notification_controller);
   ~NetworkPortalDetectorImpl() override;
 
-  // NetworkPortalDetector implementation:
-  void AddObserver(Observer* observer) override;
-  void AddAndFireObserver(Observer* observer) override;
-  void RemoveObserver(Observer* observer) override;
-  CaptivePortalState GetCaptivePortalState(const std::string& guid) override;
-  bool IsEnabled() override;
-  void Enable(bool start_detection) override;
-  bool StartDetectionIfIdle() override;
-  void SetStrategy(PortalDetectorStrategy::StrategyId id) override;
-  void OnLockScreenRequest() override;
-
-  // NetworkStateHandlerObserver implementation:
-  void DefaultNetworkChanged(const NetworkState* network) override;
-
-  // PortalDetectorStrategy::Delegate implementation:
-  int NoResponseResultCount() override;
-  base::TimeTicks AttemptStartTime() override;
-  base::TimeTicks NowTicks() override;
-
  private:
   friend class ::NetworkingConfigTest;
   friend class NetworkPortalDetectorImplTest;
   friend class NetworkPortalDetectorImplBrowserTest;
 
+  using CaptivePortalStateMap = base::hash_map<std::string, CaptivePortalState>;
+
+  enum State {
+    // No portal check is running.
+    STATE_IDLE = 0,
+    // Waiting for portal check.
+    STATE_PORTAL_CHECK_PENDING,
+    // Portal check is in progress.
+    STATE_CHECKING_FOR_PORTAL,
+  };
+
   struct DetectionAttemptCompletedReport {
     DetectionAttemptCompletedReport();
 
@@ -111,18 +104,6 @@ class NetworkPortalDetectorImpl
     int response_code;
   };
 
-  typedef std::string NetworkId;
-  typedef base::hash_map<NetworkId, CaptivePortalState> CaptivePortalStateMap;
-
-  enum State {
-    // No portal check is running.
-    STATE_IDLE = 0,
-    // Waiting for portal check.
-    STATE_PORTAL_CHECK_PENDING,
-    // Portal check is in progress.
-    STATE_CHECKING_FOR_PORTAL,
-  };
-
   // Starts detection process.
   void StartDetection();
 
@@ -146,6 +127,25 @@ class NetworkPortalDetectorImpl
   void OnAttemptCompleted(
       const captive_portal::CaptivePortalDetector::Results& results);
 
+  // NetworkPortalDetector implementation:
+  void AddObserver(Observer* observer) override;
+  void AddAndFireObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
+  CaptivePortalState GetCaptivePortalState(const std::string& guid) override;
+  bool IsEnabled() override;
+  void Enable(bool start_detection) override;
+  bool StartDetectionIfIdle() override;
+  void SetStrategy(PortalDetectorStrategy::StrategyId id) override;
+  void OnLockScreenRequest() override;
+
+  // NetworkStateHandlerObserver implementation:
+  void DefaultNetworkChanged(const NetworkState* network) override;
+
+  // PortalDetectorStrategy::Delegate implementation:
+  int NoResponseResultCount() override;
+  base::TimeTicks AttemptStartTime() override;
+  base::TimeTicks NowTicks() override;
+
   // content::NotificationObserver implementation:
   void Observe(int type,
                const content::NotificationSource& source,
@@ -220,7 +220,7 @@ class NetworkPortalDetectorImpl
   // Connection state of the default network.
   std::string default_connection_state_;
 
-  State state_;
+  State state_ = STATE_IDLE;
   CaptivePortalStateMap portal_state_map_;
   base::ObserverList<Observer> observers_;
 
@@ -234,7 +234,7 @@ class NetworkPortalDetectorImpl
   scoped_ptr<captive_portal::CaptivePortalDetector> captive_portal_detector_;
 
   // True if the NetworkPortalDetector is enabled.
-  bool enabled_;
+  bool enabled_ = false;
 
   // Start time of portal detection.
   base::TimeTicks detection_start_time_;
@@ -249,16 +249,16 @@ class NetworkPortalDetectorImpl
   scoped_ptr<PortalDetectorStrategy> strategy_;
 
   // Last received result from captive portal detector.
-  CaptivePortalStatus last_detection_result_;
+  CaptivePortalStatus last_detection_result_ = CAPTIVE_PORTAL_STATUS_UNKNOWN;
 
   // Number of detection attempts with same result in a row.
-  int same_detection_result_count_;
+  int same_detection_result_count_ = 0;
 
   // Number of detection attempts in a row with NO RESPONSE result.
-  int no_response_result_count_;
+  int no_response_result_count_ = 0;
 
   // UI notification controller about captive portal state.
-  NetworkPortalNotificationController notification_controller_;
+  scoped_ptr<NetworkPortalNotificationController> notification_controller_;
 
   content::NotificationRegistrar registrar_;
 

chrome/browser/chromeos/net/network_portal_detector_impl_browsertest.cc

@@ -128,7 +128,8 @@ class NetworkPortalDetectorImplBrowserTest
         base::Bind(&ErrorCallbackFunction));
 
     network_portal_detector_ = new NetworkPortalDetectorImpl(
-        g_browser_process->system_request_context());
+        g_browser_process->system_request_context(),
+        true /* create_notification_controller */);
     NetworkPortalDetector::InitializeForTesting(network_portal_detector_);
     network_portal_detector_->Enable(false /* start_detection */);
     set_detector(network_portal_detector_->captive_portal_detector_.get());
@@ -151,12 +152,12 @@ class NetworkPortalDetectorImplBrowserTest
 
   void SetIgnoreNoNetworkForTesting() {
     network_portal_detector_->notification_controller_
-        .SetIgnoreNoNetworkForTesting();
+        ->SetIgnoreNoNetworkForTesting();
   }
 
   const NetworkPortalWebDialog* GetDialog() const {
     return network_portal_detector_->notification_controller_
-        .GetDialogForTesting();
+        ->GetDialogForTesting();
   }
 
  private:

chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc

@@ -99,7 +99,7 @@ class NetworkPortalDetectorImplTest
     EXPECT_TRUE(user_manager::UserManager::Get()->GetPrimaryUser());
 
     network_portal_detector_.reset(
-        new NetworkPortalDetectorImpl(profile_->GetRequestContext()));
+        new NetworkPortalDetectorImpl(profile_->GetRequestContext(), false));
     network_portal_detector_->Enable(false);
 
     set_detector(network_portal_detector_->captive_portal_detector_.get());
@@ -152,6 +152,14 @@ class NetworkPortalDetectorImplTest
     return network_portal_detector_.get();
   }
 
+  void AddObserver(NetworkPortalDetector::Observer* observer) {
+    network_portal_detector()->AddObserver(observer);
+  }
+
+  void RemoveObserver(NetworkPortalDetector::Observer* observer) {
+    network_portal_detector()->RemoveObserver(observer);
+  }
+
   NetworkPortalDetectorImpl::State state() {
     return network_portal_detector()->state();
   }
@@ -360,7 +368,7 @@ TEST_F(NetworkPortalDetectorImplTest, Online2Offline) {
   ASSERT_TRUE(is_state_idle());
 
   MockObserver observer;
-  network_portal_detector()->AddObserver(&observer);
+  AddObserver(&observer);
 
   NetworkPortalDetector::CaptivePortalState offline_state;
   offline_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_OFFLINE;
@@ -401,7 +409,7 @@ TEST_F(NetworkPortalDetectorImplTest, Online2Offline) {
     Mock::VerifyAndClearExpectations(&observer);
   }
 
-  network_portal_detector()->RemoveObserver(&observer);
+  RemoveObserver(&observer);
 
   ASSERT_TRUE(
       MakeResultHistogramChecker()

chrome/browser/chromeos/net/network_portal_notification_controller.cc

@@ -36,6 +36,7 @@
 #include "chrome/grit/theme_resources.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_type_pattern.h"
 #include "components/captive_portal/captive_portal_detector.h"
 #include "components/user_manager/user_manager.h"
@@ -235,12 +236,24 @@ const char NetworkPortalNotificationController::kNotificationMetric[] =
 const char NetworkPortalNotificationController::kUserActionMetric[] =
     "CaptivePortal.Notification.UserAction";
 
-NetworkPortalNotificationController::NetworkPortalNotificationController()
-    : dialog_(nullptr),
-      ignore_no_network_for_testing_(false),
-      weak_factory_(this) {}
+NetworkPortalNotificationController::NetworkPortalNotificationController(
+    NetworkPortalDetector* network_portal_detector)
+    : network_portal_detector_(network_portal_detector), weak_factory_(this) {
+  if (NetworkHandler::IsInitialized()) {  // NULL for unit tests.
+    NetworkHandler::Get()->network_state_handler()->AddObserver(this,
+                                                                FROM_HERE);
+  }
+  if (network_portal_detector_)
+    network_portal_detector_->AddObserver(this);
+}
 
 NetworkPortalNotificationController::~NetworkPortalNotificationController() {
+  if (network_portal_detector_)
+    network_portal_detector_->RemoveObserver(this);
+  if (NetworkHandler::IsInitialized()) {
+    NetworkHandler::Get()->network_state_handler()->RemoveObserver(this,
+                                                                   FROM_HERE);
+  }
 }
 
 void NetworkPortalNotificationController::DefaultNetworkChanged(

chrome/browser/chromeos/net/network_portal_notification_controller.h

@@ -9,22 +9,24 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/network/portal_detector/network_portal_detector.h"
 #include "ui/message_center/notification.h"
 
 namespace extensions {
-
 class Extension;
 class NetworkingConfigService;
-
-}  // namespace extensions
+}
 
 namespace chromeos {
 
 class NetworkState;
 class NetworkPortalWebDialog;
+class NetworkPortalNotificationControllerTest;
 
-class NetworkPortalNotificationController {
+class NetworkPortalNotificationController
+    : public NetworkStateHandlerObserver,
+      public NetworkPortalDetector::Observer {
  public:
   // The values of these metrics are being used for UMA gathering, so it is
   // important that they don't change between releases.
@@ -53,8 +55,9 @@ class NetworkPortalNotificationController {
   static const char kNotificationMetric[];
   static const char kUserActionMetric[];
 
-  NetworkPortalNotificationController();
-  ~NetworkPortalNotificationController();
+  explicit NetworkPortalNotificationController(
+      NetworkPortalDetector* network_portal_dectector);
+  ~NetworkPortalNotificationController() override;
 
   // |retry_detection_callback| will be called if the controller learns about a
   // potential change of the captive portal (e.g. if an extension notifies about
@@ -66,12 +69,6 @@ class NetworkPortalNotificationController {
     retry_detection_callback_ = retry_detection_callback;
   }
 
-  void DefaultNetworkChanged(const NetworkState* network);
-
-  void OnPortalDetectionCompleted(
-      const NetworkState* network,
-      const NetworkPortalDetector::CaptivePortalState& state);
-
   // Creates NetworkPortalWebDialog.
   void ShowDialog();
 
@@ -93,6 +90,8 @@ class NetworkPortalNotificationController {
   const NetworkPortalWebDialog* GetDialogForTesting() const;
 
  private:
+  friend NetworkPortalNotificationControllerTest;
+
   // Creates the default notification informing the user that a captive portal
   // has been detected. On click the captive portal login page is opened in the
   // browser.
@@ -116,14 +115,25 @@ class NetworkPortalNotificationController {
       const NetworkState* network,
       const NetworkPortalDetector::CaptivePortalState& state);
 
+  // NetworkStateHandlerObserver:
+  void DefaultNetworkChanged(const NetworkState* network) override;
+
+  // NetworkPortalDetector::Observer:
+  void OnPortalDetectionCompleted(
+      const NetworkState* network,
+      const NetworkPortalDetector::CaptivePortalState& state) override;
+
   // Last network path for which notification was displayed.
   std::string last_network_path_;
 
+  // Backpointer to owner.
+  NetworkPortalDetector* network_portal_detector_ = nullptr;
+
   // Currently displayed authorization dialog, or NULL if none.
-  NetworkPortalWebDialog* dialog_;
+  NetworkPortalWebDialog* dialog_ = nullptr;
 
   // Do not close Portal Login dialog on "No network" error in browser tests.
-  bool ignore_no_network_for_testing_;
+  bool ignore_no_network_for_testing_ = false;
 
   // This is called if the controller learns about a potential change of the
   // captive portal.

chrome/browser/chromeos/net/network_portal_notification_controller_unittest.cc

@@ -64,7 +64,8 @@ class NotificationObserver : public message_center::MessageCenterObserver {
 class NetworkPortalNotificationControllerTest : public testing::Test {
  public:
   NetworkPortalNotificationControllerTest()
-      : user_manager_enabler_(new chromeos::FakeChromeUserManager()) {}
+      : user_manager_enabler_(new chromeos::FakeChromeUserManager()),
+        controller_(nullptr) {}
   ~NetworkPortalNotificationControllerTest() override {}
 
   void SetUp() override {

chrome/browser/extensions/api/networking_config_chromeos_apitest_chromeos.cc

@@ -8,6 +8,7 @@
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/net/network_portal_detector_impl.h"
+#include "chrome/browser/chromeos/net/network_portal_notification_controller.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_device_client.h"
@@ -105,7 +106,8 @@ class NetworkingConfigTest
     content::RunAllPendingInMessageLoop();
 
     network_portal_detector_ = new NetworkPortalDetectorImpl(
-        g_browser_process->system_request_context());
+        g_browser_process->system_request_context(),
+        true /* create_notification_controller */);
     NetworkPortalDetector::InitializeForTesting(network_portal_detector_);
     network_portal_detector_->Enable(false /* start_detection */);
     set_detector(network_portal_detector_->captive_portal_detector_.get());
@@ -133,6 +135,12 @@ class NetworkingConfigTest
     CompleteURLFetch(net::OK, 204, nullptr);
   }
 
+  NetworkPortalDetector::CaptivePortalStatus GetCaptivePortalStatus(
+      const std::string& guid) {
+    return network_portal_detector_->GetCaptivePortalState(kWifi1ServiceGUID)
+        .status;
+  }
+
   NetworkPortalDetectorImpl* network_portal_detector_ = nullptr;
 
  private:
@@ -178,7 +186,6 @@ IN_PROC_BROWSER_TEST_F(NetworkingConfigTest, FullTest) {
   // Simulate the captive portal vanishing.
   SimulateSuccessfulCaptivePortalAuth();
 
-  NetworkPortalDetector::CaptivePortalState state =
-      network_portal_detector_->GetCaptivePortalState(kWifi1ServiceGUID);
-  ASSERT_EQ(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE, state.status);
+  ASSERT_EQ(NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE,
+            GetCaptivePortalStatus(kWifi1ServiceGUID));
 }

chromeos/chromeos.gyp

@@ -419,6 +419,8 @@
       'network/portal_detector/network_portal_detector.h',
       'network/portal_detector/network_portal_detector_strategy.cc',
       'network/portal_detector/network_portal_detector_strategy.h',
+      'network/portal_detector/network_portal_detector_stub.cc',
+      'network/portal_detector/network_portal_detector_stub.h',
       'network/shill_property_handler.cc',
       'network/shill_property_handler.h',
       'network/shill_property_util.cc',
@@ -503,7 +505,6 @@
       'network/network_configuration_handler_unittest.cc',
       'network/network_connection_handler_unittest.cc',
       'network/network_device_handler_unittest.cc',
-      'network/network_profile_handler_stub.h',
       'network/network_sms_handler_unittest.cc',
       'network/network_state_handler_unittest.cc',
       'network/network_state_unittest.cc',

chromeos/network/portal_detector/network_portal_detector.cc

@@ -21,7 +21,8 @@ const char kCaptivePortalStatusUnrecognized[] = "Unrecognized";
 
 // static
 bool NetworkPortalDetector::set_for_testing_ = false;
-NetworkPortalDetector* NetworkPortalDetector::network_portal_detector_ = NULL;
+NetworkPortalDetector* NetworkPortalDetector::network_portal_detector_ =
+    nullptr;
 
 // static
 void NetworkPortalDetector::InitializeForTesting(
@@ -29,12 +30,11 @@ void NetworkPortalDetector::InitializeForTesting(
   if (network_portal_detector) {
     CHECK(!set_for_testing_)
         << "NetworkPortalDetector::InitializeForTesting is called twice";
-    CHECK(network_portal_detector);
     delete network_portal_detector_;
     network_portal_detector_ = network_portal_detector;
     set_for_testing_ = true;
   } else {
-    network_portal_detector_ = NULL;
+    network_portal_detector_ = nullptr;
     set_for_testing_ = false;
   }
 }
@@ -49,7 +49,7 @@ void NetworkPortalDetector::Shutdown() {
   CHECK(network_portal_detector_ || set_for_testing_)
       << "NetworkPortalDetector::Shutdown() called without Initialize()";
   delete network_portal_detector_;
-  network_portal_detector_ = NULL;
+  network_portal_detector_ = nullptr;
 }
 
 // static
@@ -79,47 +79,4 @@ std::string NetworkPortalDetector::CaptivePortalStatusString(
   return kCaptivePortalStatusUnrecognized;
 }
 
-// NetworkPortalDetectorStubImpl
-
-NetworkPortalDetectorStubImpl::NetworkPortalDetectorStubImpl() {
-}
-
-NetworkPortalDetectorStubImpl::~NetworkPortalDetectorStubImpl() {
-}
-
-void NetworkPortalDetectorStubImpl::AddObserver(Observer* /* observer */) {
-}
-
-void NetworkPortalDetectorStubImpl::AddAndFireObserver(Observer* observer) {
-  if (observer)
-    observer->OnPortalDetectionCompleted(NULL, CaptivePortalState());
-}
-
-void NetworkPortalDetectorStubImpl::RemoveObserver(Observer* /* observer */) {
-}
-
-NetworkPortalDetector::CaptivePortalState
-NetworkPortalDetectorStubImpl::GetCaptivePortalState(
-    const std::string& /* service_path */) {
-  return CaptivePortalState();
-}
-
-bool NetworkPortalDetectorStubImpl::IsEnabled() {
-  return false;
-}
-
-void NetworkPortalDetectorStubImpl::Enable(bool /* start_detection */) {
-}
-
-bool NetworkPortalDetectorStubImpl::StartDetectionIfIdle() {
-  return false;
-}
-
-void NetworkPortalDetectorStubImpl::SetStrategy(
-    PortalDetectorStrategy::StrategyId /* id */) {
-}
-
-void NetworkPortalDetectorStubImpl::OnLockScreenRequest() {
-}
-
 }  // namespace chromeos

chromeos/network/portal_detector/network_portal_detector.h

@@ -101,6 +101,12 @@ class CHROMEOS_EXPORT NetworkPortalDetector {
   // doesn't equal to |id|, detection is restarted.
   virtual void SetStrategy(PortalDetectorStrategy::StrategyId id) = 0;
 
+  // Closes portal login window before screen is locked.
+  virtual void OnLockScreenRequest() = 0;
+
+  // Returns non-localized string representation of |status|.
+  static std::string CaptivePortalStatusString(CaptivePortalStatus status);
+
   // Initializes network portal detector for testing. The
   // |network_portal_detector| will be owned by the internal pointer
   // and deleted by Shutdown().
@@ -119,12 +125,6 @@ class CHROMEOS_EXPORT NetworkPortalDetector {
   // by calls to InitializeForTesting().
   static NetworkPortalDetector* Get();
 
-  // Returns non-localized string representation of |status|.
-  static std::string CaptivePortalStatusString(CaptivePortalStatus status);
-
-  // Closes portal login window before screen is locked.
-  virtual void OnLockScreenRequest() = 0;
-
  protected:
   NetworkPortalDetector() {}
   virtual ~NetworkPortalDetector() {}
@@ -145,29 +145,6 @@ class CHROMEOS_EXPORT NetworkPortalDetector {
   DISALLOW_COPY_AND_ASSIGN(NetworkPortalDetector);
 };
 
-class CHROMEOS_EXPORT NetworkPortalDetectorStubImpl
-    : public NetworkPortalDetector {
- public:
-  NetworkPortalDetectorStubImpl();
-  ~NetworkPortalDetectorStubImpl() override;
-
- protected:
-  // NetworkPortalDetector
-  void AddObserver(Observer* observer) override;
-  void AddAndFireObserver(Observer* observer) override;
-  void RemoveObserver(Observer* observer) override;
-  CaptivePortalState GetCaptivePortalState(
-      const std::string& service_path) override;
-  bool IsEnabled() override;
-  void Enable(bool start_detection) override;
-  bool StartDetectionIfIdle() override;
-  void SetStrategy(PortalDetectorStrategy::StrategyId id) override;
-  void OnLockScreenRequest() override;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(NetworkPortalDetectorStubImpl);
-};
-
 }  // namespace chromeos
 
 #endif  // CHROMEOS_NETWORK_PORTAL_DETECTOR_NETWORK_PORTAL_DETECTOR_H_

chromeos/network/portal_detector/network_portal_detector_stub.cc

@@ -0,0 +1,43 @@
+// Copyright 2015 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 "chromeos/network/portal_detector/network_portal_detector_stub.h"
+
+namespace chromeos {
+
+NetworkPortalDetectorStub::NetworkPortalDetectorStub() {}
+
+NetworkPortalDetectorStub::~NetworkPortalDetectorStub() {}
+
+void NetworkPortalDetectorStub::AddObserver(Observer* observer) {}
+
+void NetworkPortalDetectorStub::AddAndFireObserver(Observer* observer) {
+  if (observer)
+    observer->OnPortalDetectionCompleted(nullptr, CaptivePortalState());
+}
+
+void NetworkPortalDetectorStub::RemoveObserver(Observer* observer) {}
+
+NetworkPortalDetector::CaptivePortalState
+NetworkPortalDetectorStub::GetCaptivePortalState(
+    const std::string& service_path) {
+  return CaptivePortalState();
+}
+
+bool NetworkPortalDetectorStub::IsEnabled() {
+  return false;
+}
+
+void NetworkPortalDetectorStub::Enable(bool start_detection) {}
+
+bool NetworkPortalDetectorStub::StartDetectionIfIdle() {
+  return false;
+}
+
+void NetworkPortalDetectorStub::SetStrategy(
+    PortalDetectorStrategy::StrategyId id) {}
+
+void NetworkPortalDetectorStub::OnLockScreenRequest() {}
+
+}  // namespace chromeos

chromeos/network/portal_detector/network_portal_detector_stub.h

@@ -0,0 +1,35 @@
+// Copyright 2015 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 CHROMEOS_NETWORK_PORTAL_DETECTOR_NETWORK_PORTAL_DETECTOR_STUB_H_
+#define CHROMEOS_NETWORK_PORTAL_DETECTOR_NETWORK_PORTAL_DETECTOR_STUB_H_
+
+#include "chromeos/network/portal_detector/network_portal_detector.h"
+
+namespace chromeos {
+
+class CHROMEOS_EXPORT NetworkPortalDetectorStub : public NetworkPortalDetector {
+ public:
+  NetworkPortalDetectorStub();
+  ~NetworkPortalDetectorStub() override;
+
+ private:
+  // NetworkPortalDetector:
+  void AddObserver(Observer* observer) override;
+  void AddAndFireObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
+  CaptivePortalState GetCaptivePortalState(
+      const std::string& service_path) override;
+  bool IsEnabled() override;
+  void Enable(bool start_detection) override;
+  bool StartDetectionIfIdle() override;
+  void SetStrategy(PortalDetectorStrategy::StrategyId id) override;
+  void OnLockScreenRequest() override;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkPortalDetectorStub);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_PORTAL_DETECTOR_NETWORK_PORTAL_DETECTOR_STUB_H_