Refactor common invalidation framework types

chrome/browser/policy/cloud/cloud_policy_invalidator.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/policy/cloud/cloud_policy_invalidator.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/hash.h"
 #include "base/location.h"
@@ skipping 74 matching lines @@
   DCHECK(state_ == STARTED);
   DCHECK(thread_checker_.CalledOnValidThread());
   invalidation_service_enabled_ = state == syncer::INVALIDATIONS_ENABLED;
   UpdateInvalidationsEnabled();
 }
 
 void CloudPolicyInvalidator::OnIncomingInvalidation(
     const syncer::ObjectIdInvalidationMap& invalidation_map) {
   DCHECK(state_ == STARTED);
   DCHECK(thread_checker_.CalledOnValidThread());
-  const syncer::ObjectIdInvalidationMap::const_iterator invalidation =
-      invalidation_map.find(object_id_);
-  if (invalidation == invalidation_map.end()) {
+  const syncer::SingleObjectInvalidationSet& list =
+      invalidation_map.ForObject(object_id_);
+  if (list.IsEmpty()) {
     NOTREACHED();
     return;
   }
-  HandleInvalidation(invalidation->second);
+  HandleInvalidation(list.back());
 }
 
 void CloudPolicyInvalidator::OnCoreConnected(CloudPolicyCore* core) {}
 
 void CloudPolicyInvalidator::OnRefreshSchedulerStarted(CloudPolicyCore* core) {
   DCHECK(state_ == STOPPED);
   DCHECK(thread_checker_.CalledOnValidThread());
   state_ = STARTED;
   OnStoreLoaded(core_->store());
   core_->store()->AddObserver(this);
@@ skipping 37 matching lines @@
   UpdateRegistration(store->policy());
   UpdateMaxFetchDelay(store->policy_map());
 }
 
 void CloudPolicyInvalidator::OnStoreError(CloudPolicyStore* store) {}
 
 void CloudPolicyInvalidator::HandleInvalidation(
     const syncer::Invalidation& invalidation) {
   // The invalidation service may send an invalidation more than once if there
   // is a delay in acknowledging it. Duplicate invalidations are ignored.
-  if (invalid_ && ack_handle_.Equals(invalidation.ack_handle))
+  if (invalid_ && ack_handle_.Equals(invalidation.ack_handle()))
     return;
 
   // If there is still a pending invalidation, acknowledge it, since we only
   // care about the latest invalidation.
   if (invalid_)
     AcknowledgeInvalidation();
 
   // Update invalidation state.
   invalid_ = true;
-  ack_handle_ = invalidation.ack_handle;
-  invalidation_version_ = invalidation.version;
+  ack_handle_ = invalidation.ack_handle();
 
   // When an invalidation with unknown version is received, use negative
   // numbers based on the number of such invalidations received. This
   // ensures that the version numbers do not collide with "real" versions
   // (which are positive) or previous invalidations with unknown version.
-  if (invalidation_version_ == syncer::Invalidation::kUnknownVersion)
+  if (invalidation.is_unknown_version()) {
     invalidation_version_ = -(++unknown_version_invalidation_count_);
+  } else {
+    invalidation_version_ = invalidation.version();
+  }
 
   // In order to prevent the cloud policy server from becoming overwhelmed when
   // a policy with many users is modified, delay for a random period of time
   // before fetching the policy. Delay for at least 20ms so that if multiple
   // invalidations are received in quick succession, only one fetch will be
   // performed.
   base::TimeDelta delay = base::TimeDelta::FromMilliseconds(
       base::RandInt(20, max_fetch_delay_));
 
-  // If there is a payload, the policy can be refreshed at any time, so set
-  // the version and payload on the client immediately. Otherwise, the refresh
+  std::string payload;
+  if (!invalidation.is_unknown_version())
+    payload = invalidation.payload();

[Steve Condie, 2013/10/01 18:53:10]

Optional. To avoid making an unnecessary copy of the payload here the payload
variable could still be a const ref with a conditional operator in the
initializer to make it refer to either invalidation.payload() or a new local
variable set to empty string.

[rlarocque, 2013/10/01 19:29:57]

That's a good suggestion, but I'd prefer to leave the code as is.  I'm working
under the assumption that none of this invalidations code is performance
critical, so I'm trying to optimize for clarity.  I'm also biased against the
ternary operator, and try to avoid it if possible.

+
+  // If there is a payload, the invalidate callback can run at any time, so set

[Steve Condie, 2013/10/01 18:53:10]

It looks like you accidentally modified this comment when merging one of my
changes. Please revert the comment change.

[rlarocque, 2013/10/01 19:29:57]

Sorry about that.  Reverted.

+  // the version and payload on the client immediately. Otherwise, the callback
   // must only run after at least kMissingPayloadDelay minutes.
-  const std::string& payload = invalidation.payload;
-  if (!invalidation.payload.empty())
+  if (!payload.empty())
     core_->client()->SetInvalidationInfo(invalidation_version_, payload);
   else
     delay += base::TimeDelta::FromMinutes(kMissingPayloadDelay);
 
   // Schedule the policy to be refreshed.
   task_runner_->PostDelayedTask(
       FROM_HERE,
       base::Bind(
           &CloudPolicyInvalidator::RefreshPolicy,
           weak_factory_.GetWeakPtr(),
@@ skipping 139 matching lines @@
     if (invalidations_enabled_)
       return METRIC_POLICY_REFRESH_CHANGED;
     return METRIC_POLICY_REFRESH_CHANGED_NO_INVALIDATIONS;
   }
   if (invalid_)
     return METRIC_POLICY_REFRESH_INVALIDATED_UNCHANGED;
   return METRIC_POLICY_REFRESH_UNCHANGED;
 }
 
 }  // namespace policy