Fix cloud policy duplicate registrations issue.

chrome/browser/policy/device_management_service.cc

Index: chrome/browser/policy/device_management_service.cc
diff --git a/chrome/browser/policy/device_management_service.cc b/chrome/browser/policy/device_management_service.cc
index b218e31d0e00fcd7eda7f0c4e0d7597b0476159e..7cdc87d30f3cc1e425d12e65c6be76982812fd76 100644
--- a/chrome/browser/policy/device_management_service.cc
+++ b/chrome/browser/policy/device_management_service.cc
@@ -53,6 +53,9 @@ const char kPostContentType[] = "application/protobuf";
 const char kServiceTokenAuthHeader[] = "Authorization: GoogleLogin auth=";
 const char kDMTokenAuthHeader[] = "Authorization: GoogleDMToken token=";
 
+// Number of times to retry on ERR_NETWORK_CHANGED errors.
+const int kMaxNetworkChangedRetries = 3;
+
 // HTTP Error Codes of the DM Server with their concrete meanings in the context
 // of the DM Server communication.
 const int kSuccess = 200;
@@ -90,11 +93,38 @@ bool IsProxyError(const net::URLRequestStatus status) {
   return false;
 }
 
-bool IsProtobufMimeType(const net::URLFetcher* source) {
-  return source->GetResponseHeaders()->HasHeaderValue(
+bool IsProtobufMimeType(const net::URLFetcher* fetcher) {
+  return fetcher->GetResponseHeaders()->HasHeaderValue(
       "content-type", "application/x-protobuffer");
 }
 
+bool FailedWithProxy(const net::URLFetcher* fetcher) {
+  if ((fetcher->GetLoadFlags() & net::LOAD_BYPASS_PROXY) != 0) {
+    // The request didn't use a proxy.
+    return false;
+  }
+
+  if (!fetcher->GetStatus().is_success() &&
+      IsProxyError(fetcher->GetStatus())) {
+    LOG(WARNING) << "Proxy failed while contacting dmserver.";
+    return true;
+  }
+
+  if (fetcher->GetStatus().is_success() &&
+      fetcher->GetResponseCode() == kSuccess &&
+      fetcher->WasFetchedViaProxy() &&
+      !IsProtobufMimeType(fetcher)) {
+    // The proxy server can be misconfigured but pointing to an existing
+    // server that replies to requests. Try to recover if a successful
+    // request that went through a proxy returns an unexpected mime type.
+    LOG(WARNING) << "Got bad mime-type in response from dmserver that was "
+                 << "fetched via a proxy.";
+    return true;
+  }
+
+  return false;
+}
+
 const char* UserAffiliationToString(UserAffiliation affiliation) {
   switch (affiliation) {
     case USER_AFFILIATION_MANAGED:
@@ -275,6 +305,14 @@ class DeviceManagementRequestJobImpl : public DeviceManagementRequestJob {
   // Configures the fetcher, setting up payload and headers.
   void ConfigureRequest(net::URLFetcher* fetcher);
 
+  // Returns true if this job should be retried. |fetcher| has just completed,
+  // and can be inspected to determine if the request failed and should be
+  // retried.
+  bool ShouldRetry(const net::URLFetcher* fetcher);
+
+  // Invoked right before retrying this job.
+  void PrepareRetry();
+
  protected:
   // DeviceManagementRequestJob:
   virtual void Run() OVERRIDE;
@@ -286,6 +324,12 @@ class DeviceManagementRequestJobImpl : public DeviceManagementRequestJob {
   // Pointer to the service this job is associated with.
   DeviceManagementService* service_;
 
+  // Whether the BYPASS_PROXY flag should be set by ConfigureRequest().
+  bool bypass_proxy_;
+
+  // Number of times that this job has been retried due to ERR_NETWORK_CHANGED.
+  int retries_count_;
+
   DISALLOW_COPY_AND_ASSIGN(DeviceManagementRequestJobImpl);
 };
 
@@ -293,7 +337,9 @@ DeviceManagementRequestJobImpl::DeviceManagementRequestJobImpl(
     JobType type,
     DeviceManagementService* service)
     : DeviceManagementRequestJob(type),
-      service_(service) {}
+      service_(service),
+      bypass_proxy_(false),
+      retries_count_(0) {}
 
 DeviceManagementRequestJobImpl::~DeviceManagementRequestJobImpl() {
   service_->RemoveJob(this);
@@ -389,6 +435,10 @@ GURL DeviceManagementRequestJobImpl::GetURL(
 
 void DeviceManagementRequestJobImpl::ConfigureRequest(
     net::URLFetcher* fetcher) {
+  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
+                        net::LOAD_DO_NOT_SAVE_COOKIES |
+                        net::LOAD_DISABLE_CACHE |
+                        (bypass_proxy_ ? net::LOAD_BYPASS_PROXY : 0));
   std::string payload;
   CHECK(request_.SerializeToString(&payload));
   fetcher->SetUploadData(kPostContentType, payload);
@@ -400,6 +450,35 @@ void DeviceManagementRequestJobImpl::ConfigureRequest(
   fetcher->SetExtraRequestHeaders(extra_headers);
 }
 
+bool DeviceManagementRequestJobImpl::ShouldRetry(
+    const net::URLFetcher* fetcher) {
+  if (FailedWithProxy(fetcher) && !bypass_proxy_) {
+    // Retry the job if it failed due to a broken proxy, by bypassing the
+    // proxy on the next try.
+    bypass_proxy_ = true;
+    return true;
+  }
+
+  // Early device policy fetches on ChromeOS and Auto-Enrollment checks are
+  // often interrupted during ChromeOS startup when network change notifications
+  // are sent. Allowing the fetcher to retry once after that is enough to
+  // recover; allow it to retry up to 3 times just in case.
+  if (fetcher->GetStatus().error() == net::ERR_NETWORK_CHANGED &&
+      retries_count_ < kMaxNetworkChangedRetries) {
+    ++retries_count_;
+    return true;
+  }
+
+  // The request didn't fail, or the limit of retry attempts has been reached;
+  // forward the result to the job owner.
+  return false;
+}
+
+void DeviceManagementRequestJobImpl::PrepareRetry() {
+  if (!retry_callback_.is_null())
+    retry_callback_.Run(this);
+}
+
 void DeviceManagementRequestJobImpl::ReportError(DeviceManagementStatus code) {
   em::DeviceManagementResponse dummy_response;
   callback_.Run(code, dummy_response);
@@ -441,6 +520,11 @@ DeviceManagementRequestJob::DeviceManagementRequestJob(JobType type) {
   AddParameter(dm_protocol::kParamPlatform, GetPlatformString());
 }
 
+void DeviceManagementRequestJob::SetRetryCallback(
+    const RetryCallback& retry_callback) {
+  retry_callback_ = retry_callback;
+}
+
 void DeviceManagementRequestJob::Start(const Callback& callback) {
   callback_ = callback;
   Run();
@@ -451,6 +535,9 @@ void DeviceManagementRequestJob::AddParameter(const std::string& name,
   query_params_.push_back(std::make_pair(name, value));
 }
 
+// A random value that other fetchers won't likely use.
+const int DeviceManagementService::kURLFetcherID = 0xde71ce1d;
+
 DeviceManagementService::~DeviceManagementService() {
   // All running jobs should have been cancelled by now.
   DCHECK(pending_jobs_.empty());
@@ -481,7 +568,7 @@ void DeviceManagementService::Initialize() {
   initialized_ = true;
 
   while (!queued_jobs_.empty()) {
-    StartJob(queued_jobs_.front(), false);
+    StartJob(queued_jobs_.front());
     queued_jobs_.pop_front();
   }
 }
@@ -503,21 +590,10 @@ DeviceManagementService::DeviceManagementService(
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
 
-void DeviceManagementService::StartJob(DeviceManagementRequestJobImpl* job,
-                                       bool bypass_proxy) {
+void DeviceManagementService::StartJob(DeviceManagementRequestJobImpl* job) {
   net::URLFetcher* fetcher = net::URLFetcher::Create(
-      0, job->GetURL(server_url_), net::URLFetcher::POST, this);
-  fetcher->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                        net::LOAD_DO_NOT_SAVE_COOKIES |
-                        net::LOAD_DISABLE_CACHE |
-                        (bypass_proxy ? net::LOAD_BYPASS_PROXY : 0));
+      kURLFetcherID, job->GetURL(server_url_), net::URLFetcher::POST, this);
   fetcher->SetRequestContext(request_context_getter_.get());
-  // Early device policy fetches on ChromeOS and Auto-Enrollment checks are
-  // often interrupted during ChromeOS startup when network change notifications
-  // are sent. Allowing the fetcher to retry once after that is enough to
-  // recover; allow it to retry up to 3 times just in case.
-  // http://crosbug.com/16114
-  fetcher->SetAutomaticallyRetryOnNetworkChanges(3);
   job->ConfigureRequest(fetcher);
   pending_jobs_[fetcher] = job;
   fetcher->Start();
@@ -534,31 +610,10 @@ void DeviceManagementService::OnURLFetchComplete(
   DeviceManagementRequestJobImpl* job = entry->second;
   pending_jobs_.erase(entry);
 
-  // Retry the job if it failed due to a broken proxy, by bypassing the
-  // proxy on the next try. Don't retry if this URLFetcher already bypassed
-  // the proxy.
-  bool retry = false;
-  if ((source->GetLoadFlags() & net::LOAD_BYPASS_PROXY) == 0) {
-    if (!source->GetStatus().is_success() &&
-        IsProxyError(source->GetStatus())) {
-      LOG(WARNING) << "Proxy failed while contacting dmserver.";
-      retry = true;
-    } else if (source->GetStatus().is_success() &&
-               source->GetResponseCode() == kSuccess &&
-               source->WasFetchedViaProxy() &&
-               !IsProtobufMimeType(source)) {
-      // The proxy server can be misconfigured but pointing to an existing
-      // server that replies to requests. Try to recover if a successful
-      // request that went through a proxy returns an unexpected mime type.
-      LOG(WARNING) << "Got bad mime-type in response from dmserver that was "
-                   << "fetched via a proxy.";
-      retry = true;
-    }
-  }
-
-  if (retry) {
-    LOG(WARNING) << "Retrying dmserver request without using a proxy.";
-    StartJob(job, true);
+  if (job->ShouldRetry(source)) {
+    VLOG(1) << "Retrying dmserver request.";
+    job->PrepareRetry();
+    StartJob(job);
   } else {
     std::string data;
     source->GetResponseAsString(&data);
@@ -570,7 +625,7 @@ void DeviceManagementService::OnURLFetchComplete(
 
 void DeviceManagementService::AddJob(DeviceManagementRequestJobImpl* job) {
   if (initialized_)
-    StartJob(job, false);
+    StartJob(job);
   else
     queued_jobs_.push_back(job);
 }