Remove a few related uses of NOTIFICATION_SESSION_STARTED

In these files, it was being used to clear notification registrations
to fix http://crbug.com/125276

In NetworkStateInformer, it was doing nothing except removing its own
registration.

In SignInScreenController, the referenced UI object is owned, and
therefore can't have been freed, so the precaution was no longer
necessary.

In ExistingUserController, we can replace with a check to
IsSessionStarted before handling NOTIFICATION_AUTH_SUPPLIED. I don't
understand why this code lives in ExistingUserController to begin with,
and there's a TODO/bug to rework the bodge, but this should preserve
behavior.

Bug: 268984
Change-Id: I219b69997e4873ed60695442a757bd8d549dc00a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771614
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690917}

chrome/browser/chromeos/login/existing_user_controller.cc

@@ -372,8 +372,6 @@ ExistingUserController::ExistingUserController()
       network_state_helper_(new login::NetworkStateHelper) {
   registrar_.Add(this, chrome::NOTIFICATION_AUTH_SUPPLIED,
                  content::NotificationService::AllSources());
-  registrar_.Add(this, chrome::NOTIFICATION_SESSION_STARTED,
-                 content::NotificationService::AllSources());
   show_user_names_subscription_ = cros_settings_->AddSettingsObserver(
       kAccountsPrefShowUserNamesOnSignIn,
       base::Bind(&ExistingUserController::DeviceSettingsChanged,
@@ -480,30 +478,27 @@ void ExistingUserController::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_SESSION_STARTED) {
-    // Stop listening to any notification once session has started.
-    // Sign in screen objects are marked for deletion with DeleteSoon so
-    // make sure no object would be used after session has started.
-    // http://crbug.com/125276
-    registrar_.RemoveAll();
+  DCHECK_EQ(type, chrome::NOTIFICATION_AUTH_SUPPLIED);
+
+  // Don't transfer http auth cache on NOTIFICATION_AUTH_SUPPLIED after user
+  // session starts.
+  if (session_manager::SessionManager::Get()->IsSessionStarted())
     return;
-  }
-  if (type == chrome::NOTIFICATION_AUTH_SUPPLIED) {
-    // Possibly the user has authenticated against a proxy server and we might
-    // need the credentials for enrollment and other system requests from the
-    // main |g_browser_process| request context (see bug
-    // http://crosbug.com/24861). So we transfer any credentials to the global
-    // request context here.
-    // The issue we have here is that the NOTIFICATION_AUTH_SUPPLIED is sent
-    // just after the UI is closed but before the new credentials were stored
-    // in the profile. Therefore we have to give it some time to make sure it
-    // has been updated before we copy it.
-    // TODO(pmarko): Find a better way to do this, see https://crbug.com/796512.
-    VLOG(1) << "Authentication was entered manually, possibly for proxyauth.";
-    base::PostDelayedTask(
-        FROM_HERE, base::BindOnce(&TransferHttpAuthCaches),
-        base::TimeDelta::FromMilliseconds(kAuthCacheTransferDelayMs));
-  }
+
+  // Possibly the user has authenticated against a proxy server and we might
+  // need the credentials for enrollment and other system requests from the
+  // main |g_browser_process| request context (see bug
+  // http://crosbug.com/24861). So we transfer any credentials to the global
+  // request context here.
+  // The issue we have here is that the NOTIFICATION_AUTH_SUPPLIED is sent
+  // just after the UI is closed but before the new credentials were stored
+  // in the profile. Therefore we have to give it some time to make sure it
+  // has been updated before we copy it.
+  // TODO(pmarko): Find a better way to do this, see https://crbug.com/796512.
+  VLOG(1) << "Authentication was entered manually, possibly for proxyauth.";
+  base::PostDelayedTask(
+      FROM_HERE, base::BindOnce(&TransferHttpAuthCaches),
+      base::TimeDelta::FromMilliseconds(kAuthCacheTransferDelayMs));
 }
 
 ////////////////////////////////////////////////////////////////////////////////

chrome/browser/chromeos/login/signin_screen_controller.cc

@@ -34,8 +34,6 @@ SignInScreenController::SignInScreenController(OobeUI* oobe_ui)
   // WeakPtr logic. See crbug.com/685287.
   user_board_view_->Bind(user_selection_screen_.get());
 
-  registrar_.Add(this, chrome::NOTIFICATION_SESSION_STARTED,
-                 content::NotificationService::AllSources());
   user_manager::UserManager::Get()->AddObserver(this);
 }
 
@@ -89,18 +87,4 @@ void SignInScreenController::SetWebUIHandler(
   user_selection_screen_->SetHandler(webui_handler_);
 }
 
-void SignInScreenController::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK_EQ(chrome::NOTIFICATION_SESSION_STARTED, type);
-
-  // Stop listening to any notification once session has started.
-  // Sign in screen objects are marked for deletion with DeleteSoon so
-  // make sure no object would be used after session has started.
-  // http://crbug.com/125276
-  registrar_.RemoveAll();
-  user_manager::UserManager::Get()->RemoveObserver(this);
-}
-
 }  // namespace chromeos

