Commit 1b02f435 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Re-land "Remove NOTIFICATION_PROFILE_ADDED from UserCloudPolicyManagerChromeOS"

This re-lands commit 00722f1e.

The change was reverted for causing unit test failures under asan.
The fix for those failures was committed as e9179a4f.
Thus, this change is identical to the original.

Original change's description:
> Remove NOTIFICATION_PROFILE_ADDED from UserCloudPolicyManagerChromeOS
>
> Use ProfileManagerObserver::OnProfileAdded instead.
>
> Also worth noting that this removes an obsolete superclass from
> UserCloudPolicyManagerChromeOS, i.e. KeyedService. Both KeyedService
> and ConfigurationPolicyProvider have a "virtual void Shutdown()"
> so this removes that collision (not that it was doing any harm).
>
> Bug: 268984,171406
> Change-Id: Ia0b3dd4dd34a0f62e84dd057928d61c5677231d5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854525
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: Denis Kuznetsov <antrim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#707909}

TBR=antrim@chromium.org

Change-Id: I9c7342b0cdf9b8d1555c27ab41ff1015633dbbf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874717Reviewed-by: default avatarEvan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708427}
parent c0cca6e4
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/login/helper.h" #include "chrome/browser/chromeos/login/helper.h"
#include "chrome/browser/chromeos/login/session/user_session_manager.h" #include "chrome/browser/chromeos/login/session/user_session_manager.h"
#include "chrome/browser/chromeos/login/users/affiliation.h" #include "chrome/browser/chromeos/login/users/affiliation.h"
...@@ -52,8 +51,6 @@ ...@@ -52,8 +51,6 @@
#include "components/user_manager/user.h" #include "components/user_manager/user.h"
#include "components/user_manager/user_manager.h" #include "components/user_manager/user_manager.h"
#include "content/public/browser/network_service_instance.h" #include "content/public/browser/network_service_instance.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_source.h"
#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_context_getter.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/shared_url_loader_factory.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -190,9 +187,8 @@ UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS( ...@@ -190,9 +187,8 @@ UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS(
// for creating the invalidator for user remote commands. The invalidator must // for creating the invalidator for user remote commands. The invalidator must
// not be initialized before then because the invalidation service cannot be // not be initialized before then because the invalidation service cannot be
// started because it depends on components initialized at the end of profile // started because it depends on components initialized at the end of profile
// creation. // creation. https://crbug.com/171406
registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED, observed_profile_manager_.Add(g_browser_process->profile_manager());
content::Source<Profile>(profile));
} }
void UserCloudPolicyManagerChromeOS::ForceTimeoutForTest() { void UserCloudPolicyManagerChromeOS::ForceTimeoutForTest() {
...@@ -213,7 +209,7 @@ void UserCloudPolicyManagerChromeOS::SetSystemURLLoaderFactoryForTests( ...@@ -213,7 +209,7 @@ void UserCloudPolicyManagerChromeOS::SetSystemURLLoaderFactoryForTests(
system_url_loader_factory_for_tests_ = system_url_loader_factory; system_url_loader_factory_for_tests_ = system_url_loader_factory;
} }
UserCloudPolicyManagerChromeOS::~UserCloudPolicyManagerChromeOS() {} UserCloudPolicyManagerChromeOS::~UserCloudPolicyManagerChromeOS() = default;
void UserCloudPolicyManagerChromeOS::Connect( void UserCloudPolicyManagerChromeOS::Connect(
PrefService* local_state, PrefService* local_state,
...@@ -352,6 +348,7 @@ UserCloudPolicyManagerChromeOS::GetAppInstallEventLogUploader() { ...@@ -352,6 +348,7 @@ UserCloudPolicyManagerChromeOS::GetAppInstallEventLogUploader() {
} }
void UserCloudPolicyManagerChromeOS::Shutdown() { void UserCloudPolicyManagerChromeOS::Shutdown() {
observed_profile_manager_.RemoveAll();
app_install_event_log_uploader_.reset(); app_install_event_log_uploader_.reset();
if (client()) if (client())
client()->RemoveObserver(this); client()->RemoveObserver(this);
...@@ -735,16 +732,11 @@ void UserCloudPolicyManagerChromeOS::StartRefreshSchedulerIfReady() { ...@@ -735,16 +732,11 @@ void UserCloudPolicyManagerChromeOS::StartRefreshSchedulerIfReady() {
policy_prefs::kUserPolicyRefreshRate); policy_prefs::kUserPolicyRefreshRate);
} }
void UserCloudPolicyManagerChromeOS::Observe( void UserCloudPolicyManagerChromeOS::OnProfileAdded(Profile* profile) {
int type, if (profile != profile_)
const content::NotificationSource& source, return;
const content::NotificationDetails& details) {
DCHECK_EQ(chrome::NOTIFICATION_PROFILE_ADDED, type);
// Now that the profile is fully created we can unsubscribe from the observed_profile_manager_.RemoveAll();
// notification.
registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_ADDED,
content::Source<Profile>(profile_));
// If true FCMInvalidationService will be used as invalidation service and // If true FCMInvalidationService will be used as invalidation service and
// TiclInvalidationService otherwise. // TiclInvalidationService otherwise.
......
...@@ -14,9 +14,12 @@ ...@@ -14,9 +14,12 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/scoped_observer.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "chrome/browser/chromeos/policy/wildcard_login_checker.h" #include "chrome/browser/chromeos/policy/wildcard_login_checker.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile_manager_observer.h"
#include "components/account_id/account_id.h" #include "components/account_id/account_id.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/keyed_service/core/keyed_service_shutdown_notifier.h" #include "components/keyed_service/core/keyed_service_shutdown_notifier.h"
...@@ -24,8 +27,6 @@ ...@@ -24,8 +27,6 @@
#include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h"
#include "components/policy/core/common/cloud/cloud_policy_manager.h" #include "components/policy/core/common/cloud/cloud_policy_manager.h"
#include "components/policy/core/common/cloud/cloud_policy_service.h" #include "components/policy/core/common/cloud/cloud_policy_service.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
class GoogleServiceAuthError; class GoogleServiceAuthError;
class PrefService; class PrefService;
...@@ -51,8 +52,7 @@ class RemoteCommandsInvalidator; ...@@ -51,8 +52,7 @@ class RemoteCommandsInvalidator;
class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
public CloudPolicyClient::Observer, public CloudPolicyClient::Observer,
public CloudPolicyService::Observer, public CloudPolicyService::Observer,
public content::NotificationObserver, public ProfileManagerObserver {
public KeyedService {
public: public:
// Enum describing what behavior we want to enforce here. // Enum describing what behavior we want to enforce here.
enum class PolicyEnforcement { enum class PolicyEnforcement {
...@@ -223,12 +223,10 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, ...@@ -223,12 +223,10 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
// call it multiple times. // call it multiple times.
void StartRefreshSchedulerIfReady(); void StartRefreshSchedulerIfReady();
// content::NotificationObserver: // ProfileManagerObserver:
void Observe(int type, void OnProfileAdded(Profile* profile) override;
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
// Observer called on profile shutdown. // Called on profile shutdown.
void ProfileShutdown(); void ProfileShutdown();
// Profile associated with the current user. // Profile associated with the current user.
...@@ -288,9 +286,6 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, ...@@ -288,9 +286,6 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
// injected in the constructor to make it easier to write tests. // injected in the constructor to make it easier to write tests.
base::OnceClosure fatal_error_callback_; base::OnceClosure fatal_error_callback_;
// Used to register for notification that profile creation is complete.
content::NotificationRegistrar registrar_;
// Invalidator used for remote commands to be delivered to this user. // Invalidator used for remote commands to be delivered to this user.
std::unique_ptr<RemoteCommandsInvalidator> invalidator_; std::unique_ptr<RemoteCommandsInvalidator> invalidator_;
...@@ -304,6 +299,9 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, ...@@ -304,6 +299,9 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
scoped_refptr<network::SharedURLLoaderFactory> scoped_refptr<network::SharedURLLoaderFactory>
signin_url_loader_factory_for_tests_; signin_url_loader_factory_for_tests_;
ScopedObserver<ProfileManager, ProfileManagerObserver>
observed_profile_manager_{this};
// Refresh token used in tests instead of the user context refresh token to // Refresh token used in tests instead of the user context refresh token to
// fetch the policy OAuth token. // fetch the policy OAuth token.
base::Optional<std::string> user_context_refresh_token_for_tests_; base::Optional<std::string> user_context_refresh_token_for_tests_;
......
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