Index: chrome/browser/chromeos/policy/active_directory_policy_manager.cc |
diff --git a/chrome/browser/chromeos/policy/active_directory_policy_manager.cc b/chrome/browser/chromeos/policy/active_directory_policy_manager.cc |
index 195140d9e972a2b2efc033267aa4707a4a941935..fc1f40020a4d6a3e080b96a47ccd5d2f7af2ba84 100644 |
--- a/chrome/browser/chromeos/policy/active_directory_policy_manager.cc |
+++ b/chrome/browser/chromeos/policy/active_directory_policy_manager.cc |
@@ -31,17 +31,24 @@ ActiveDirectoryPolicyManager::~ActiveDirectoryPolicyManager() {} |
std::unique_ptr<ActiveDirectoryPolicyManager> |
ActiveDirectoryPolicyManager::CreateForDevicePolicy( |
std::unique_ptr<CloudPolicyStore> store) { |
- return base::WrapUnique( |
- new ActiveDirectoryPolicyManager(EmptyAccountId(), std::move(store))); |
+ // Can't use MakeUnique<> because the constructor is private. |
+ return base::WrapUnique(new ActiveDirectoryPolicyManager( |
+ EmptyAccountId(), false, base::TimeDelta(), base::OnceClosure(), |
+ std::move(store))); |
} |
// static |
std::unique_ptr<ActiveDirectoryPolicyManager> |
ActiveDirectoryPolicyManager::CreateForUserPolicy( |
const AccountId& account_id, |
+ bool wait_for_policy_fetch, |
+ base::TimeDelta initial_policy_fetch_timeout, |
+ base::OnceClosure exit_session, |
std::unique_ptr<CloudPolicyStore> store) { |
- return base::WrapUnique( |
- new ActiveDirectoryPolicyManager(account_id, std::move(store))); |
+ // Can't use MakeUnique<> because the constructor is private. |
+ return base::WrapUnique(new ActiveDirectoryPolicyManager( |
+ account_id, wait_for_policy_fetch, initial_policy_fetch_timeout, |
+ std::move(exit_session), std::move(store))); |
} |
void ActiveDirectoryPolicyManager::Init(SchemaRegistry* registry) { |
@@ -70,10 +77,13 @@ void ActiveDirectoryPolicyManager::Shutdown() { |
bool ActiveDirectoryPolicyManager::IsInitializationComplete( |
PolicyDomain domain) const { |
+ if (waiting_for_initial_policy_fetch_) { |
+ return false; |
+ } |
if (domain == POLICY_DOMAIN_CHROME) { |
return store_->is_initialized(); |
} |
- return true; |
+ return false; |
emaxx
2017/07/12 20:18:19
Not sure I understand this change: now the compone
Thiemo Nagel
2017/07/14 12:18:55
Since we don't support component policy for Chroma
Thiemo Nagel
2017/07/14 12:21:29
I've changed it back.
|
} |
void ActiveDirectoryPolicyManager::RefreshPolicies() { |
@@ -84,6 +94,9 @@ void ActiveDirectoryPolicyManager::OnStoreLoaded( |
CloudPolicyStore* cloud_policy_store) { |
DCHECK_EQ(store_.get(), cloud_policy_store); |
PublishPolicy(); |
+ if (fetch_ever_completed_) { |
+ CancelWaitForPolicy(fetch_ever_succeeded_ /* success */); |
+ } |
} |
void ActiveDirectoryPolicyManager::OnStoreError( |
@@ -93,12 +106,39 @@ void ActiveDirectoryPolicyManager::OnStoreError( |
// complete on the ConfigurationPolicyProvider interface. Technically, this is |
// only required on the first load, but doesn't hurt in any case. |
PublishPolicy(); |
+ if (fetch_ever_completed_) { |
+ CancelWaitForPolicy(false /* success */); |
+ } |
+} |
+ |
+void ActiveDirectoryPolicyManager::ForceTimeoutForTest() { |
+ DCHECK(initial_policy_timeout_.IsRunning()); |
+ // Stop the timer to mimic what happens when a real timer fires, then invoke |
+ // the timer callback directly. |
+ initial_policy_timeout_.Stop(); |
+ OnBlockingFetchTimeout(); |
} |
ActiveDirectoryPolicyManager::ActiveDirectoryPolicyManager( |
const AccountId& account_id, |
+ bool wait_for_policy_fetch, |
+ base::TimeDelta initial_policy_fetch_timeout, |
+ base::OnceClosure exit_session, |
std::unique_ptr<CloudPolicyStore> store) |
- : account_id_(account_id), store_(std::move(store)) {} |
+ : account_id_(account_id), |
+ waiting_for_initial_policy_fetch_(wait_for_policy_fetch), |
+ exit_session_(std::move(exit_session)), |
+ store_(std::move(store)) { |
+ DCHECK(!(account_id == EmptyAccountId() && wait_for_policy_fetch)); |
+ DCHECK_NE(wait_for_policy_fetch, initial_policy_fetch_timeout.is_zero()); |
+ initial_policy_fetch_may_fail_ = !initial_policy_fetch_timeout.is_max(); |
+ if (wait_for_policy_fetch && !initial_policy_fetch_timeout.is_max()) { |
+ initial_policy_timeout_.Start( |
+ FROM_HERE, initial_policy_fetch_timeout, |
+ base::Bind(&ActiveDirectoryPolicyManager::OnBlockingFetchTimeout, |
+ base::Unretained(this))); |
emaxx
2017/07/12 20:18:19
nit: Ideally we'd have this documented why Unretai
Thiemo Nagel
2017/07/14 12:18:55
Good point. It used to be safe because the timer w
|
+ } |
+} |
void ActiveDirectoryPolicyManager::PublishPolicy() { |
if (!store_->is_initialized()) { |
@@ -133,12 +173,60 @@ void ActiveDirectoryPolicyManager::DoFetch( |
} |
void ActiveDirectoryPolicyManager::OnPolicyFetched(bool success) { |
- if (!success) { |
+ fetch_ever_completed_ = true; |
+ if (success) { |
+ fetch_ever_succeeded_ = true; |
+ } else { |
LOG(ERROR) << "Active Directory policy fetch failed."; |
+ if (store_->is_initialized()) { |
+ CancelWaitForPolicy(false /* success */); |
+ } |
} |
// Load independently of success or failure to keep up to date with whatever |
// has happened on the authpolicyd / session manager side. |
store_->Load(); |
} |
+void ActiveDirectoryPolicyManager::OnBlockingFetchTimeout() { |
+ DCHECK(waiting_for_initial_policy_fetch_); |
+ LOG(WARNING) << "Timed out while waiting for the policy fetch. " |
+ << "The session will start with the cached policy."; |
+ CancelWaitForPolicy(false); |
+} |
+ |
+void ActiveDirectoryPolicyManager::CancelWaitForPolicy(bool success) { |
+ if (!waiting_for_initial_policy_fetch_) |
+ return; |
+ |
+ initial_policy_timeout_.Stop(); |
+ |
+ // If there was an error, and we don't want to allow profile initialization |
+ // to go forward after a failed policy fetch, then just return (profile |
+ // initialization will not complete). |
+ // TODO(tnagel): Maybe add code to retry policy fetching? |
+ if (!store_->has_policy()) { |
emaxx
2017/07/12 20:18:20
I'm quite confused about complexity of the state m
Thiemo Nagel
2017/07/14 12:18:55
After a lot of offline discussion: It's hard to si
|
+ // If there's no policy at all (not even cached) we can't continue. |
+ LOG(ERROR) << "Policy could not be obtained. " |
+ << "Aborting profile initialization"; |
+ if (exit_session_) { |
+ std::move(exit_session_).Run(); |
+ } |
+ return; |
+ } |
+ if (!success && !initial_policy_fetch_may_fail_) { |
+ LOG(ERROR) << "Policy fetch failed for the user. " |
+ << "Aborting profile initialization"; |
+ if (exit_session_) { |
+ std::move(exit_session_).Run(); |
+ } |
+ return; |
+ } |
+ |
+ waiting_for_initial_policy_fetch_ = false; |
+ |
+ // Publish policy (even though it hasn't changed) in order to signal load |
+ // complete on the ConfigurationPolicyProvider interface. |
+ PublishPolicy(); |
+} |
+ |
} // namespace policy |