chrome/browser/chromeos/login/signin_screen_controller.h

@@ -13,8 +13,6 @@
 #include "components/user_manager/remove_user_delegate.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 
 class AccountId;
 
@@ -30,8 +28,7 @@ class OobeUI;
 // the user. It is a 'per-session' class; SignInScreenHandler, in comparsion, is
 // tied to the WebContents lifetime and therefore may live beyond this class.
 class SignInScreenController : public user_manager::RemoveUserDelegate,
-                               public user_manager::UserManager::Observer,
-                               public content::NotificationObserver {
+                               public user_manager::UserManager::Observer {
  public:
   explicit SignInScreenController(OobeUI* oobe_ui);
   ~SignInScreenController() override;
@@ -66,11 +63,6 @@ class SignInScreenController : public user_manager::RemoveUserDelegate,
   // user_manager::UserManager::Observer implementation:
   void OnUserImageChanged(const user_manager::User& user) override;
 
-  // content::NotificationObserver implementation.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
-
   static SignInScreenController* instance_;
 
   OobeUI* oobe_ui_ = nullptr;
@@ -83,9 +75,6 @@ class SignInScreenController : public user_manager::RemoveUserDelegate,
 
   base::WeakPtr<UserBoardView> user_board_view_;
 
-  // Used for notifications during the login process.
-  content::NotificationRegistrar registrar_;
-
   DISALLOW_COPY_AND_ASSIGN(SignInScreenController);
 };
 

chrome/browser/chromeos/login/test/oobe_screens_utils.cc

@@ -20,6 +20,7 @@
 #include "chrome/browser/ui/webui/chromeos/login/network_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/update_screen_handler.h"
 #include "chrome/browser/ui/webui/chromeos/login/welcome_screen_handler.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 

chrome/browser/chromeos/login/ui/login_display_host_common.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/ui/webui/chromeos/internet_detail_dialog.h"
 #include "chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h"
 #include "components/keep_alive_registry/keep_alive_types.h"
+#include "content/public/browser/notification_service.h"
 #include "ui/base/ui_base_features.h"
 
 namespace chromeos {

chrome/browser/ui/webui/chromeos/login/network_state_informer.cc

@@ -7,7 +7,6 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/screens/network_error.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chromeos/network/network_state.h"
@@ -105,10 +104,6 @@ void NetworkStateInformer::Init() {
       this, FROM_HERE);
 
   network_portal_detector::GetInstance()->AddAndFireObserver(this);
-
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_SESSION_STARTED,
-                 content::NotificationService::AllSources());
 }
 
 void NetworkStateInformer::AddObserver(NetworkStateInformerObserver* observer) {
@@ -131,16 +126,6 @@ void NetworkStateInformer::OnPortalDetectionCompleted(
   UpdateStateAndNotify();
 }
 
-void NetworkStateInformer::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_SESSION_STARTED)
-    registrar_.RemoveAll();
-  else
-    NOTREACHED() << "Unknown notification: " << type;
-}
-
 void NetworkStateInformer::OnPortalDetected() {
   UpdateStateAndNotify();
 }

chrome/browser/ui/webui/chromeos/login/network_state_informer.h

@@ -18,9 +18,6 @@
 #include "chrome/browser/chromeos/login/ui/captive_portal_window_proxy.h"
 #include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/network/portal_detector/network_portal_detector.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/notification_service.h"
 
 namespace base {
 class Value;
@@ -34,7 +31,6 @@ namespace chromeos {
 class NetworkStateInformer
     : public chromeos::NetworkStateHandlerObserver,
       public chromeos::NetworkPortalDetector::Observer,
-      public content::NotificationObserver,
       public CaptivePortalWindowProxyDelegate,
       public base::RefCounted<NetworkStateInformer> {
  public:
@@ -74,11 +70,6 @@ class NetworkStateInformer
       const NetworkState* network,
       const NetworkPortalDetector::CaptivePortalState& state) override;
 
-  // content::NotificationObserver implementation.
-  void Observe(int type,
-               const content::NotificationSource& source,
-               const content::NotificationDetails& details) override;
-
   // CaptivePortalWindowProxyDelegate implementation:
   void OnPortalDetected() override;
 
@@ -102,7 +93,6 @@ class NetworkStateInformer
   std::unique_ptr<base::Value> proxy_config_;
 
   base::ObserverList<NetworkStateInformerObserver>::Unchecked observers_;
-  content::NotificationRegistrar registrar_;
 
   base::WeakPtrFactory<NetworkStateInformer> weak_ptr_factory_{this};
 };

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

@@ -97,6 +97,8 @@
 #include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_type.h"
 #include "components/version_info/version_info.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "google_apis/gaia/gaia_auth_util.h"