Commit b9743e0e authored by Ella Ge's avatar Ella Ge Committed by Commit Bot

Revert "Remove NOTIFICATION_PROFILE_ADDED from UserCloudPolicyManagerChromeOS"

This reverts commit 00722f1e.

Reason for revert: Suspect causing UserCloudPolicyTokenForwarderTest crash https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/15882


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,bartfab@chromium.org,estade@chromium.org

Change-Id: Iacac6735ab45557c35a3c05e58e142ec26cecca7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 268984, 171406
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874185Reviewed-by: default avatarElla Ge <eirage@chromium.org>
Commit-Queue: Ella Ge <eirage@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708246}
parent bf8cb87f
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#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"
...@@ -51,6 +52,8 @@ ...@@ -51,6 +52,8 @@
#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"
...@@ -187,8 +190,9 @@ UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS( ...@@ -187,8 +190,9 @@ 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. https://crbug.com/171406 // creation.
observed_profile_manager_.Add(g_browser_process->profile_manager()); registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
content::Source<Profile>(profile));
} }
void UserCloudPolicyManagerChromeOS::ForceTimeoutForTest() { void UserCloudPolicyManagerChromeOS::ForceTimeoutForTest() {
...@@ -209,7 +213,7 @@ void UserCloudPolicyManagerChromeOS::SetSystemURLLoaderFactoryForTests( ...@@ -209,7 +213,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() = default; UserCloudPolicyManagerChromeOS::~UserCloudPolicyManagerChromeOS() {}
void UserCloudPolicyManagerChromeOS::Connect( void UserCloudPolicyManagerChromeOS::Connect(
PrefService* local_state, PrefService* local_state,
...@@ -348,7 +352,6 @@ UserCloudPolicyManagerChromeOS::GetAppInstallEventLogUploader() { ...@@ -348,7 +352,6 @@ 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);
...@@ -732,11 +735,16 @@ void UserCloudPolicyManagerChromeOS::StartRefreshSchedulerIfReady() { ...@@ -732,11 +735,16 @@ void UserCloudPolicyManagerChromeOS::StartRefreshSchedulerIfReady() {
policy_prefs::kUserPolicyRefreshRate); policy_prefs::kUserPolicyRefreshRate);
} }
void UserCloudPolicyManagerChromeOS::OnProfileAdded(Profile* profile) { void UserCloudPolicyManagerChromeOS::Observe(
if (profile != profile_) int type,
return; const content::NotificationSource& source,
const content::NotificationDetails& details) {
DCHECK_EQ(chrome::NOTIFICATION_PROFILE_ADDED, type);
observed_profile_manager_.RemoveAll(); // Now that the profile is fully created we can unsubscribe from the
// 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,12 +14,9 @@ ...@@ -14,12 +14,9 @@
#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"
...@@ -27,6 +24,8 @@ ...@@ -27,6 +24,8 @@
#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;
...@@ -52,7 +51,8 @@ class RemoteCommandsInvalidator; ...@@ -52,7 +51,8 @@ class RemoteCommandsInvalidator;
class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
public CloudPolicyClient::Observer, public CloudPolicyClient::Observer,
public CloudPolicyService::Observer, public CloudPolicyService::Observer,
public ProfileManagerObserver { public content::NotificationObserver,
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,10 +223,12 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, ...@@ -223,10 +223,12 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
// call it multiple times. // call it multiple times.
void StartRefreshSchedulerIfReady(); void StartRefreshSchedulerIfReady();
// ProfileManagerObserver: // content::NotificationObserver:
void OnProfileAdded(Profile* profile) override; void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
// Called on profile shutdown. // Observer called on profile shutdown.
void ProfileShutdown(); void ProfileShutdown();
// Profile associated with the current user. // Profile associated with the current user.
...@@ -286,6 +288,9 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, ...@@ -286,6 +288,9 @@ 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_;
...@@ -299,9 +304,6 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager, ...@@ -299,9 +304,6 @@ 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