Fix handling of device cloud signing policy key rotation

chrome/browser/chromeos/settings/session_manager_operation.cc

Index: chrome/browser/chromeos/settings/session_manager_operation.cc
diff --git a/chrome/browser/chromeos/settings/session_manager_operation.cc b/chrome/browser/chromeos/settings/session_manager_operation.cc
index bfe8316c87e30cb90f995b6934152ddfec25c6e7..64db533a82372e60e72b69293e5f46e5813be0e2 100644
--- a/chrome/browser/chromeos/settings/session_manager_operation.cc
+++ b/chrome/browser/chromeos/settings/session_manager_operation.cc
@@ -46,7 +46,7 @@ void SessionManagerOperation::Start(
 
 void SessionManagerOperation::RestartLoad(bool key_changed) {
   if (key_changed)
-    public_key_ = NULL;
+    public_key_ = nullptr;
 
   if (!is_loading_)
     return;
@@ -76,20 +76,17 @@ void SessionManagerOperation::ReportResult(
 }
 
 void SessionManagerOperation::EnsurePublicKey(const base::Closure& callback) {
-  if (force_key_load_ || !public_key_.get() || !public_key_->is_loaded()) {
+  if (force_key_load_ || !public_key_ || !public_key_->is_loaded()) {
     scoped_refptr<base::TaskRunner> task_runner =
         content::BrowserThread::GetBlockingPool()
             ->GetTaskRunnerWithShutdownBehavior(
                 base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
     base::PostTaskAndReplyWithResult(
-        task_runner.get(),
-        FROM_HERE,
-        base::Bind(&SessionManagerOperation::LoadPublicKey,
-                   owner_key_util_,
-                   public_key_),
+        task_runner.get(), FROM_HERE,
+        base::Bind(&SessionManagerOperation::LoadPublicKey, owner_key_util_,
+                   force_key_load_ ? nullptr : public_key_),
         base::Bind(&SessionManagerOperation::StorePublicKey,
-                   weak_factory_.GetWeakPtr(),
-                   callback));
+                   weak_factory_.GetWeakPtr(), callback));
   } else {
     callback.Run();
   }
@@ -102,7 +99,7 @@ scoped_refptr<PublicKey> SessionManagerOperation::LoadPublicKey(
   scoped_refptr<PublicKey> public_key(new PublicKey());
 
   // Keep already-existing public key.
-  if (current_key.get() && current_key->is_loaded()) {
+  if (current_key && current_key->is_loaded()) {
     public_key->data() = current_key->data();
   }
   if (!public_key->is_loaded() && util->IsPublicKeyPresent()) {
@@ -118,7 +115,7 @@ void SessionManagerOperation::StorePublicKey(const base::Closure& callback,
   force_key_load_ = false;
   public_key_ = new_key;
 
-  if (!public_key_.get() || !public_key_->is_loaded()) {
+  if (!public_key_ || !public_key_->is_loaded()) {
     ReportResult(DeviceSettingsService::STORE_KEY_UNAVAILABLE);
     return;
   }
@@ -236,7 +233,10 @@ StoreSettingsOperation::StoreSettingsOperation(
     std::unique_ptr<em::PolicyFetchResponse> policy)
     : SessionManagerOperation(callback),
       policy_(std::move(policy)),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+  if (policy_->has_new_public_key())
+    force_key_load_ = true;

[Mattias Nissler (ping if slow), 2016/12/08 12:32:21]

This change is good as it'll make sure won't race. FWIW, I'd still advise to
keep the OWNER_KEY_SET handling intact (which you do), since for some cases keys
are generated and installed outside Chrome (e.g. consumer ownership).

+}
 
 StoreSettingsOperation::~StoreSettingsOperation() {}